Re: [PATCH v2 1/5] hyperv: Functions to work with invocation parameters.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2017-04-19 20:21 GMT+02:00 Sri Ramanujam <sramanujam@xxxxxxxxx>:
> 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 | 246 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/hyperv/hyperv_wmi.h |  78 ++++++++++++++-
>  2 files changed, 323 insertions(+), 1 deletion(-)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index a3c7dc0..c9a7666 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -142,6 +143,251 @@ hypervVerifyResponse(WsManClient *client, WsXmlDocH response,

> +
> +/*
> + * hypervFreeInvokeParams:
> + * @params: Params object to be freed
> + *
> + */
> +void
> +hypervFreeInvokeParams(hypervInvokeParamsListPtr params)
> +{
> +    if (params == NULL)
> +        return;
> +
> +    VIR_DISPOSE_N(params->params, params->nbAvailParams);
> +    VIR_FREE(params);
> +
> +    return;

Unnecessary return statement.

> +}
> +
> +static inline int
> +hypervCheckParams(hypervInvokeParamsListPtr params)
> +{
> +    int result = -1;
> +
> +    if (params->nbParams + 1 > params->nbAvailParams) {
> +        if (VIR_EXPAND_N(params->params, params->nbAvailParams, 5) < 0)
> +            goto cleanup;

Just return -1 here directly.

> +    }
> +
> +    result = 0;
> + cleanup:
> +    return result;
> +}

Just return 0 here, no need for a goto and a label.

> +
> +/*
> + * 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;

Just return -1 here directly.

> +
> +    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;
> +}

Just return 0 here, no need for a goto and a label.

> +
> +/*
> + * hypervAddEprParam:
> + * @params: Params objec to add to

s/objec/object/

> + * @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)
> +{
> +    int result = -1;
> +    hypervParamPtr p = NULL;
> +    hypervWmiClassInfoPtr classInfo = NULL;
> +
> +    if (hypervGetWmiClassInfo(priv, eprInfo, &classInfo) < 0)
> +        goto cleanup;
> +
> +    if (hypervCheckParams(params) < 0)
> +        goto cleanup;

Just merge the two if conditions with an || and return -1 directly.

> +    p = &params->params[params->nbParams];
> +    p->type = HYPERV_EPR_PARAM;
> +    p->epr.name = name;
> +    p->epr.query = query;
> +    p->epr.info = classInfo;
> +    params->nbParams++;
> +
> +    result = 0;
> + cleanup:
> +    return result;
> +}

Just return 0 here, no need for a goto and a label.

> +/*
> + * 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;
> +    char *tmp = 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 err;
> +    typeinfo = classInfo->serializerInfo;

Missing empty line after goto err. Also the label should be named
error, not err.

> +
> +    /* loop through the items to find out how many fields there are */
> +    for (i = 0; typeinfo[i+1].name != NULL; i++) {}
> +
> +    count = i + 1;
> +    table = virHashCreate(count, virHashValueFree);

virHashCreate can fail and return NULL. You need to handle that here.

> +
> +    for (i = 0; typeinfo[i+1].name != NULL; i++) {
> +        item = &typeinfo[i];
> +        if (VIR_STRDUP(tmp, item->name) < 0)
> +            goto err;
> +
> +        if (virHashAddEntry(table, tmp, NULL) < 0)
> +            goto err;
> +
> +        VIR_FREE(tmp);

Why are you creating a temporary copy of the item name? This is not
necessary, just call virHashAddEntry with item->name directly.

> +    }
> +
> +    goto success;

Just return table here and remove the success label. Keep the err
label after the reuen but rename it to error.

> +
> + err:
> +    virHashFree(table);
> + success:
> +    return 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)
> +{
> +    int result = -1;
> +    hypervParamPtr p = NULL;
> +    hypervWmiClassInfoPtr classInfo = NULL;
> +
> +    if (hypervCheckParams(params) < 0)
> +        goto cleanup;
> +
> +    /* Get the typeinfo out of the class info list */
> +    if (hypervGetWmiClassInfo(priv, info, &classInfo) < 0)
> +        goto cleanup;
> +
> +    p = &params->params[params->nbParams];
> +    p->type = HYPERV_EMBEDDED_PARAM;
> +    p->embedded.name = name;
> +    p->embedded.table = table;
> +    p->embedded.info = classInfo;
> +    params->nbParams++;
> +
> +    result = 0;
> +
> + cleanup:
> +    return result;
> +}
> +
> +
>
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>   * Object
> diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
> index edb8efa..c2a0592 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;
> +
> +
> +hypervInvokeParamsListPtr hypervInitInvokeParamsList(hypervPrivate *priv,
> +        const char *method, const char *selector, hypervWmiClassInfoListPtr obj);
> +
> +void hypervFreeInvokeParams(hypervInvokeParamsListPtr params);
> +
> +int hypervAddSimpleParam(hypervInvokeParamsListPtr params, const char *name,
> +        const char *value);
> +
> +int hypervAddEprParam(hypervInvokeParamsListPtr params, const char *name,
> +        hypervPrivate *priv, virBufferPtr query,
> +        hypervWmiClassInfoListPtr eprInfo);
> +
> +virHashTablePtr hypervCreateEmbeddedParam(hypervPrivate *priv,
> +        hypervWmiClassInfoListPtr info);
> +
> +int hypervSetEmbeddedProperty(virHashTablePtr table, const char *name,
> +        char *value);
> +
> +int hypervAddEmbeddedParam(hypervInvokeParamsListPtr params, hypervPrivate *priv,
> +        const char *name, virHashTablePtr table, hypervWmiClassInfoListPtr info);
>
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>   * CIM/Msvm_ReturnCode
> --
> 2.9.3
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux