Re: [PATCH 07/16] hyperv: implement ability to send xml soap requests

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

 




On 08/09/2016 08:39 AM, Jason Miesionczek wrote:
> also added ability to get/set auto start
> ---
>  src/hyperv/hyperv_driver.c | 101 +++++++
>  src/hyperv/hyperv_wmi.c    | 670 ++++++++++++++++++++++++++++++++++++++++++++-
>  src/hyperv/hyperv_wmi.h    |  58 ++++
>  src/hyperv/openwsman.h     |   4 +
>  4 files changed, 827 insertions(+), 6 deletions(-)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 861d5ab..aea7837 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1604,6 +1604,105 @@ hypervNodeGetFreeMemory(virConnectPtr conn)
>      return res;
>  }
>  
> +static int
> +hypervDomainSetAutostart(virDomainPtr domain, int autostart)
> +{
> +    int result = -1;
> +    invokeXmlParam *params = NULL;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    virBuffer queryVssd = VIR_BUFFER_INITIALIZER;
> +    Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> +    properties_t *tab_props = NULL;
> +    eprParam eprparam;
> +    embeddedParam embeddedparam;
> +    int nb_params;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService";

This should be a #define somewhere I think

> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    /* Prepare EPR param */
> +    virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> +    virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string);
> +    eprparam.query = &query;
> +    eprparam.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> +    /* Prepare EMBEDDED param */
> +    virBufferAsprintf(&queryVssd,
> +                      "associators of "
> +                      "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> +                      "Name=\"%s\"} "
> +                      "where AssocClass = Msvm_SettingsDefineState "
> +                      "ResultClass = Msvm_VirtualSystemSettingData",
> +                      uuid_string);

Again seems like some sort of #define for all the syntax

> +
> +    if (hypervGetMsvmVirtualSystemSettingDataList(priv, &queryVssd, &virtualSystemSettingData) < 0)

Long line - try 80 chars

> +        goto cleanup;
> +
> +    embeddedparam.nbProps = 2;
> +    if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> +        goto cleanup;
> +    (*tab_props).name = "AutomaticStartupAction";
> +    (*tab_props).val = autostart ? "2" : "0";
> +    (*(tab_props+1)).name = "InstanceID";
> +    (*(tab_props+1)).val = virtualSystemSettingData->data->InstanceID;

Odd construct (*(tab_props+1))

why not tab_props[0]->name = "..." and tab_props[1]->name = "..."

Those names could be constants too.

> +
> +    embeddedparam.instanceName =  "Msvm_VirtualSystemGlobalSettingData";
> +    embeddedparam.prop_t = tab_props;
> +
> +    /* Create invokeXmlParam tab */
> +    nb_params = 2;
> +    if (VIR_ALLOC_N(params, nb_params) < 0)
> +        goto cleanup;
> +    (*params).name = "ComputerSystem";
> +    (*params).type = EPR_PARAM;
> +    (*params).param = &eprparam;
> +    (*(params+1)).name = "SystemSettingData";
> +    (*(params+1)).type = EMBEDDED_PARAM;
> +    (*(params+1)).param = &embeddedparam;

More of the same with (*params)

> +
> +    result = hypervInvokeMethod(priv, params, nb_params, "ModifyVirtualSystem",
> +                             MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector);

long line - put selector); on it's own line

