[libvirt] [PATCHv5 2/3] API: remote support for VIR_TYPED_PARAM_STRING

Stefan Berger stefanb at linux.vnet.ibm.com
Wed Nov 2 10:56:03 UTC 2011


On 11/01/2011 07:47 PM, Eric Blake wrote:
> Send and receive string typed parameters across RPC.
>
> * src/remote/remote_protocol.x (remote_typed_param_value): Add
> another union value.
> * daemon/remote.c (remoteSerializeTypedParameters)
> (remoteDeserializeTypedParameters): Handle strings on rpc.
> * src/remote/remote_driver.c (remoteFreeTypedParameters)
> (remoteSerializeTypedParameters)
> (remoteDeserializeTypedParameters): Likewise.
> * 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>
> ---
>   daemon/remote.c              |   33 +++++++++++++++++++++++++++++----
>   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, 61 insertions(+), 7 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index bd0c3e3..e3a9775 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -701,7 +701,7 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
>
>       for (i = 0; i<  nparams; ++i) {
>           /* remoteDispatchClientRequest will free this: */
> -        val[i].field = strdup (params[i].field);
> +        val[i].field = strdup(params[i].field);
>           if (val[i].field == NULL) {
>               virReportOOMError();
>               goto cleanup;
> @@ -726,6 +726,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:
>               virNetError(VIR_ERR_RPC, _("unknown parameter type: %d"),
>                           params[i].type);
> @@ -739,8 +746,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 +763,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 +814,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 +832,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;
>   }
>
> @@ -1647,6 +1671,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..2182c2c 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[i].type == VIR_TYPED_PARAM_STRING)
> +                VIR_FREE(params[i].value.s);
Index has to be be '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";
Rest looks good to me. With nit fixed ACK.




More information about the libvir-list mailing list