[libvirt] [PATCHv6 2/5] API: remote support for VIR_TYPED_PARAM_STRING
Stefan Berger
stefanb at linux.vnet.ibm.com
Thu Nov 3 23:31:38 UTC 2011
On 11/03/2011 02:05 PM, Eric Blake wrote:
> 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 at redhat.com>
> ---
>
> v6: address review comments from Stefan, add filtering of string
> params on Get functions
>
> daemon/remote.c | 88 +++++++++++++++++++++++++++++------------
> src/remote/remote_driver.c | 28 ++++++++++++-
> src/remote/remote_protocol.x | 2 +
> src/remote_protocol-structs | 2 +
> src/rpc/gendispatch.pl | 3 +-
> 5 files changed, 94 insertions(+), 29 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index bd0c3e3..aa3f768 100644
> --- 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 ea7fb24..9d689af 100644
> --- 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 a174af8..5ea1114 100644
> --- 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 12cedef..b460b77 100644
> --- 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 56af258..b36ca69 100755
> --- 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";
ACK
More information about the libvir-list
mailing list