> +
> + cleanup:
> +    hypervFreeObject(priv, (hypervObject *) virtualSystemSettingData);
> +    VIR_FREE(tab_props);
> +    VIR_FREE(params);
> +    virBufferFreeAndReset(&query);
> +    virBufferFreeAndReset(&queryVssd);
> +
> +    return result;
> +}
> +
> +
> +
> +static int
> +hypervDomainGetAutostart(virDomainPtr domain, int *autostart)
> +{
> +    int result = -1;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    hypervPrivate *priv = domain->conn->privateData;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    Msvm_VirtualSystemGlobalSettingData *vsgsd = NULL;
> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +    virBufferAddLit(&query, MSVM_VIRTUALSYSTEMGLOBALSETTINGDATA_WQL_SELECT);
> +    virBufferAsprintf(&query, "where SystemName = \"%s\"", uuid_string);
> +
> +    if (hypervGetMsvmVirtualSystemGlobalSettingDataList(priv, &query, &vsgsd) < 0)

Long line

> +        goto cleanup;
> +
> +    *autostart = vsgsd->data->AutomaticStartupAction;
> +    result = 0;
> +
> + cleanup:
> +    hypervFreeObject(priv, (hypervObject *) vsgsd);
> +    virBufferFreeAndReset(&query);
> +
> +    return result;
> +}
> +
>  static virHypervisorDriver hypervHypervisorDriver = {
>      .name = "Hyper-V",
>      .connectOpen = hypervConnectOpen, /* 0.9.5 */
> @@ -1645,6 +1744,8 @@ static virHypervisorDriver hypervHypervisorDriver = {
>      .domainGetMaxVcpus = hypervDomainGetMaxVcpus, /* 1.2.10 */
>      .domainGetVcpusFlags = hypervDomainGetVcpusFlags, /* 1.2.10 */
>      .domainGetVcpus = hypervDomainGetVcpus, /* 1.2.10 */
> +    .domainSetAutostart = hypervDomainSetAutostart, /* 1.2.10 */
> +    .domainGetAutostart = hypervDomainGetAutostart, /* 1.2.10 */

2.3.0 at the earliest

>  };
>  

FWIW: It "feels" as if the hunks above should be separate from the hunks
below. I believe above depends on below and below could be any patch
before above.

Although I assume this is what Matthias was referring to in his comments
from patch 2. If that's the case, then this could be closer to patch 2
if it's kept.

>  /* Retrieves host system UUID  */
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index 13acd09..51d9b43 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include <config.h>
> +#include <wsman-soap.h>  /* Where struct _WsXmlDoc is defined (necessary to dereference WsXmlDocH type) */
>  
>  #include "internal.h"
>  #include "virerror.h"
> @@ -33,15 +34,10 @@
>  #include "hyperv_private.h"
>  #include "hyperv_wmi.h"
>  #include "virstring.h"
> +#include "hyperv_wmi_cimtypes.generated.h"
>  
>  #define WS_SERIALIZER_FREE_MEM_WORKS 0
>  
> -#define ROOT_CIMV2 \
> -    "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/cimv2/*";
> -
> -#define ROOT_VIRTUALIZATION \
> -    "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/*";
> -
>  #define VIR_FROM_THIS VIR_FROM_HYPERV
>  
>  
> @@ -667,6 +663,668 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain,
>      return 0;
>  }
>  
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * hypervInvokeMethod
> + *   Function to invoke WSMAN request with simple, EPR or embedded parameters
> + */
> +
> +/* Create XML structure */
> +static int
> +hypervCreateXmlStruct(const char *methodName, const char *classURI,
> +                      WsXmlDocH *xmlDocRoot, WsXmlNodeH *xmlNodeMethod)
> +{
> +    virBuffer method_buff = VIR_BUFFER_INITIALIZER;
> +    char *methodNameInput = NULL;
> +
> +    virBufferAsprintf(&method_buff, "%s_INPUT", methodName);
> +    methodNameInput = virBufferContentAndReset(&method_buff);
> +
> +    if (methodNameInput == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not create Xml Doc"));

Overkill on the extra lines... Could put the "%s", on the same line as
ReportError

> +        goto cleanup;
> +    }
> +
> +    *xmlDocRoot = ws_xml_create_doc(NULL, methodNameInput);
> +    if (*xmlDocRoot == NULL) {

Could go with "if (!(*xmlDocRoot = ws_xml_create_doc(...))) {"

> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not create Xml Doc with given parameter xmlDocRoot"));

Likewise

> +        goto cleanup;
> +    }
> +
> +    *xmlNodeMethod = xml_parser_get_root(*xmlDocRoot);
> +    if (*xmlNodeMethod == NULL) {

Similar if construct

> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not get xmlDocRoot root node"));

Likewise

> +        goto cleanup;
> +    }
> +
> +    /* Add namespace to xmlNodeMethode */
> +    ws_xml_set_ns(*xmlNodeMethod, classURI, "p");
> +
> +    VIR_FREE(methodNameInput);
> +    return 0;
> +
> + cleanup:
> +
> +    virBufferFreeAndReset(&method_buff);
> +    VIR_FREE(methodNameInput);
> +    if (*xmlDocRoot != NULL) {
> +        ws_xml_destroy_doc(*xmlDocRoot);
> +        *xmlDocRoot = NULL;
> +    }
> +
> +    return -1;
> +}
> +
> +
> +/* Look for the type of a given property class and specifies if it is an array */
> +static int
> +hypervGetPropType(const char *className, const char *propName, const char **propType, bool *isArray)

Long line - split args over multiple lines

> +{
> +    int i, y;

Use 'size_t' not 'int' for loop variables

Typically we use loop variables of i, j, k...  I think taking this
method tricks the checker though.

> +
> +    i = 0;
> +    while (cimClasses[i].name[0] != '\0') {

So I guess we're guaranteed this is a never ending loop by some other
means?  My question being - how is cimClasses constructed such that we
know 'i' won't go beyond the number of elements in the array?

BTW: Check out STREQ_NULLABLE

> +        if (STREQ(cimClasses[i].name, className)){

s/)){/)) {/  - syntax-check

> +            y = 0;
> +            while (cimClasses[i].cimTypesPtr[y].name[0] != '\0') {
> +                if (STREQ(cimClasses[i].cimTypesPtr[y].name, propName)){

s/)){/)) {/  - syntax-check

Similar comment to above regarding how do we know 'y' will never go
beyond the bounds such that cimTypesPtr[y] isn't valid?

And of course STREQ_NULLABLE applies here as well

> +                    *propType = cimClasses[i].cimTypesPtr[y].type;
> +                    *isArray = cimClasses[i].cimTypesPtr[y].isArray;
> +                    return 0;
> +                }
> +                y++;
> +            }
> +            break;

break?  so we never get to i++ once we find a matching className?, but
if we don't find a matching className we can go for a while can't we...

> +        }
> +        i++;
> +    }
> +
> +    return -1;
> +}
> +
> +
> +/* Adding an Simple param node to a parent node given in parameter */
> +static int
> +hypervAddSimpleParam(const char *paramName, const char* value,
> +                     const char *classURI, WsXmlNodeH *parentNode)
> +{
> +    int result = -1;
> +    WsXmlNodeH xmlNodeParam = NULL;
> +
> +    xmlNodeParam = ws_xml_add_child(*parentNode, classURI, paramName, value);
> +    if (xmlNodeParam == NULL) {

Consider "if (!(xmlNodeParam = ws*(...))) {"

It's a long line, but should be split across 2 in any case.

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not create simple param"));
> +        goto cleanup;

Just return -1

> +    }
> +
> +    result = 0;

just return 0;

> +
> + cleanup:
> +    return result;

Removing pointless cleanup since there is no cleanup

> +}
> +
> +
> +/* Adding EPR param node to a parent node given in parameter */
> +static int
> +hypervAddEprParam(const char *paramName, virBufferPtr query, const char *root,
> +                  const char *classURI, WsXmlNodeH *parentNode, WsXmlDocH doc, hypervPrivate *priv)
> +{
> +
> +    int result = -1;
> +    WsXmlNodeH xmlNodeParam = NULL;
> +    WsXmlNodeH xmlNodTemp = NULL;
> +    WsXmlNodeH xmlNodeAdr = NULL;
> +    WsXmlNodeH xmlNodeRef = NULL;
> +    xmlNodePtr xmlNodeAdrPtr = NULL;
> +    xmlNodePtr xmlNodeRefPtr = NULL;
> +    WsXmlDocH xmlDocResponse = NULL;
> +    xmlDocPtr docPtr = (xmlDocPtr) doc->parserDoc;
> +    WsXmlNsH ns = NULL;
> +    client_opt_t *options = NULL;
> +    filter_t *filter = NULL;
> +    char *enumContext = NULL;
> +    char *query_string = NULL;
> +
> +    /* Request options and filter */
> +    options = wsmc_options_init();
> +
> +    if (options == NULL) {

if (!(options = wsmc*())) {

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not initialize options"));
> +        goto cleanup;
> +    }
> +
> +    wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR);
> +
> +    query_string = virBufferContentAndReset(query);
> +    filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string);
> +    if (filter == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not create filter"));
> +        goto cleanup;
> +    }
> +
> +    /* Invoke enumerate action*/
> +    xmlDocResponse = wsmc_action_enumerate(priv->client,root, options, filter);

s/client,root/client, root/   (from syntax-check)

> +
> +    /* Check return value */
> +    if (hyperyVerifyResponse(priv->client, xmlDocResponse, "enumeration") < 0) {
> +        goto cleanup;
> +    }

Brackets - syntax-check

