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 | 125 +++++++++----------------------------------- src/libvirt_private.syms | 1 + src/remote/remote_driver.c | 107 ++------------------------------------ src/rpc/gendispatch.pl | 9 ++-- src/util/virtypedparam.c | 127 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 7 +++ 6 files changed, 169 insertions(+), 207 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 370f442..f76a176 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -101,11 +101,12 @@ 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); +#define remoteDeserializeTypedParameters(remote_params_val, \ + remote_params_len, \ + limit, params, nparams) \ + virTypedParamsDeserialize(__FUNCTION__, \ + (virTypedParameterRemotePtr) remote_params_val, \ + remote_params_len, limit, params, nparams) static int remoteSerializeTypedParameters(virTypedParameterPtr params, @@ -1495,84 +1496,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 +5315,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 (remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; if (!(xml = virDomainMigrateBegin3Params(dom, params, nparams, @@ -5445,9 +5368,9 @@ remoteDispatchDomainMigratePrepare3Params(virNetServerPtr server ATTRIBUTE_UNUSE goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; /* Wacky world of XDR ... */ @@ -5506,9 +5429,9 @@ remoteDispatchDomainMigratePrepareTunnel3Params(virNetServerPtr server ATTRIBUTE goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)) || @@ -5579,9 +5502,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 (remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; dconnuri = args->dconnuri == NULL ? NULL : *args->dconnuri; @@ -5636,9 +5559,9 @@ remoteDispatchDomainMigrateFinish3Params(virNetServerPtr server ATTRIBUTE_UNUSED goto cleanup; } - if (!(params = remoteDeserializeTypedParameters(args->params.params_val, - args->params.params_len, - 0, &nparams))) + if (remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + 0, ¶ms, &nparams) < 0) goto cleanup; dom = virDomainMigrateFinish3Params(priv->conn, params, nparams, @@ -5696,9 +5619,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 (remoteDeserializeTypedParameters(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 3d0ec9d..cbd71fa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2349,6 +2349,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..d712005 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -72,10 +72,12 @@ VIR_LOG_INIT("remote.remote_driver"); # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif -#define remoteDeserializeTypedParameters(ret_params_val, ret_params_len, \ +#define remoteDeserializeTypedParameters(remote_params_val, \ + remote_params_len, \ limit, params, nparams) \ - deserializeTypedParameters(__FUNCTION__, ret_params_val, ret_params_len, \ - limit, params, nparams) + virTypedParamsDeserialize(__FUNCTION__, \ + (virTypedParameterRemotePtr) remote_params_val, \ + remote_params_len, limit, params, nparams) static bool inside_daemon; @@ -1747,105 +1749,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, diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 3740130..3ee4267 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 (remoteDeserializeTypedParameters(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+\];/) { diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f3ce157..649d032 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1315,3 +1315,130 @@ virTypedParamsFree(virTypedParameterPtr params, virTypedParamsClear(params, nparams); VIR_FREE(params); } + + +/** + * virTypedParamsDeserialize: + * @funcname: caller's funcname + * @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(const char *funcname, + 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, + _("%s: too many parameters '%u' for limit '%d'"), + funcname, 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, + _("%s: too many parameters '%u' for nparams '%d'"), + funcname, remote_params_len, *nparams); + goto cleanup; + } + } else { + if (VIR_ALLOC_N(*params, remote_params_len) < 0) + goto cleanup; + } + *nparams = remote_params_len; + + /* Deserialise 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, + _("%s: parameter %s too big for destination"), + funcname, 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, _("%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; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 7dd3a78..88b6241 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -104,6 +104,13 @@ int virTypedParamsCopy(virTypedParameterPtr *dst, char *virTypedParameterToString(virTypedParameterPtr param); +int virTypedParamsDeserialize(const char *funcname, + 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list