[libvirt] [PATCH v4 1/5] hyperv: Functions to work with invocation parameters.

[Matthias Bolte]

2017-05-19 22:57 GMT+02:00 Sri Ramanujam <sramanujam at datto.com>:
> This commit introduces functionality for creating and working with
> invoke parameters. This commit does not include any code for serializing
> and actually performing the method invocations; it merely defines the
> functions and API for using invocation parameters in driver code.
>
> HYPERV_DEFAULT_PARAM_COUNT was chosen because almost no method
> invocations have more than 4 parameters.
>
> Functions added:
> * hypervInitInvokeParamsList
> * hypervFreeInvokeParams
> * hypervAddSimpleParam
> * hypervAddEprParam
> * hypervCreateEmbeddedParam
> * hypervSetEmbeddedProperty
> * hypervAddEmbeddedParam
> ---
>  src/hyperv/hyperv_wmi.c | 252 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/hyperv/hyperv_wmi.h |  78 ++++++++++++++-
>  2 files changed, 329 insertions(+), 1 deletion(-)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index a3c7dc0..217a3b2 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -2,6 +2,7 @@
>   * hyperv_wmi.c: general WMI over WSMAN related functions and structures for
>   *               managing Microsoft Hyper-V hosts
>   *
> + * Copyright (C) 2017 Datto Inc
>   * Copyright (C) 2014 Red Hat, Inc.
>   * Copyright (C) 2011 Matthias Bolte <matthias.bolte at googlemail.com>
>   * Copyright (C) 2009 Michael Sievers <msievers83 at googlemail.com>
> @@ -142,6 +143,257 @@ hypervVerifyResponse(WsManClient *client, WsXmlDocH response,
>  }
>
>
> +/*
> + * Methods to work with method invocation parameters
> + */
> +
> +/*
> + * hypervCreateInvokeParamsList:
> + * @priv: hypervPrivate object associated with the connection.
> + * @method: The name of the method you are calling
> + * @selector: The selector for the object you are invoking the method on
> + * @obj: The WmiInfo of the object class you are invoking the method on.
> + *
> + * Create a new InvokeParamsList object for the method call.
> + *
> + * Returns a pointer to the newly instantiated object on success, which should
> + * be freed by hypervInvokeMethod. Otherwise returns NULL.
> + */
> +hypervInvokeParamsListPtr
> +hypervCreateInvokeParamsList(hypervPrivate *priv, const char *method,
> +        const char *selector, hypervWmiClassInfoListPtr obj)
> +{
> +    hypervInvokeParamsListPtr params = NULL;
> +    hypervWmiClassInfoPtr info = NULL;
> +
> +    if (hypervGetWmiClassInfo(priv, obj, &info) < 0)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC(params) < 0)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC_N(params->params,
> +                HYPERV_DEFAULT_PARAM_COUNT) < 0) {
> +        VIR_FREE(params);
> +        goto cleanup;
> +    }
> +
> +    params->method = method;
> +    params->ns = info->rootUri;
> +    params->resourceUri = info->resourceUri;
> +    params->selector = selector;
> +    params->nbParams = 0;
> +    params->nbAvailParams = HYPERV_DEFAULT_PARAM_COUNT;
> +
> + cleanup:
> +    return params;
> +}
> +
> +/*
> + * hypervFreeInvokeParams:
> + * @params: Params object to be freed
> + *
> + */
> +void
> +hypervFreeInvokeParams(hypervInvokeParamsListPtr params)
> +{
> +    hypervParamPtr p = NULL;
> +    size_t i = 0;
> +
> +    if (params == NULL)
> +        return;
> +
> +    for (i = 0; i < params->nbParams; i++) {
> +        p = &(params->params[i]);
> +
> +        switch (p->type) {
> +            case HYPERV_SIMPLE_PARAM:
> +                break;
> +            case HYPERV_EPR_PARAM:
> +                virBufferFreeAndReset(p->epr.query);
> +                break;
> +            case HYPERV_EMBEDDED_PARAM:
> +                virHashFree(p->embedded.table);
> +                break;
> +            default:
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("Invalid parameter type passed to free"));
> +        }
> +    }
> +
> +    VIR_DISPOSE_N(params->params, params->nbAvailParams);
> +    VIR_FREE(params);
> +}
> +
> +static inline int
> +hypervCheckParams(hypervInvokeParamsListPtr params)
> +{
> +    if (params->nbParams + 1 > params->nbAvailParams) {
> +        if (VIR_EXPAND_N(params->params, params->nbAvailParams, 5) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * hypervAddSimpleParam:
> + * @params: Params object to add to
> + * @name: Name of the parameter
> + * @value: Value of the parameter
> + *
> + * Add a param of type HYPERV_SIMPLE_PARAM, which is essentially a serialized
> + * key/value pair.
> + *
> + * Returns -1 on failure, 0 on success.
> + */
> +int
> +hypervAddSimpleParam(hypervInvokeParamsListPtr params, const char *name,
> +        const char *value)
> +{
> +    int result = -1;
> +    hypervParamPtr p = NULL;
> +
> +    if (hypervCheckParams(params) < 0)
> +        goto cleanup;
> +
> +    p = &params->params[params->nbParams];
> +    p->type = HYPERV_SIMPLE_PARAM;
> +
> +    p->simple.name = name;
> +    p->simple.value = value;
> +
> +    params->nbParams++;
> +
> +    result = 0;
> +
> + cleanup:
> +    return result;
> +}
> +
> +/*
> + * hypervAddEprParam:
> + * @params: Params object to add to
> + * @name: Parameter name
> + * @priv: hypervPrivate object associated with the connection
> + * @query: WQL filter
> + * @eprInfo: WmiInfo of the object being filtered
> + *
> + * Adds an EPR param to the params list. Returns -1 on failure, 0 on success.
> + */
> +int
> +hypervAddEprParam(hypervInvokeParamsListPtr params, const char *name,
> +        hypervPrivate *priv, virBufferPtr query,
> +        hypervWmiClassInfoListPtr eprInfo)
> +{
> +    hypervParamPtr p = NULL;
> +    hypervWmiClassInfoPtr classInfo = NULL;
> +
> +    if (hypervGetWmiClassInfo(priv, eprInfo, &classInfo) < 0 ||
> +            hypervCheckParams(params) < 0)
> +        return -1;
> +
> +    p = &params->params[params->nbParams];
> +    p->type = HYPERV_EPR_PARAM;
> +    p->epr.name = name;
> +    p->epr.query = query;
> +    p->epr.info = classInfo;
> +    params->nbParams++;
> +
> +    return 0;
> +}
> +
> +/*
> + * hypervCreateEmbeddedParam:
> + * @priv: hypervPrivate object associated with the connection
> + * @info: WmiInfo of the object type to serialize
> + *
> + * Instantiates a virHashTable pre-filled with all the properties pre-added
> + * a key/value pairs set to NULL. The user then sets only those properties that
> + * they wish to serialize, and passes the table via hypervAddEmbeddedParam.
> + *
> + * Returns a pointer to the virHashTable on success, otherwise NULL.
> + */
> +virHashTablePtr
> +hypervCreateEmbeddedParam(hypervPrivate *priv, hypervWmiClassInfoListPtr info)
> +{
> +    size_t i;
> +    int count = 0;
> +    virHashTablePtr table = NULL;
> +    XmlSerializerInfo *typeinfo = NULL;
> +    XmlSerializerInfo *item = NULL;
> +    hypervWmiClassInfoPtr classInfo = NULL;
> +
> +    /* Get the typeinfo out of the class info list */
> +    if (hypervGetWmiClassInfo(priv, info, &classInfo) < 0)
> +        goto error;
> +
> +    typeinfo = classInfo->serializerInfo;
> +
> +    /* loop through the items to find out how many fields there are */
> +    for (i = 0; typeinfo[i+1].name != NULL; i++) {}
> +
> +    count = i + 1;

Even if this code is correct, I'd change it like this

for (i = 0; typeinfo[i].name != NULL; i++) {}

count = i;

Because in the next for loop this i+1 pattern is wrong and results in a bug.

> +    table = virHashCreate(count, NULL);
> +    if (table == NULL)
> +        goto error;
> +
> +    for (i = 0; typeinfo[i+1].name != NULL; i++) {
> +        item = &typeinfo[i];

Because you use i+1 here this loop misses the last item in the
typeinfo. This needs to be

for (i = 0; typeinfo[i].name != NULL; i++) {

to also catch the last item.

> +
> +        if (virHashAddEntry(table, item->name, NULL) < 0)
> +            goto error;

If this goto is taken then ...

> +    }
> +
> +    return table;
> +
> + error:
> +    virHashFree(table);
> +    return table;

... this will not return NULL as it should. You need to return NULL
here, not table.

> +}
> +
> +int
> +hypervSetEmbeddedProperty(virHashTablePtr table, const char *name, char *value)
> +{
> +    return virHashUpdateEntry(table, name, value);
> +}
> +
> +/*
> + * hypervAddEmbeddedParam:
> + * @params: Params list to add to
> + * @priv: hypervPrivate object associated with the connection
> + * @name: Name of the parameter
> + * @table: table of properties to add
> + * @info: WmiInfo of the object to serialize
> + *
> + * Add a virHashTable containing object properties as an embedded param to
> + * an invocation list. Returns -1 on failure, 0 on success.
> + */
> +int
> +hypervAddEmbeddedParam(hypervInvokeParamsListPtr params, hypervPrivate *priv,
> +        const char *name, virHashTablePtr table, hypervWmiClassInfoListPtr info)
> +{
> +    hypervParamPtr p = NULL;
> +    hypervWmiClassInfoPtr classInfo = NULL;
> +
> +    if (hypervCheckParams(params) < 0)
> +        return -1;
> +
> +    /* Get the typeinfo out of the class info list */
> +    if (hypervGetWmiClassInfo(priv, info, &classInfo) < 0)
> +        return -1;
> +
> +    p = &params->params[params->nbParams];
> +    p->type = HYPERV_EMBEDDED_PARAM;
> +    p->embedded.name = name;
> +    p->embedded.table = table;
> +    p->embedded.info = classInfo;
> +    params->nbParams++;
> +
> +    return 0;
> +}
> +
> +
>
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>   * Object
> diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
> index edb8efa..2db5bb7 100644
> --- a/src/hyperv/hyperv_wmi.h
> +++ b/src/hyperv/hyperv_wmi.h
> @@ -28,11 +28,13 @@
>  # include "hyperv_private.h"
>  # include "hyperv_wmi_classes.h"
>  # include "openwsman.h"
> -
> +# include "virhash.h"
>
>
>  # define HYPERV_WQL_QUERY_INITIALIZER { NULL, NULL }
>
> +# define HYPERV_DEFAULT_PARAM_COUNT 5
> +
>  int hypervVerifyResponse(WsManClient *client, WsXmlDocH response,
>                           const char *detail);
>
> @@ -74,6 +76,80 @@ int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery,
>  void hypervFreeObject(hypervPrivate *priv, hypervObject *object);
>
>
> +/*
> + * Invoke
> + */
> +
> +typedef enum {
> +    HYPERV_SIMPLE_PARAM,
> +    HYPERV_EPR_PARAM,
> +    HYPERV_EMBEDDED_PARAM
> +} hypervStorageType;
> +
> +struct _hypervSimpleParam {
> +    const char *name;
> +    const char *value;
> +};
> +typedef struct _hypervSimpleParam hypervSimpleParam;
> +
> +struct _hypervEprParam {
> +    const char *name;
> +    virBufferPtr query;
> +    hypervWmiClassInfoPtr info; // info of the object this param represents
> +};
> +typedef struct _hypervEprParam hypervEprParam;
> +
> +struct _hypervEmbeddedParam {
> +    const char *name;
> +    virHashTablePtr table;
> +    hypervWmiClassInfoPtr info; // info of the object this param represents
> +};
> +typedef struct _hypervEmbeddedParam hypervEmbeddedParam;
> +
> +struct _hypervParam {
> +    hypervStorageType type;
> +    union {
> +        hypervSimpleParam simple;
> +        hypervEprParam epr;
> +        hypervEmbeddedParam embedded;
> +    };
> +};
> +typedef struct _hypervParam hypervParam;
> +typedef hypervParam *hypervParamPtr;
> +
> +struct _hypervInvokeParamsList {
> +    const char *method;
> +    const char *ns;
> +    const char *resourceUri;
> +    const char *selector;
> +    hypervParamPtr params;
> +    size_t nbParams;
> +    size_t nbAvailParams;
> +};
> +typedef struct _hypervInvokeParamsList hypervInvokeParamsList;
> +typedef hypervInvokeParamsList *hypervInvokeParamsListPtr;


-- 
Matthias Bolte
http://photron.blogspot.com