[libvirt] [PATCH v2 0/4] Export remote typed params methods internally via util

Erik Skultety (4): util: Introduce virTypedParameterRemote datatype util: Export remoteDeserializeTypedParameters internally via util util: Export remoteFreeTypedParameters internally via util util: Export remoteSerializeTypedParameters internally via util daemon/remote.c | 316 ++++++++++------------------------------- src/libvirt_private.syms | 3 + src/remote/remote_driver.c | 342 +++++++++++---------------------------------- src/rpc/gendispatch.pl | 26 ++-- src/util/virtypedparam.c | 246 ++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 39 ++++++ 6 files changed, 459 insertions(+), 513 deletions(-) -- 2.4.3

Both admin and remote protocols define their own types (remote_typed_param vs admin_typed_param). Because of the naming convention, admin typed params wouldn't be able to reuse the serialization/deserialization methods, which are tailored for use by remote protocol, even if those method were exported properly. In that case, introduce a new internal data type structurally copying both admin and remote protocols which, eventually, would allow serializer and deserializer to be used in a more generic way. --- src/util/virtypedparam.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 9bef204..7dd3a78 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -36,6 +36,30 @@ verify(!(VIR_TYPED_PARAM_LAST & VIR_TYPED_PARAM_MULTIPLE)); +typedef struct _virTypedParameterRemoteValue virTypedParameterRemoteValue; +typedef struct virTypedParameterRemoteValue *virTypedParameterRemoteValuePtr; + +struct _virTypedParameterRemoteValue { + int type; + union { + int i; /* exempt from syntax-check */ + unsigned int ui; + long long int l; + unsigned long long int ul; + double d; + char b; + char *s; + } remote_typed_param_value; +}; + +typedef struct _virTypedParameterRemote *virTypedParameterRemotePtr; + +struct _virTypedParameterRemote { + char *field; + virTypedParameterRemoteValue value; +}; + + int virTypedParamsValidate(virTypedParameterPtr params, int nparams, /* const char *name, int type ... */ ...) ATTRIBUTE_SENTINEL ATTRIBUTE_RETURN_CHECK; -- 2.4.3

