Re: [libvirt] [PATCH 2/5] API: remote support for VIR_TYPED_PARAM_STRING

From: Eric Blake&lt;eblake(a)redhat.com&gt; 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&lt;eblake(a)redhat.com&gt; --- 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 f3b8ad5..94fd3e7 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";