> +
> +    /* Get enumerate conext*/
> +    enumContext = wsmc_get_enum_context(xmlDocResponse);
> +
> +    ws_xml_destroy_doc(xmlDocResponse);
> +

One less empty line

> +
> +    /* Invoke pull action*/
> +    xmlDocResponse = wsmc_action_pull(priv->client, classURI, options, filter, enumContext);
> +
> +    /* Check return value */
> +    if (hyperyVerifyResponse(priv->client, xmlDocResponse, "pull") < 0) {
> +        goto cleanup;
> +    }

brackets - syntax-check

> +
> +    /* Extract EPR nodes childs */
> +    xmlNodTemp = ws_xml_get_soap_body(xmlDocResponse);
> +    if (xmlNodTemp == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup SOAP body"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodTemp = ws_xml_get_child(xmlNodTemp, 0, XML_NS_ENUMERATION, WSENUM_PULL_RESP);
> +    if (xmlNodTemp == NULL) {

Long line above, consider combining if stmt.

Repeats numerous times below


> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup pull response"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodTemp = ws_xml_get_child(xmlNodTemp, 0, XML_NS_ENUMERATION, WSENUM_ITEMS);
> +    if (xmlNodTemp == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup pull response items"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodTemp = ws_xml_get_child(xmlNodTemp, 0, XML_NS_ADDRESSING, WSA_EPR);
> +    if (xmlNodTemp == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup pull response item EPR"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodeAdr = ws_xml_get_child(xmlNodTemp, 0, XML_NS_ADDRESSING, WSA_ADDRESS);
> +    if (xmlNodeAdr == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup pull response item ADDRESS"));
> +        goto cleanup;
> +    }
> +    xmlNodeAdrPtr = xmlDocCopyNode((xmlNodePtr) xmlNodeAdr, docPtr, 1);
> +    if (xmlNodeAdrPtr == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not copy item ADDRESS"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodeRef = ws_xml_get_child(xmlNodTemp, 0, XML_NS_ADDRESSING, WSA_REFERENCE_PARAMETERS);
> +    if (xmlNodeRef == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not lookup pull response item REFERENCE PARAMETERS"));
> +        goto cleanup;
> +    }
> +    xmlNodeRefPtr = xmlDocCopyNode((xmlNodePtr) xmlNodeRef, docPtr, 1);
> +    if (xmlNodeRefPtr == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not copy item REFERENCE PARAMETERS"));
> +        goto cleanup;
> +    }
> +
> +    /* Build XmlDoc with adding previous EPR nodes childs */
> +    xmlNodeParam = ws_xml_add_child(*parentNode, classURI, paramName, NULL);
> +    if (xmlNodeParam == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not add child node to xmlNodeParam"));
> +        goto cleanup;
> +    }
> +
> +/*
> +  The folowing line has been commented because of a memory corruption issue reported in the openwsman library
> +  [ issue #43 - xml_parser_ns_add: alloc item size, not pointer size ]
> +  xmlNodeSetLang((xmlNodePtr) xmlNodeParam, BAD_CAST "en-US");
> +*/

Comments should be :

/*
 * text line 1
 * text line 2
 */

and they should be in line with the indention as well as be less the 80
chars on the line

> +    ns = ws_xml_ns_add(xmlNodeParam, "http://schemas.xmlsoap.org/ws/2004/08/addressing";, "a");
> +    if (ns == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not set namespace adressing to xmlNodeParam"));
> +        goto cleanup;
> +    }
> +
> +    ns = NULL;
> +    ns = ws_xml_ns_add(xmlNodeParam, "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd";, "w");
> +    if (ns == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not set namespace wsman to xmlNodeParam"));
> +        goto cleanup;
> +    }
> +
> +    if (xmlAddChild((xmlNodePtr) *parentNode,(xmlNodePtr) xmlNodeParam) == NULL) {

s/Node,(/Node, (/  - syntax-check

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not add child to xml parent node"));
> +        goto cleanup;
> +    }
> +
> +    if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeAdrPtr) == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not add child to xml parent node"));
> +        goto cleanup;
> +    }
> +
> +    if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeRefPtr) == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not add child to xml parent node"));
> +        goto cleanup;
> +    }
> +
> +    result = 0;
> +
> + cleanup:
> +    if (options != NULL) {
> +        wsmc_options_destroy(options);
> +    }
> +    if (filter != NULL) {
> +        filter_destroy(filter);
> +    }

Both - brackets - syntax-check

> +    ws_xml_destroy_doc(xmlDocResponse);
> +    VIR_FREE(enumContext);
> +    VIR_FREE(query_string);
> +
> +    return result;
> +}
> +
> +
> +/* Adding an Embedded Instance node to a parent node given in parameter */
> +static int
> +hypervAddEmbeddedParam(properties_t *prop_t, int nbProps, const char *paramName,
> +                       const char *instanceName, const char *classURI, WsXmlNodeH *parentNode)
> +{
> +
> +    int result = -1;
> +    WsXmlNodeH xmlNodeInstance = NULL;
> +    WsXmlNodeH xmlNodeProperty = NULL;
> +    WsXmlNodeH xmlNodeParam = NULL;
> +    WsXmlNodeH xmlNodeArray = NULL;
> +    WsXmlDocH xmlDocTemp = NULL;
> +    WsXmlDocH xmlDocCdata = NULL;
> +    xmlBufferPtr xmlBufferNode = NULL;
> +    const xmlChar *xmlCharCdataContent = NULL;
> +    xmlNodePtr xmlNodeCdata = NULL;
> +    const char *type = NULL;
> +    bool isArray = false;
> +    int len = 0;
> +    int	i = 0;
> +
> +    /* Add child to given parent node*/
> +    xmlNodeParam = ws_xml_add_child(*parentNode, classURI, paramName, NULL);
> +    if (xmlNodeParam == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not add child node to xmlNodeParam"));
> +        goto cleanup;
> +    }
> +
> +    /* Create temp Xml doc */
> +    /* INSTANCE node */
> +    xmlDocTemp = ws_xml_create_doc(NULL, "INSTANCE");
> +    if (xmlDocTemp == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not create temporary Xml doc"));
> +        goto cleanup;
> +    }
> +
> +    xmlNodeInstance = xml_parser_get_root(xmlDocTemp);
> +    if (xmlNodeInstance == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not get root of temporary Xml doc"));
> +        goto cleanup;
> +    }
> +
> +    /* Add CLASSNAME node to INSTANCE node */
> +    if (ws_xml_add_node_attr(xmlNodeInstance, NULL, "CLASSNAME", instanceName) == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not add attribute to node "));
> +        goto cleanup;
> +    }
> +
> +    /* Property nodes */
> +    while (i < nbProps) {
> +
> +        if (prop_t[i].name == NULL && prop_t[i].val == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not get properties from array"));
> +            goto cleanup;
> +        }
> +
> +        if (hypervGetPropType(instanceName, prop_t[i].name, &type, &isArray) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not get properties from array"));
> +            goto cleanup;
> +        }
> +
> +        xmlNodeProperty = ws_xml_add_child(xmlNodeInstance, NULL,
> +                                           isArray ? "PROPERTY.ARRAY" : "PROPERTY", NULL);
> +        if (xmlNodeProperty == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not add child to node"));
> +            goto cleanup;
> +        }
> +
> +        if (ws_xml_add_node_attr(xmlNodeProperty, NULL, "NAME", prop_t[i].name) == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not add attribute to node"));
> +            goto cleanup;
> +        }
> +
> +        if (ws_xml_add_node_attr(xmlNodeProperty, NULL, "TYPE", type) == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not add attribute to node"));
> +            goto cleanup;
> +        }
> +
> +        /* Add the node VALUE.ARRAY if the attribute is an array */
> +        if (isArray) {
> +            xmlNodeArray = ws_xml_add_child(xmlNodeProperty, NULL, "VALUE.ARRAY", NULL);
> +            if (xmlNodeArray == NULL) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               "%s",
> +                               _("Could not add child to node"));
> +                goto cleanup;
> +            }
> +        }
> +
> +        if (ws_xml_add_child(isArray ? xmlNodeArray : xmlNodeProperty, NULL, "VALUE", prop_t[i].val) == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s",
> +                           _("Could not add child to node"));
> +            goto cleanup;
> +        }
> +
> +        xmlNodeArray = NULL;
> +        xmlNodeProperty = NULL;
> +        i++;
> +    }
> +
> +    /* Create CDATA node */
> +    xmlBufferNode = xmlBufferCreate();
> +    if (xmlNodeDump(xmlBufferNode, (xmlDocPtr) xmlDocTemp->parserDoc, (xmlNodePtr) xmlNodeInstance, 0, 0) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not get root of temporary Xml doc"));
> +        goto cleanup;
> +    }
> +
> +    len = xmlBufferLength(xmlBufferNode);
> +    xmlCharCdataContent = xmlBufferContent(xmlBufferNode);
> +    xmlNodeCdata = xmlNewCDataBlock((xmlDocPtr) xmlDocCdata, xmlCharCdataContent, len);
> +    if (xmlNodeCdata == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not get root of temporary Xml doc"));
> +        goto cleanup;
> +    }
> +    /* Add CDATA node child to the root node of the main doc given */
> +    if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeCdata) == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not get root of temporary Xml doc"));
> +        goto cleanup;
> +    }
> +
> +    result = 0;
> +
> + cleanup:
> +    ws_xml_destroy_doc(xmlDocCdata);
> +    ws_xml_destroy_doc(xmlDocTemp);
> +    if (xmlBufferNode != NULL)
> +        xmlBufferFree(xmlBufferNode);

