API: remote support for VIR_TYPED_PARAM_STRING

Send and receive string typed parameters across RPC.  This also
completes the back-compat mentioned in the previous patch - the
only time we have an older client talking to a newer server is
if RPC is in use, so filtering out strings during RPC prevents
returning an unknown type to the older client.

* src/remote/remote_protocol.x (remote_typed_param_value): Add
another union value.
* daemon/remote.c (remoteDeserializeTypedParameters): Handle
strings on rpc.
(remoteSerializeTypedParameters): Likewise; plus filter out
strings when replying to older clients.  Adjust callers.
* src/remote/remote_driver.c (remoteFreeTypedParameters)
(remoteSerializeTypedParameters)
(remoteDeserializeTypedParameters): Handle strings on rpc.
* src/rpc/gendispatch.pl: Properly clean up typed arrays.
* src/remote_protocol-structs: Update.
Based on an initial patch by Hu Tao, with feedback from
Daniel P. Berrange.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/daemon/remote.c b/daemon/remote.c
index bd0c3e3a1e7727b0be8ff22c75ed7e5552b0c16b..aa3f7683d17afe5607dfb8f9cb15af76d8ef946b 100644 (file)
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -92,11 +92,6 @@ static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr
 static void make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterPtr nwfilter_src);
 static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src);
 
-static int
-remoteSerializeTypedParameters(virTypedParameterPtr params,
-                               int nparams,
-                               remote_typed_param **ret_params_val,
-                               u_int *ret_params_len);
 static virTypedParameterPtr
 remoteDeserializeTypedParameters(remote_typed_param *args_params_val,
                                  u_int args_params_len,
@@ -682,14 +677,17 @@ cleanup:
     return rv;
 }
 
-/* Helper to serialize typed parameters. */
+/* Helper to serialize typed parameters. This also filters out any string
+ * parameters that must not be returned to older clients.  */
 static int
 remoteSerializeTypedParameters(virTypedParameterPtr params,
                                int nparams,
                                remote_typed_param **ret_params_val,
-                               u_int *ret_params_len)
+                               u_int *ret_params_len,
+                               unsigned int flags)
 {
     int i;
+    int j;
     int rv = -1;
     remote_typed_param *val;
 
@@ -699,38 +697,53 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
         goto cleanup;
     }
 