Currently, the deserializer is hardcoded into remote_driver which makes it impossible for admin to use it. One way to achieve a shared implementation (besides moving the code to another module) would be pass @ret_params_val as a void pointer as opposed to the remote_typed_param pointer and add a new extra argument specifying which of those two protocols is being used and typecast the pointer at the function entry. An example from remote_protocol: struct remote_typed_param_value { int type; union { int i; u_int ui; int64_t l; uint64_t ul; double d; int b; remote_nonnull_string s; } remote_typed_param_value_u; }; typedef struct remote_typed_param_value remote_typed_param_value; struct remote_typed_param { remote_nonnull_string field; remote_typed_param_value value; }; That would leave us with a bunch of if-then-elses that needed to be used across the method. This patch takes the other approach using the new datatype introduced in one of earlier commits. --- daemon/remote.c | 120 ++++---------------------- src/libvirt_private.syms | 1 + src/remote/remote_driver.c | 209 ++++++++++++--------------------------------- src/rpc/gendispatch.pl | 19 +++-- src/util/virtypedparam.c | 125 +++++++++++++++++++++++++++ src/util/virtypedparam.h | 6 ++ 6 files changed, 213 insertions(+), 267 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 370f442..d655cfd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -101,12 +101,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 virTypedParameterPtr -remoteDeserializeTypedParameters(remote_typed_param *args_params_val, - u_int args_params_len, - int limit, - int *nparams); - static int remoteSerializeTypedParameters(virTypedParameterPtr params, int nparams, @@ -1495,84 +1489,6 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, return rv; } -/* Helper to deserialize typed parameters. */ -static virTypedParameterPtr -remoteDeserializeTypedParameters(remote_typed_param *args_params_val, - u_int args_params_len, - int limit, - int *nparams) -{ - size_t i = 0; - int rv = -1; - virTypedParameterPtr params = NULL; - - /* Check the length of the returned list carefully. */ - if (limit && args_params_len > limit) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); - goto cleanup; - } - if (VIR_ALLOC_N(params, args_params_len) < 0) - goto cleanup; - - *nparams = args_params_len; - - /* Deserialise the result. */ - for (i = 0; i < args_params_len; ++i) { - if (virStrcpyStatic(params[i].field, - args_params_val[i].field) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Parameter %s too big for destination"), - args_params_val[i].field); - goto cleanup; - } - params[i].type = args_params_val[i].value.type; - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - params[i].value.i = - args_params_val[i].value.remote_typed_param_value_u.i; - break; - case VIR_TYPED_PARAM_UINT: - params[i].value.ui = - args_params_val[i].value.remote_typed_param_value_u.ui; - break; - case VIR_TYPED_PARAM_LLONG: - params[i].value.l = - args_params_val[i].value.remote_typed_param_value_u.l; - break; - case VIR_TYPED_PARAM_ULLONG: - params[i].value.ul = - args_params_val[i].value.remote_typed_param_value_u.ul; - break; - case VIR_TYPED_PARAM_DOUBLE: - params[i].value.d = - args_params_val[i].value.remote_typed_param_value_u.d; - break; - case VIR_TYPED_PARAM_BOOLEAN: - params[i].value.b = - args_params_val[i].value.remote_typed_param_value_u.b; - break; - case VIR_TYPED_PARAM_STRING: - if (VIR_STRDUP(params[i].value.s, - args_params_val[i].value.remote_typed_param_value_u.s) < 0) - goto cleanup; - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown parameter type: %d"), - params[i].type); - goto cleanup; - } - } - - rv = 0; - - cleanup: - if (rv < 0) { - virTypedParamsFree(params, i); - params = NULL; - } - return params; -} - static int remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, @@ -5392,9 +5308,9 @@ remoteDispatchDomainMigrateBegin3Params(virNetServerPtr server ATTRIBUTE_UNUSED, if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; if (!(xml = virDomainMigrateBegin3Params(dom, params, nparams, @@ -5445,9 +5361,9 @@ remoteDispatchDomainMigratePrepare3Params(virNetServerPtr server ATTRIBUTE_UNUSE goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; /* Wacky world of XDR ... */ @@ -5506,9 +5422,9 @@ remoteDispatchDomainMigratePrepareTunnel3Params(virNetServerPtr server ATTRIBUTE goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)) || @@ -5579,9 +5495,9 @@ remoteDispatchDomainMigratePerform3Params(virNetServerPtr server ATTRIBUTE_UNUSE if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; dconnuri = args->dconnuri == NULL ? NULL : *args->dconnuri; @@ -5636,9 +5552,9 @@ remoteDispatchDomainMigrateFinish3Params(virNetServerPtr server ATTRIBUTE_UNUSED goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; dom = virDomainMigrateFinish3Params(priv->conn, params, nparams, @@ -5696,9 +5612,9 @@ remoteDispatchDomainMigrateConfirm3Params(virNetServerPtr server ATTRIBUTE_UNUSE if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; if (virDomainMigrateConfirm3Params(dom, params, nparams, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5f4322f..f25c6fa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2350,6 +2350,7 @@ virTypedParameterTypeFromString; virTypedParameterTypeToString; virTypedParamsCheck; virTypedParamsCopy; +virTypedParamsDeserialize; virTypedParamsFilter; virTypedParamsGetStringList; virTypedParamsReplaceString; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d9d7ec8..b4c58e2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -72,11 +72,6 @@ VIR_LOG_INIT("remote.remote_driver"); # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif -#define remoteDeserializeTypedParameters(ret_params_val, ret_params_len, \ - limit, params, nparams) \ - deserializeTypedParameters(__FUNCTION__, ret_params_val, ret_params_len, \ - limit, params, nparams) - static bool inside_daemon; struct private_data { @@ -1747,105 +1742,6 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, return rv; } -/* Helper to deserialize typed parameters. */ -static int -deserializeTypedParameters(const char *funcname, - remote_typed_param *ret_params_val, - u_int ret_params_len, - int limit, - virTypedParameterPtr *params, - int *nparams) -{ - size_t i = 0; - int rv = -1; - bool userAllocated = *params != NULL; - - if (ret_params_len > limit) { - virReportError(VIR_ERR_RPC, - _("%s: too many parameters '%u' for limit '%d'"), - funcname, ret_params_len, limit); - goto cleanup; - } - - if (userAllocated) { - /* Check the length of the returned list carefully. */ - if (ret_params_len > *nparams) { - virReportError(VIR_ERR_RPC, - _("%s: too many parameters '%u' for nparams '%d'"), - funcname, ret_params_len, *nparams); - goto cleanup; - } - } else { - if (VIR_ALLOC_N(*params, ret_params_len) < 0) - goto cleanup; - } - *nparams = ret_params_len; - - /* Deserialise the result. */ - for (i = 0; i < ret_params_len; ++i) { - virTypedParameterPtr param = *params + i; - remote_typed_param *ret_param = ret_params_val + i; - - if (virStrcpyStatic(param->field, - ret_param->field) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s: parameter %s too big for destination"), - funcname, ret_param->field); - goto cleanup; - } - - param->type = ret_param->value.type; - switch (param->type) { - case VIR_TYPED_PARAM_INT: - param->value.i = - ret_param->value.remote_typed_param_value_u.i; - break; - case VIR_TYPED_PARAM_UINT: - param->value.ui = - ret_param->value.remote_typed_param_value_u.ui; - break; - case VIR_TYPED_PARAM_LLONG: - param->value.l = - ret_param->value.remote_typed_param_value_u.l; - break; - case VIR_TYPED_PARAM_ULLONG: - param->value.ul = - ret_param->value.remote_typed_param_value_u.ul; - break; - case VIR_TYPED_PARAM_DOUBLE: - param->value.d = - ret_param->value.remote_typed_param_value_u.d; - break; - case VIR_TYPED_PARAM_BOOLEAN: - param->value.b = - ret_param->value.remote_typed_param_value_u.b; - break; - case VIR_TYPED_PARAM_STRING: - if (VIR_STRDUP(param->value.s, - ret_param->value.remote_typed_param_value_u.s) < 0) - goto cleanup; - break; - default: - virReportError(VIR_ERR_RPC, _("%s: unknown parameter type: %d"), - funcname, param->type); - goto cleanup; - } - } - - rv = 0; - - cleanup: - if (rv < 0) { - if (userAllocated) { - virTypedParamsClear(*params, i); - } else { - virTypedParamsFree(*params, i); - *params = NULL; - } - } - return rv; -} - static int remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, u_int ret_errors_len, @@ -1922,12 +1818,12 @@ remoteDomainBlockStatsFlags(virDomainPtr domain, *nparams = ret.params.params_len; - /* Deserialise the result. */ - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + /* Deserialize the result. */ + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -1971,11 +1867,11 @@ remoteDomainGetMemoryParameters(virDomainPtr domain, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -2019,11 +1915,11 @@ remoteDomainGetNumaParameters(virDomainPtr domain, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_NUMA_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_NUMA_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -2067,11 +1963,11 @@ remoteDomainGetBlkioParameters(virDomainPtr domain, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -2991,11 +2887,11 @@ static int remoteDomainGetBlockIoTune(virDomainPtr domain, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -3081,9 +2977,10 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr cpu_params = ¶ms[cpu * nparams]; remote_typed_param *stride = &ret.params.params_val[cpu * ret.nparams]; - if (remoteDeserializeTypedParameters(stride, ret.nparams, - REMOTE_NODE_CPU_STATS_MAX, - &cpu_params, &tmp) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) stride, + ret.nparams, + REMOTE_NODE_CPU_STATS_MAX, + &cpu_params, &tmp) < 0) goto cleanup; } @@ -5480,10 +5377,10 @@ remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUS int nparams = 0; virObjectEventPtr event = NULL; - if (remoteDeserializeTypedParameters(msg->params.params_val, - msg->params.params_len, - REMOTE_DOMAIN_EVENT_TUNABLE_MAX, - ¶ms, &nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) msg->params.params_val, + msg->params.params_len, + REMOTE_DOMAIN_EVENT_TUNABLE_MAX, + ¶ms, &nparams) < 0) return; dom = get_nonnull_domain(conn, msg->dom); @@ -6720,11 +6617,11 @@ remoteDomainGetInterfaceParameters(virDomainPtr domain, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -6900,11 +6797,11 @@ remoteNodeGetMemoryParameters(virConnectPtr conn, goto cleanup; } - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_NODE_MEMORY_PARAMETERS_MAX, - ¶ms, - nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_NODE_MEMORY_PARAMETERS_MAX, + ¶ms, + nparams) < 0) goto cleanup; rv = 0; @@ -7021,10 +6918,10 @@ remoteDomainGetJobStats(virDomainPtr domain, *type = ret.type; - if (remoteDeserializeTypedParameters(ret.params.params_val, - ret.params.params_len, - REMOTE_DOMAIN_JOB_STATS_MAX, - params, nparams) < 0) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_JOB_STATS_MAX, + params, nparams) < 0) goto cleanup; rv = 0; @@ -7802,11 +7699,11 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, if (!(elem->dom = get_nonnull_domain(conn, rec->dom))) goto cleanup; - if (remoteDeserializeTypedParameters(rec->params.params_val, - rec->params.params_len, - REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX, - &elem->params, - &elem->nparams)) + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) rec->params.params_val, + rec->params.params_len, + REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX, + &elem->params, + &elem->nparams)) goto cleanup; tmpret[i] = elem; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 3740130..d7f42b1 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -557,10 +557,11 @@ elsif ($mode eq "server") { } push(@args_list, "$1"); push(@args_list, "n$1"); - push(@getters_list, " if (($1 = remoteDeserializeTypedParameters(args->$1.$1_val,\n" . - " args->$1.$1_len,\n" . - " $2,\n" . - " &n$1)) == NULL)\n" . + push(@getters_list, " if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->$1.$1_val,\n" . + " args->$1.$1_len,\n" . + " $2,\n" . + " &$1,\n" . + " &n$1) < 0)\n" . " goto cleanup;\n"); push(@free_list, " virTypedParamsFree($1, n$1);"); } elsif ($args_member =~ m/<\S+>;/ or $args_member =~ m/\[\S+\];/) { @@ -1356,11 +1357,11 @@ elsif ($mode eq "client") { } } elsif ($ret_member =~ m/^remote_typed_param (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { splice(@args_list, int($3), 0, ("virTypedParameterPtr $1")); - push(@ret_list2, "if (remoteDeserializeTypedParameters(ret.$1.$1_val,\n" . - " ret.$1.$1_len,\n" . - " $2,\n" . - " &$1,\n" . - " n$1) < 0)\n" . + push(@ret_list2, "if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.$1.$1_val,\n" . + " ret.$1.$1_len,\n" . + " $2,\n" . + " &$1,\n" . + " n$1) < 0)\n" . " goto cleanup;\n"); $single_ret_cleanup = 1; } elsif ($ret_member =~ m/^remote_typed_param (\S+)<\S+>;/) { diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f3ce157..f1e7cf2 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1315,3 +1315,128 @@ virTypedParamsFree(virTypedParameterPtr params, virTypedParamsClear(params, nparams); VIR_FREE(params); } + + +/** + * virTypedParamsDeserialize: + * @remote_params: protocol data to be deserialized (obtained from remote side) + * @remote_params_len: number of parameters returned in @remote_params + * @limit: user specified maximum limit to @remote_params_len + * @params: pointer which will hold the deserialized @remote_params data + * @nparams: number of entries in @params + * + * This function will attempt to deserialize protocol-encoded data obtained + * from remote side. Two modes of operation are supported, depending on the + * caller's design: + * 1) Older APIs do not rely on deserializer allocating memory for @params, + * thus calling the deserializer twice, once to find out the actual number of + * parameters for @params to hold, followed by an allocation of @params and + * a second call to the deserializer to actually retrieve the data. + * 2) Newer APIs rely completely on the deserializer to allocate the right + * ammount of memory for @params to hold all the data obtained in + * @remote_params. + * + * If used with model 1, two checks are performed, first one comparing the user + * specified limit to the actual size of remote data and the second one + * verifying the user allocated amount of memory is indeed capable of holding + * remote data @remote_params. + * With model 2, only the first check against @limit is performed. + * + * Returns 0 on success or -1 in case of an error. + */ +int +virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, + unsigned int remote_params_len, + int limit, + virTypedParameterPtr *params, + int *nparams) +{ + size_t i = 0; + int rv = -1; + bool userAllocated = *params != NULL; + + if (limit && remote_params_len > limit) { + virReportError(VIR_ERR_RPC, + _("too many parameters '%u' for limit '%d'"), + remote_params_len, limit); + goto cleanup; + } + + if (userAllocated) { + /* Check the length of the returned list carefully. */ + if (remote_params_len > *nparams) { + virReportError(VIR_ERR_RPC, + _("too many parameters '%u' for nparams '%d'"), + remote_params_len, *nparams); + goto cleanup; + } + } else { + if (VIR_ALLOC_N(*params, remote_params_len) < 0) + goto cleanup; + } + *nparams = remote_params_len; + + /* Deserialize the result. */ + for (i = 0; i < remote_params_len; ++i) { + virTypedParameterPtr param = *params + i; + virTypedParameterRemotePtr remote_param = remote_params + i; + + if (virStrcpyStatic(param->field, + remote_param->field) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("parameter %s too big for destination"), + remote_param->field); + goto cleanup; + } + + param->type = remote_param->value.type; + switch (param->type) { + case VIR_TYPED_PARAM_INT: + param->value.i = + remote_param->value.remote_typed_param_value.i; + break; + case VIR_TYPED_PARAM_UINT: + param->value.ui = + remote_param->value.remote_typed_param_value.ui; + break; + case VIR_TYPED_PARAM_LLONG: + param->value.l = + remote_param->value.remote_typed_param_value.l; + break; + case VIR_TYPED_PARAM_ULLONG: + param->value.ul = + remote_param->value.remote_typed_param_value.ul; + break; + case VIR_TYPED_PARAM_DOUBLE: + param->value.d = + remote_param->value.remote_typed_param_value.d; + break; + case VIR_TYPED_PARAM_BOOLEAN: + param->value.b = + remote_param->value.remote_typed_param_value.b; + break; + case VIR_TYPED_PARAM_STRING: + if (VIR_STRDUP(param->value.s, + remote_param->value.remote_typed_param_value.s) < 0) + goto cleanup; + break; + default: + virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), + param->type); + goto cleanup; + } + } + + rv = 0; + + cleanup: + if (rv < 0) { + if (userAllocated) { + virTypedParamsClear(*params, i); + } else { + virTypedParamsFree(*params, i); + *params = NULL; + } + } + return rv; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 7dd3a78..98bf3aa 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -104,6 +104,12 @@ int virTypedParamsCopy(virTypedParameterPtr *dst, char *virTypedParameterToString(virTypedParameterPtr param); +int virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, + unsigned int remote_params_len, + int limit, + virTypedParameterPtr *params, + int *nparams); + VIR_ENUM_DECL(virTypedParameter) # define VIR_TYPED_PARAMS_DEBUG(params, nparams) \ -- 2.4.3

Since the method is static to remote_driver, it can't even be used by our daemon. Other than that, it would be useful to be able to use it with admin as well. This patch uses the new virTypedParameterRemote datatype introduced in one of previous patches. --- src/libvirt_private.syms | 1 + src/remote/remote_driver.c | 39 +++++++++++++-------------------------- src/rpc/gendispatch.pl | 3 ++- src/util/virtypedparam.c | 29 +++++++++++++++++++++++++++++ src/util/virtypedparam.h | 3 +++ 5 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f25c6fa..690e957 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2353,6 +2353,7 @@ virTypedParamsCopy; virTypedParamsDeserialize; virTypedParamsFilter; virTypedParamsGetStringList; +virTypedParamsRemoteFree; virTypedParamsReplaceString; virTypedParamsValidate; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b4c58e2..4d569c9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1663,25 +1663,6 @@ remoteConnectListAllDomains(virConnectPtr conn, return rv; } -/* Helper to free typed parameters. */ -static void -remoteFreeTypedParameters(remote_typed_param *args_params_val, - u_int args_params_len) -{ - size_t i; - - if (args_params_val == NULL) - return; - - 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); -} - /* Helper to serialize typed parameters. */ static int remoteSerializeTypedParameters(virTypedParameterPtr params, @@ -1738,7 +1719,7 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, rv = 0; cleanup: - remoteFreeTypedParameters(val, nparams); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) val, nparams); return rv; } @@ -6991,7 +6972,8 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain, rv = ret.xml; /* caller frees */ cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); remoteDriverUnlock(priv); return rv; @@ -7069,7 +7051,8 @@ remoteDomainMigratePrepare3Params(virConnectPtr dconn, rv = 0; cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); VIR_FREE(ret.uri_out); remoteDriverUnlock(priv); return rv; @@ -7159,7 +7142,8 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn, rv = 0; cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); remoteDriverUnlock(priv); return rv; @@ -7231,7 +7215,8 @@ remoteDomainMigratePerform3Params(virDomainPtr dom, rv = 0; cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); remoteDriverUnlock(priv); return rv; @@ -7307,7 +7292,8 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn, (char *) &ret); cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); remoteDriverUnlock(priv); return rv; @@ -7363,7 +7349,8 @@ remoteDomainMigrateConfirm3Params(virDomainPtr domain, rv = 0; cleanup: - remoteFreeTypedParameters(args.params.params_val, args.params.params_len); + virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val, + args.params.params_len); remoteDriverUnlock(priv); return rv; } diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index d7f42b1..9d64e63 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1217,7 +1217,8 @@ elsif ($mode eq "client") { " xdr_free((xdrproc_t)xdr_$call->{args}, (char *)&args);\n" . " goto done;\n" . " }"); - push(@free_list, " remoteFreeTypedParameters(args.params.params_val, args.params.params_len);\n"); + push(@free_list, " virTypedParamsRemoteFree((virTypedParameterRemotePtr) args.params.params_val,\n" . + " args.params.params_len);\n"); } elsif ($args_member =~ m/^((?:unsigned )?int) (\S+);\s*\/\*\s*call-by-reference\s*\*\//) { my $type_name = "$1 *"; my $arg_name = $2; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f1e7cf2..1bc7865 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1316,6 +1316,35 @@ virTypedParamsFree(virTypedParameterPtr params, VIR_FREE(params); } +/** + * virTypedParamsRemoteFree: + * @remote_params_val: array of typed parameters as specified by + * (remote|admin)_protocol.h + * @remote_params_len: number of parameters in @remote_params_val + * + * Frees memory used by string representations of parameter identificators, + * memory used by string values of parameters and the memory occupied by + * @remote_params_val itself. + * + * Returns nothing. + */ +void +virTypedParamsRemoteFree(virTypedParameterRemotePtr remote_params_val, + unsigned int remote_params_len) +{ + size_t i; + + if (!remote_params_val) + return; + + for (i = 0; i < remote_params_len; i++) { + VIR_FREE(remote_params_val[i].field); + if (remote_params_val[i].value.type == VIR_TYPED_PARAM_STRING) + VIR_FREE(remote_params_val[i].value.remote_typed_param_value.s); + } + VIR_FREE(remote_params_val); +} + /** * virTypedParamsDeserialize: diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 98bf3aa..df3547f 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -104,6 +104,9 @@ int virTypedParamsCopy(virTypedParameterPtr *dst, char *virTypedParameterToString(virTypedParameterPtr param); +void virTypedParamsRemoteFree(virTypedParameterRemotePtr remote_params_val, + unsigned int remote_params_len); + int virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, unsigned int remote_params_len, int limit, -- 2.4.3

Same as for deserializer, this method might get handy for admin one day. The major reason for this patch is to stay consistent with idea, i.e. when deserializer can be shared, why not serializer as well. The only problem to be solved was that the daemon side serializer uses a code snippet which handles sparse arrays returned by some APIs as well as removes any string parameters that can't be returned to older clients. This patch makes of the new virTypedParameterRemote datatype introduced by one of the pvious patches. --- daemon/remote.c | 196 +++++++++++++-------------------------------- src/libvirt_private.syms | 1 + src/remote/remote_driver.c | 96 +++++----------------- src/rpc/gendispatch.pl | 4 +- src/util/virtypedparam.c | 92 +++++++++++++++++++++ src/util/virtypedparam.h | 6 ++ 6 files changed, 175 insertions(+), 220 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d655cfd..ca692a9 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -102,13 +102,6 @@ static void make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterP 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, - unsigned int flags); - -static int remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors, int nerrors, remote_domain_disk_error **ret_errors_val, @@ -989,10 +982,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn, data.callbackID = callback->callbackID; make_nonnull_domain(&data.dom, dom); - if (remoteSerializeTypedParameters(params, nparams, - &data.params.params_val, - &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &data.params.params_val, + &data.params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) return -1; remoteDispatchObjectEventSend(callback->client, remoteProgram, @@ -1410,85 +1403,6 @@ remoteDispatchDomainGetSchedulerType(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } -/* 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, - unsigned int flags) -{ - size_t i; - size_t j; - int rv = -1; - remote_typed_param *val; - - *ret_params_len = nparams; - if (VIR_ALLOC_N(val, nparams) < 0) - goto cleanup; - - for (i = 0, j = 0; i < nparams; ++i) { - /* virDomainGetCPUStats can return a sparse array; also, we - * can't pass back strings to older clients. */ - if (!params[i].type || - (!(flags & VIR_TYPED_PARAM_STRING_OKAY) && - params[i].type == VIR_TYPED_PARAM_STRING)) { - --*ret_params_len; - continue; - } - - /* remoteDispatchClientRequest will free this: */ - if (VIR_STRDUP(val[j].field, params[i].field) < 0) - goto cleanup; - val[j].value.type = params[i].type; - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - val[j].value.remote_typed_param_value_u.i = params[i].value.i; - break; - case VIR_TYPED_PARAM_UINT: - val[j].value.remote_typed_param_value_u.ui = params[i].value.ui; - break; - case VIR_TYPED_PARAM_LLONG: - val[j].value.remote_typed_param_value_u.l = params[i].value.l; - break; - case VIR_TYPED_PARAM_ULLONG: - val[j].value.remote_typed_param_value_u.ul = params[i].value.ul; - break; - case VIR_TYPED_PARAM_DOUBLE: - val[j].value.remote_typed_param_value_u.d = params[i].value.d; - break; - case VIR_TYPED_PARAM_BOOLEAN: - val[j].value.remote_typed_param_value_u.b = params[i].value.b; - break; - case VIR_TYPED_PARAM_STRING: - if (VIR_STRDUP(val[j].value.remote_typed_param_value_u.s, params[i].value.s) < 0) - goto cleanup; - break; - default: - virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), - params[i].type); - goto cleanup; - } - j++; - } - - *ret_params_val = val; - val = NULL; - rv = 0; - - cleanup: - if (val) { - for (i = 0; i < nparams; i++) { - VIR_FREE(val[i].field); - if (val[i].value.type == VIR_TYPED_PARAM_STRING) - VIR_FREE(val[i].value.remote_typed_param_value_u.s); - } - VIR_FREE(val); - } - return rv; -} - static int remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, @@ -1523,10 +1437,10 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS if (virDomainGetSchedulerParameters(dom, params, &nparams) < 0) goto cleanup; - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - 0) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + 0) < 0) goto cleanup; rv = 0; @@ -1632,10 +1546,10 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE args->flags) < 0) goto cleanup; - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; rv = 0; @@ -1805,11 +1719,11 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - /* Serialise the block stats. */ - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + /* Serialize the block stats. */ + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -2472,10 +2386,10 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -2534,10 +2448,10 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + flags) < 0) goto cleanup; success: @@ -2596,10 +2510,10 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -2841,11 +2755,11 @@ remoteDispatchDomainGetBlockIoTune(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - /* Serialise the block I/O tuning parameters. */ - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + /* Serialize the block I/O tuning parameters. */ + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -4381,10 +4295,10 @@ remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUS goto success; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + flags) < 0) goto cleanup; success: @@ -4443,10 +4357,10 @@ remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED, if (args->nparams == 0) goto success; - if (remoteSerializeTypedParameters(params, args->nparams * args->ncpus, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + if (virTypedParamsSerialize(params, args->nparams * args->ncpus, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -5112,10 +5026,10 @@ remoteDispatchNodeGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED, goto success; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - args->flags) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) goto cleanup; success: @@ -5259,10 +5173,10 @@ remoteDispatchDomainGetJobStats(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - if (remoteSerializeTypedParameters(params, nparams, - &ret->params.params_val, - &ret->params.params_len, - 0) < 0) + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + 0) < 0) goto cleanup; rv = 0; @@ -6305,11 +6219,11 @@ remoteDispatchConnectGetAllDomainStats(virNetServerPtr server ATTRIBUTE_UNUSED, make_nonnull_domain(&dst->dom, retStats[i]->dom); - if (remoteSerializeTypedParameters(retStats[i]->params, - retStats[i]->nparams, - &dst->params.params_val, - &dst->params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + if (virTypedParamsSerialize(retStats[i]->params, + retStats[i]->nparams, + (virTypedParameterRemotePtr *) &dst->params.params_val, + &dst->params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) goto cleanup; } } else { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 690e957..69be352 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2355,6 +2355,7 @@ virTypedParamsFilter; virTypedParamsGetStringList; virTypedParamsRemoteFree; virTypedParamsReplaceString; +virTypedParamsSerialize; virTypedParamsValidate; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 4d569c9..58787cd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1663,66 +1663,6 @@ remoteConnectListAllDomains(virConnectPtr conn, return rv; } -/* Helper to serialize typed parameters. */ -static int -remoteSerializeTypedParameters(virTypedParameterPtr params, - int nparams, - remote_typed_param **args_params_val, - u_int *args_params_len) -{ - size_t i; - int rv = -1; - remote_typed_param *val; - - *args_params_len = nparams; - if (VIR_ALLOC_N(val, nparams) < 0) - goto cleanup; - - for (i = 0; i < nparams; ++i) { - /* call() will free this: */ - if (VIR_STRDUP(val[i].field, params[i].field) < 0) - goto cleanup; - val[i].value.type = params[i].type; - switch (params[i].type) { - case VIR_TYPED_PARAM_INT: - val[i].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; - break; - case VIR_TYPED_PARAM_LLONG: - val[i].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; - break; - case VIR_TYPED_PARAM_DOUBLE: - val[i].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; - break; - case VIR_TYPED_PARAM_STRING: - if (VIR_STRDUP(val[i].value.remote_typed_param_value_u.s, - params[i].value.s) < 0) - goto cleanup; - break; - default: - virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), - params[i].type); - goto cleanup; - } - } - - *args_params_val = val; - val = NULL; - rv = 0; - - cleanup: - virTypedParamsRemoteFree((virTypedParameterRemotePtr) val, nparams); - return rv; -} - static int remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, u_int ret_errors_len, @@ -6944,9 +6884,9 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain, goto cleanup; } - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_begin3_params_args, (char *) &args); goto cleanup; @@ -7011,9 +6951,9 @@ remoteDomainMigratePrepare3Params(virConnectPtr dconn, goto cleanup; } - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_prepare3_params_args, (char *) &args); goto cleanup; @@ -7098,9 +7038,9 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn, args.cookie_in.cookie_in_len = cookieinlen; args.flags = flags; - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_params_args, (char *) &args); goto cleanup; @@ -7187,9 +7127,9 @@ remoteDomainMigratePerform3Params(virDomainPtr dom, args.cookie_in.cookie_in_len = cookieinlen; args.flags = flags; - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_perform3_params_args, (char *) &args); goto cleanup; @@ -7259,9 +7199,9 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn, args.flags = flags; args.cancelled = cancelled; - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_finish3_params_args, (char *) &args); goto cleanup; @@ -7333,9 +7273,9 @@ remoteDomainMigrateConfirm3Params(virDomainPtr domain, args.flags = flags; args.cancelled = cancelled; - if (remoteSerializeTypedParameters(params, nparams, - &args.params.params_val, - &args.params.params_len) < 0) { + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &args.params.params_val, + &args.params.params_len, 0) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_confirm3_params_args, (char *) &args); goto cleanup; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 9d64e63..658efae 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1213,7 +1213,9 @@ elsif ($mode eq "client") { } elsif ($args_member =~ m/^remote_typed_param (\S+)<(\S+)>;/) { push(@args_list, "virTypedParameterPtr $1"); push(@args_list, "int n$1"); - push(@setters_list2, "if (remoteSerializeTypedParameters($1, n$1, &args.$1.$1_val, &args.$1.$1_len) < 0) {\n" . + push(@setters_list2, "if (virTypedParamsSerialize($1, n$1,\n" . + " (virTypedParameterRemotePtr *) &args.$1.$1_val,\n" . + " &args.$1.$1_len, 0) < 0) {\n" . " xdr_free((xdrproc_t)xdr_$call->{args}, (char *)&args);\n" . " goto done;\n" . " }"); diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 1bc7865..138fc64 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1469,3 +1469,95 @@ virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, } return rv; } + + +/** + * virTypedParamsSerialize: + * @params: array of parameters to be serialized and later sent to remote side + * @nparams: number of elements in @params + * @remote_params_val: protocol independent remote representation of @params + * @remote_params_len: the final number of elements in @remote_params_val + * @flags: bitwise-OR of virTypedParameterFlags + * + * This method serializes typed parameters provided by @params into + * @remote_params_val which is the representation actually being sent. + * + * Server side using this method also filters out any string parameters that + * must not be returned to older clients and handles possibly sparse arrays + * returned by some APIs. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsSerialize(virTypedParameterPtr params, + int nparams, + virTypedParameterRemotePtr *remote_params_val, + unsigned int *remote_params_len, + unsigned int flags) +{ + size_t i; + size_t j; + int rv = -1; + virTypedParameterRemotePtr params_val; + + *remote_params_len = nparams; + if (VIR_ALLOC_N(params_val, nparams) < 0) + goto cleanup; + + for (i = 0, j = 0; i < nparams; ++i) { + virTypedParameterPtr param = params + i; + virTypedParameterRemotePtr val = params_val + j; + /* NOTE: Following snippet is relevant to server only, because + * virDomainGetCPUStats can return a sparse array; also, we can't pass + * back strings to older clients. */ + if (!param->type || + (!(flags & VIR_TYPED_PARAM_STRING_OKAY) && + param->type == VIR_TYPED_PARAM_STRING)) { + --*remote_params_len; + continue; + } + + /* This will be either freed by virNetServerDispatchCall or call(), + * depending on the calling side, i.e. server or client */ + if (VIR_STRDUP(val->field, param->field) < 0) + goto cleanup; + val->value.type = param->type; + switch (param->type) { + case VIR_TYPED_PARAM_INT: + val->value.remote_typed_param_value.i = param->value.i; + break; + case VIR_TYPED_PARAM_UINT: + val->value.remote_typed_param_value.ui = param->value.ui; + break; + case VIR_TYPED_PARAM_LLONG: + val->value.remote_typed_param_value.l = param->value.l; + break; + case VIR_TYPED_PARAM_ULLONG: + val->value.remote_typed_param_value.ul = param->value.ul; + break; + case VIR_TYPED_PARAM_DOUBLE: + val->value.remote_typed_param_value.d = param->value.d; + break; + case VIR_TYPED_PARAM_BOOLEAN: + val->value.remote_typed_param_value.b = param->value.b; + break; + case VIR_TYPED_PARAM_STRING: + if (VIR_STRDUP(val->value.remote_typed_param_value.s, param->value.s) < 0) + goto cleanup; + break; + default: + virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), + param->type); + goto cleanup; + } + j++; + } + + *remote_params_val = params_val; + params_val = NULL; + rv = 0; + + cleanup: + virTypedParamsRemoteFree(params_val, nparams); + return rv; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index df3547f..73f710a 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -113,6 +113,12 @@ int virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, virTypedParameterPtr *params, int *nparams); +int virTypedParamsSerialize(virTypedParameterPtr params, + int nparams, + virTypedParameterRemotePtr *remote_params_val, + unsigned int *remote_params_len, + unsigned int flags); + VIR_ENUM_DECL(virTypedParameter) # define VIR_TYPED_PARAMS_DEBUG(params, nparams) \ -- 2.4.3

On Wed, Feb 03, 2016 at 03:51:51PM +0100, Erik Skultety wrote:
Erik Skultety (4): util: Introduce virTypedParameterRemote datatype util: Export remoteDeserializeTypedParameters internally via util util: Export remoteFreeTypedParameters internally via util util: Export remoteSerializeTypedParameters internally via util
daemon/remote.c | 316 ++++++++++------------------------------- src/libvirt_private.syms | 3 + src/remote/remote_driver.c | 342 +++++++++++---------------------------------- src/rpc/gendispatch.pl | 26 ++-- src/util/virtypedparam.c | 246 ++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 39 ++++++ 6 files changed, 459 insertions(+), 513 deletions(-)
ACK series. Jan

On 03/02/16 16:15, Ján Tomko wrote:
On Wed, Feb 03, 2016 at 03:51:51PM +0100, Erik Skultety wrote:
Erik Skultety (4): util: Introduce virTypedParameterRemote datatype util: Export remoteDeserializeTypedParameters internally via util util: Export remoteFreeTypedParameters internally via util util: Export remoteSerializeTypedParameters internally via util
daemon/remote.c | 316 ++++++++++------------------------------- src/libvirt_private.syms | 3 + src/remote/remote_driver.c | 342 +++++++++++---------------------------------- src/rpc/gendispatch.pl | 26 ++-- src/util/virtypedparam.c | 246 ++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 39 ++++++ 6 files changed, 459 insertions(+), 513 deletions(-)
ACK series.
Thanks for review, pushed now. Erik
Jan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Erik Skultety
-
Ján Tomko