Syntax-check says "if (xmlBufferNode != NULL)" is unecessary

> +
> +    return result;
> +}
> +
> +
> +/* Call wsmc_action_invoke() function of OpenWsman API with XML tree given in parameters*/
> +static int
> +hypervInvokeMethodXml(hypervPrivate *priv, WsXmlDocH xmlDocRoot,
> +                      const char *methodName, const char *ressourceURI, const char *selector)
> +{
> +    int result = -1;
> +    int returnCode;
> +    char *instanceID = NULL;
> +    char *xpath_expr_string = NULL;
> +    char *returnValue = NULL;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    virBuffer xpath_expr_buff = VIR_BUFFER_INITIALIZER;
> +    client_opt_t *options = NULL;
> +    WsXmlDocH response = NULL;
> +    Msvm_ConcreteJob *concreteJob = NULL;
> +    bool completed = false;
> +
> +    options = wsmc_options_init();
> +
> +    if (options == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Could not initialize options"));
> +        goto cleanup;
> +    }
> +
> +    wsmc_add_selectors_from_str(options, selector);
> +
> +    /* Invoke action */
> +    response = wsmc_action_invoke(priv->client, ressourceURI, options, methodName, xmlDocRoot);
> +
> +    virBufferAsprintf(&xpath_expr_buff, "/s:Envelope/s:Body/p:%s_OUTPUT/p:ReturnValue", methodName);
> +    xpath_expr_string = virBufferContentAndReset(&xpath_expr_buff);
> +
> +    if (xpath_expr_string == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not lookup %s for %s invocation"),
> +                       "ReturnValue", "RequestStateChange");
> +        goto cleanup;
> +    }
> +
> +    /* Check return value */
> +    returnValue = ws_xml_get_xpath_value(response, xpath_expr_string);
> +
> +    VIR_FREE(xpath_expr_string);
> +    xpath_expr_string = NULL;
> +
> +    if (returnValue == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not lookup %s for %s invocation"),
> +                       "ReturnValue", "RequestStateChange");
> +        goto cleanup;
> +    }
> +
> +    if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not parse return code from '%s'"), returnValue);
> +        goto cleanup;
> +    }
> +
> +    if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) {
> +        virBufferAsprintf(&xpath_expr_buff, "/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:ReferenceParameters/w:SelectorSet/w:Selector[ Name='InstanceID']", methodName);
> +        xpath_expr_string = virBufferContentAndReset(&xpath_expr_buff);
> +        if (xpath_expr_string == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not lookup %s for %s invocation"),
> +                           "InstanceID", "RequestStateChange");
> +            goto cleanup;
> +        }
> +
> +        /* Get concrete job object */
> +        instanceID = ws_xml_get_xpath_value(response, xpath_expr_string);
> +        if (instanceID == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not lookup %s for %s invocation"),
> +                           "InstanceID", "RequestStateChange");
> +            goto cleanup;
> +        }
> +
> +        /* FIXME: Poll every 100ms until the job completes or fails.
> +         *        There seems to be no other way than polling. */
> +        while (!completed) {
> +            virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT);
> +            virBufferAsprintf(&query, "where InstanceID = \"%s\"", instanceID);
> +
> +            if (hypervGetMsvmConcreteJobList(priv, &query, &concreteJob) < 0) {
> +                goto cleanup;
> +            }