-    for (i = 0; i < nparams; ++i) {
+    for (i = 0, j = 0; i < nparams; ++i) {
+        if (!(flags & VIR_TYPED_PARAM_STRING_OKAY) &&
+            params[i].type == VIR_TYPED_PARAM_STRING) {
+            --*ret_params_len;
+            continue;
+        }
+
         /* remoteDispatchClientRequest will free this: */
-        val[i].field = strdup (params[i].field);
-        if (val[i].field == NULL) {
+        val[j].field = strdup(params[i].field);
+        if (val[j].field == NULL) {
             virReportOOMError();
             goto cleanup;
         }
-        val[i].value.type = params[i].type;
-        switch (params[i].type) {
+        val[j].value.type = params[i].type;
+        switch (params[j].type) {
         case VIR_TYPED_PARAM_INT:
-            val[i].value.remote_typed_param_value_u.i = params[i].value.i;
+            val[j].value.remote_typed_param_value_u.i = params[i].value.i;
             break;
         case VIR_TYPED_PARAM_UINT:
-            val[i].value.remote_typed_param_value_u.ui = params[i].value.ui;
+            val[j].value.remote_typed_param_value_u.ui = params[i].value.ui;
             break;
         case VIR_TYPED_PARAM_LLONG:
-            val[i].value.remote_typed_param_value_u.l = params[i].value.l;
+            val[j].value.remote_typed_param_value_u.l = params[i].value.l;
             break;
         case VIR_TYPED_PARAM_ULLONG:
-            val[i].value.remote_typed_param_value_u.ul = params[i].value.ul;
+            val[j].value.remote_typed_param_value_u.ul = params[i].value.ul;
             break;
         case VIR_TYPED_PARAM_DOUBLE:
-            val[i].value.remote_typed_param_value_u.d = params[i].value.d;
+            val[j].value.remote_typed_param_value_u.d = params[i].value.d;
             break;
         case VIR_TYPED_PARAM_BOOLEAN:
-            val[i].value.remote_typed_param_value_u.b = params[i].value.b;
+            val[j].value.remote_typed_param_value_u.b = params[i].value.b;
+            break;
+        case VIR_TYPED_PARAM_STRING:
+            val[j].value.remote_typed_param_value_u.s =
+                strdup(params[i].value.s);
+            if (val[j].value.remote_typed_param_value_u.s == NULL) {
+                virReportOOMError();
+                goto cleanup;
+            }
             break;
         default:
             virNetError(VIR_ERR_RPC, _("unknown parameter type: %d"),
                         params[i].type);
             goto cleanup;
         }
+        j++;
     }
 
     *ret_params_val = val;
@@ -739,8 +752,11 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
 
 cleanup:
     if (val) {
-        for (i = 0; i < nparams; i++)
+        for (i = 0; i < nparams; i++) {
             VIR_FREE(val[i].field);
+            if (params[i].type == VIR_TYPED_PARAM_STRING)
+                VIR_FREE(val[i].value.remote_typed_param_value_u.s);
+        }
         VIR_FREE(val);
     }
     return rv;
@@ -753,7 +769,7 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val,
                                  int limit,
                                  int *nparams)
 {
-    int i;
+    int i = 0;
     int rv = -1;
     virTypedParameterPtr params = NULL;
 
@@ -804,6 +820,14 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val,
             params[i].value.b =
                 args_params_val[i].value.remote_typed_param_value_u.b;
             break;
+        case VIR_TYPED_PARAM_STRING:
+            params[i].value.s =
+                strdup(args_params_val[i].value.remote_typed_param_value_u.s);
+            if (params[i].value.s == NULL) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            break;
         default:
             virNetError(VIR_ERR_INTERNAL_ERROR, _("unknown parameter type: %d"),
                         params[i].type);
@@ -814,8 +838,14 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val,
     rv = 0;
 
 cleanup:
-    if (rv < 0)
+    if (rv < 0) {
+        int j;
+        for (j = 0; j < i; ++j) {
+            if (params[j].type == VIR_TYPED_PARAM_STRING)
+                VIR_FREE(params[j].value.s);
+        }
         VIR_FREE(params);
+    }
     return params;
 }
 
@@ -854,7 +884,8 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS
 
     if (remoteSerializeTypedParameters(params, nparams,
                                        &ret->params.params_val,
-                                       &ret->params.params_len) < 0)
+                                       &ret->params.params_len,
+                                       0) < 0)
         goto cleanup;
 
     rv = 0;
@@ -908,7 +939,8 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE
 
     if (remoteSerializeTypedParameters(params, nparams,
                                        &ret->params.params_val,
-                                       &ret->params.params_len) < 0)
+                                       &ret->params.params_len,
+                                       args->flags) < 0)
         goto cleanup;
 
     rv = 0;
@@ -1095,7 +1127,8 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED,
     /* Serialise the block stats. */
     if (remoteSerializeTypedParameters(params, nparams,
                                        &ret->params.params_val,
-                                       &ret->params.params_len) < 0)
+                                       &ret->params.params_len,
+                                       args->flags) < 0)
         goto cleanup;
 
 success:
@@ -1575,7 +1608,8 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
 
     if (remoteSerializeTypedParameters(params, nparams,
                                        &ret->params.params_val,
-                                       &ret->params.params_len) < 0)
+                                       &ret->params.params_len,
+                                       args->flags) < 0)
         goto cleanup;
 
 success:
@@ -1638,7 +1672,8 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
 
     if (remoteSerializeTypedParameters(params, nparams,
                                        &ret->params.params_val,
-                                       &ret->params.params_len) < 0)
+                                       &ret->params.params_len,
+                                       args->flags) < 0)
         goto cleanup;
 
 success:
@@ -1647,6 +1682,7 @@ success:
 cleanup:
     if (rv < 0)
         virNetMessageSaveError(rerr);