brackets - syntax-check

> +            if (concreteJob == NULL) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Could not lookup %s for %s invocation"),
> +                               "Msvm_ConcreteJob", "RequestStateChange");
> +                goto cleanup;
> +            }
> +
> +            switch (concreteJob->data->JobState) {
> +                case MSVM_CONCRETEJOB_JOBSTATE_NEW:
> +                case MSVM_CONCRETEJOB_JOBSTATE_STARTING:
> +                case MSVM_CONCRETEJOB_JOBSTATE_RUNNING:
> +                case MSVM_CONCRETEJOB_JOBSTATE_SHUTTING_DOWN:
> +                    hypervFreeObject(priv, (hypervObject *) concreteJob);
> +                    concreteJob = NULL;
> +                    usleep(100 * 1000);  /* Wait 100 ms */

Is there a reason for the usleep?  Might be good to know why... That 100
ms microsleep may work on a non busy system, but what about a busy one?
I'm just pointing an area ripe for some sort of timing issue.

> +                    continue;
> +
> +                case MSVM_CONCRETEJOB_JOBSTATE_COMPLETED:
> +                    completed = true;
> +                    break;
> +
> +                case MSVM_CONCRETEJOB_JOBSTATE_TERMINATED:
> +                case MSVM_CONCRETEJOB_JOBSTATE_KILLED:
> +                case MSVM_CONCRETEJOB_JOBSTATE_EXCEPTION:
> +                case MSVM_CONCRETEJOB_JOBSTATE_SERVICE:
> +                    goto cleanup;
> +
> +                default:
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Concrete job for %s invocation is in unknown state"),
> +                                   "RequestStateChange");
> +                    goto cleanup;
> +            }
> +        }
> +    }
> +    else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) {

move the "else if" after the closing }; otherwise, syntax-check declares
both sides of if-then-else-if need open/close braces

> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invocation of %s returned an error: %s (%d)"),
> +                       "RequestStateChange", hypervReturnCodeToString(returnCode),
> +                       returnCode);
> +        goto cleanup;
> +    }
> +
> +    result = 0;
> +
> + cleanup:
> +    if (options != NULL)
> +        wsmc_options_destroy(options);
> +    if (response != NULL)
> +        ws_xml_destroy_doc(response);
> +    VIR_FREE(returnValue);
> +    VIR_FREE(instanceID);
> +    VIR_FREE(xpath_expr_string);
> +    hypervFreeObject(priv, (hypervObject *) concreteJob);
> +    virBufferFreeAndReset(&query);
> +    virBufferFreeAndReset(&xpath_expr_buff);
> +
> +    return result;
> +}
> +
> +
> +/* Calls the invoke method by passing provided parameters as an XML tree */
> +int
> +hypervInvokeMethod(hypervPrivate *priv, invokeXmlParam *param_t, int nbParameters,
> +                   const char* methodName, const char* providerURI, const char *selector)
> +{
> +    int result = -1;
> +    WsXmlDocH doc = NULL;
> +    WsXmlNodeH methodNode = NULL;
> +    simpleParam *simple;
> +    eprParam *epr;
> +    embeddedParam *embedded;
> +    int i =0;

s/int/size_t  - syntax-check
s/=0/= 0    - syntax-check


> +
> +    if (hypervCreateXmlStruct(methodName,providerURI,&doc,&methodNode) < 0) {

s/,/, /  syntax-check (3 instances)

> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Could not create xml base structure"));
> +        goto cleanup;
> +    }
> +
> +    /* Process parameters among the three allowed types */
> +    while ( i < nbParameters) {

s/( i/(i    - syntax-check

> +        switch (param_t[i].type) {
> +            case SIMPLE_PARAM:
> +                simple = (simpleParam *) param_t[i].param;
> +                if (hypervAddSimpleParam(param_t[i].name,simple->value, providerURI, &methodNode) < 0) {

s/name,simple/name, simple   - syntax-check

> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   "%s",
> +                                   _("Could not add embedded instance param to xml base structure"));
> +                    goto cleanup;
> +                }
> +                break;
> +            case EPR_PARAM:
> +                epr = (eprParam *) param_t[i].param;
> +                if (hypervAddEprParam(param_t[i].name, epr->query, epr->wmiProviderURI, providerURI, &methodNode, doc, priv) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   "%s",
> +                                   _("Could not add EPR param to xml base structure"));
> +                    goto cleanup;
> +                }
> +                break;
> +            case EMBEDDED_PARAM:
> +                embedded = (embeddedParam *) param_t[i].param;
> +                if (hypervAddEmbeddedParam(embedded->prop_t, embedded->nbProps, param_t[i].name, embedded->instanceName, providerURI, &methodNode) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   "%s",
> +                                   _("Could not add embedded instance param to xml base structure"));
> +                    goto cleanup;
> +                }
> +                break;
> +        }
> +        i++;
> +    }
> +
> +    /* Call the invoke method */
> +    if (hypervInvokeMethodXml(priv, doc, methodName, providerURI, selector) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",

The "%s", could be on previous line

> +                       _("Error during invocation action"));
> +        goto cleanup;
> +    }
> +
> +    result = 0;
> +
> + cleanup:
> +    if (doc != NULL)
> +        ws_xml_destroy_doc(doc);
> +
> +    return result;
> +}
> +
>  
>  
>  #include "hyperv_wmi.generated.c"
> diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
> index 5fbbbac..c14d229 100644
> --- a/src/hyperv/hyperv_wmi.h
> +++ b/src/hyperv/hyperv_wmi.h
> @@ -29,7 +29,11 @@
>  # include "hyperv_wmi_classes.h"
>  # include "openwsman.h"
>  
> +#define ROOT_CIMV2 \

s/#define/# define/   - syntax-check

> +    "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/cimv2/*";
>  
> +#define ROOT_VIRTUALIZATION \

s/#define/# define/  - syntax-check

> +    "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/*";
>  
>  typedef struct _hypervObject hypervObject;
>  
> @@ -38,6 +42,8 @@ int hyperyVerifyResponse(WsManClient *client, WsXmlDocH response,
>  
>  
>  
> +
> +
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>   * Object
>   */
> @@ -91,6 +97,58 @@ enum _Msvm_ReturnCode {
>  
>  const char *hypervReturnCodeToString(int returnCode);
>  
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * hypervInvokeMethod
> + *   Function to invoke WSMAN request with simple, EPR or embedded parameters
> + */
> +
> +enum _PARAM_Type {
> +    SIMPLE_PARAM = 0,
> +    EPR_PARAM = 1,
> +    EMBEDDED_PARAM = 2,
> +};
> +
> +typedef struct _invokeXmlParam invokeXmlParam;
> +struct _invokeXmlParam{
> +        const char *name;
> +        int type;
> +	void *param;

Using a <tab> instead of spaces - syntax-check
> +};
> +
> +typedef struct _eprParam eprParam;
> +struct _eprParam{
> +        virBufferPtr query;
> +	const char *wmiProviderURI;

Using <tab> instead of spaces

> +};
> +
> +typedef struct _simpleParam simpleParam;
> +struct _simpleParam{
> +	const char *value;

<tab>
> +};
> +
> +typedef struct _properties_t properties_t;
> +struct _properties_t{
> +        const char *name;
> +        const char *val;
> +};
> +
> +typedef struct _embeddedParam embeddedParam;
> +struct _embeddedParam{
> +	const char *instanceName;
> +	properties_t *prop_t;
> +	int nbProps;

All 3 have <tab> instead of spaces

John

> +};
> +
> +
> +int
> +hypervInvokeMethod(hypervPrivate *priv,
> +                   invokeXmlParam *parameters,
> +                   int nbParameters,
> +                   const char* methodName,
> +                   const char* providerURI,
> +                   const char *selector);
> +
> +
>  
>  
>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> diff --git a/src/hyperv/openwsman.h b/src/hyperv/openwsman.h
> index f66ed86..27029e7 100644
> --- a/src/hyperv/openwsman.h
> +++ b/src/hyperv/openwsman.h
> @@ -43,4 +43,8 @@
>  #  define SER_NS_INT64(ns, n, x) SER_NS_INT64_FLAGS(ns, n, x, 0)
>  # endif
>  
> +/* wsman-xml.h */
> +WsXmlDocH ws_xml_create_doc( const char *rootNsUri, const char *rootName);
> +WsXmlNodeH xml_parser_get_root(WsXmlDocH doc);
> +
>  #endif /* __OPENWSMAN_H__ */
> 

--
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]