+    virTypedParameterArrayClear(params, nparams);
     VIR_FREE(params);
     if (dom)
         virDomainFree(dom);

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index f3b8ad57d0a516884f4a5b1826b7191a98bd153b..94fd3e7a42d8e55e74340285e53fa1b9a33c81a9 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1244,8 +1244,11 @@ remoteFreeTypedParameters(remote_typed_param *args_params_val,
     if (args_params_val == NULL)
         return;
 
-    for (i = 0; i < args_params_len; i++)
+    for (i = 0; i < args_params_len; i++) {
         VIR_FREE(args_params_val[i].field);
+        if (args_params_val[i].value.type == VIR_TYPED_PARAM_STRING)
+            VIR_FREE(args_params_val[i].value.remote_typed_param_value_u.s);
+    }
 
     VIR_FREE(args_params_val);
 }
@@ -1294,6 +1297,13 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
         case VIR_TYPED_PARAM_BOOLEAN:
             val[i].value.remote_typed_param_value_u.b = params[i].value.b;
             break;
+        case VIR_TYPED_PARAM_STRING:
+            val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s);
+            if (val[i].value.remote_typed_param_value_u.s == NULL) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            break;
         default:
             remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"),
                 params[i].type);
@@ -1318,7 +1328,7 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val,
                                  virTypedParameterPtr params,
                                  int *nparams)
 {
-    int i;
+    int i = 0;
     int rv = -1;
 
     /* Check the length of the returned list carefully. */
@@ -1365,6 +1375,14 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val,
             params[i].value.b =
                 ret_params_val[i].value.remote_typed_param_value_u.b;
             break;
+        case VIR_TYPED_PARAM_STRING:
+            params[i].value.s =
+                strdup(ret_params_val[i].value.remote_typed_param_value_u.s);
+            if (params[i].value.s == NULL) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            break;
         default:
             remoteError(VIR_ERR_RPC, _("unknown parameter type: %d"),
                         params[i].type);
@@ -1375,6 +1393,12 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val,
     rv = 0;
 
 cleanup:
+    if (rv < 0) {
+        int j;
+        for (j = 0; j < i; j++)
+            if (params[j].type == VIR_TYPED_PARAM_STRING)
+                VIR_FREE(params[j].value.s);
+    }
     return rv;
 }
 

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index a174af8a5aa6b937256cfee9e853137b9f4fabcc..5ea11140cd29a1d12cb19718aa020c7a4c169e59 100644 (file)
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -317,6 +317,8 @@ union remote_typed_param_value switch (int type) {
      double d;
  case VIR_TYPED_PARAM_BOOLEAN:
      int b;
+ case VIR_TYPED_PARAM_STRING:
+     remote_nonnull_string s;
 };
 
 struct remote_typed_param {

diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 12cedefd6f0fe942bf1246425f6db13a6b49eb4a..b460b773a03db10d1442587c83952a03fb6c3464 100644 (file)
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -6,6 +6,7 @@ enum {
         VIR_TYPED_PARAM_ULLONG = 4,
         VIR_TYPED_PARAM_DOUBLE = 5,
         VIR_TYPED_PARAM_BOOLEAN = 6,
+        VIR_TYPED_PARAM_STRING = 7,
 };
 struct remote_nonnull_domain {
         remote_nonnull_string      name;
@@ -78,6 +79,7 @@ struct remote_typed_param_value {
                 uint64_t           ul;
                 double             d;
                 int                b;
+                remote_nonnull_string s;
         } remote_typed_param_value_u;
 };
 struct remote_typed_param {

diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 56af258ee883fc1ef00c666da8161842f1708f2a..b36ca692922985d7e1990fce56bddce8052d586e 100755 (executable)
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -439,7 +439,8 @@ elsif ($opt_b) {
                                         "                                                   $2,\n" .
                                         "                                                   &n$1)) == NULL)\n" .
                                         "        goto cleanup;\n");
-                    push(@free_list, "    VIR_FREE(params);");
+                    push(@free_list, "    virTypedParameterArrayClear($1, n$1);");
+                    push(@free_list, "    VIR_FREE($1);");
                 } elsif ($args_member =~ m/<\S+>;/ or $args_member =~ m/\[\S+\];/) {
                     # just make all other array types fail
                     die "unhandled type for argument value: $args_member";