2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam@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.

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index a3c7dc0..1960c4c 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c

@@ -142,6 +143,269 @@ hypervVerifyResponse(WsManClient *client, WsXmlDocH response, }

+/* + * Methods to work with method invocation parameters + */ + +/* + * hypervInitInvokeParamsList: + * @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 +hypervInitInvokeParamsList(hypervPrivate *priv, const char *method, + const char *selector, hypervWmiClassInfoListPtr obj)

I'd rename this to hypervCreateInvokeParamsList to follow the common create/free naming pattern.

+{ + 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; + int i = 0;

Use size_t instead of int for i.

+ + if (params == NULL) + return; + + for (i = 0; i < params->nbParams; i++) { + p = &(params->params[i]); + + switch(p->type) { + case HYPERV_SIMPLE_PARAM: + VIR_FREE(p->simple.name); + VIR_FREE(p->simple.value); + break; + case HYPERV_EPR_PARAM: + virBufferFreeAndReset(p->epr.query); + break; + case HYPERV_EMBEDDED_PARAM: + virHashFree(p->embedded.table); + break; + } + } + + VIR_DISPOSE_N(params->params, params->nbAvailParams); + VIR_FREE(params); +}

+/* + * 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 = ¶ms->params[params->nbParams]; + p->type = HYPERV_SIMPLE_PARAM; + + if (VIR_STRDUP(p->simple.name, name) < 0) + goto cleanup;

+ if (VIR_STRDUP(p->simple.value, value) < 0) { + VIR_FREE(p->simple.name); + goto cleanup; + }

This is the only one of the three param types where you make a copy of the name and the value. This seems unnecessary to me. Why not just store them directly: 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 = ¶ms->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;

This code assumes that typeinfo has at least one item. Is this assumption correct? Can there never be a case where typeinfo has no items?

+ table = virHashCreate(count, virHashValueFree);

I'd call virHashCreate(count, NULL) here, so that the hash table doesn't take ownership of the values. In almost all cases you don't make copies of the param names and values. I don't see why you'd need copies for embedded param values.

+ if (table == NULL) + goto error; + + for (i = 0; typeinfo[i+1].name != NULL; i++) { + item = &typeinfo[i]; + + if (virHashAddEntry(table, item->name, NULL) < 0) + goto error; + } + + return table; + + error: + virHashFree(table); + return table; +} + +int +hypervSetEmbeddedProperty(virHashTablePtr table, const char *name, char *value) +{ + char *tmp = NULL; + + if (VIR_STRDUP(tmp, value) < 0) + return -1;

Instead of creating the hash table with virHashValueFree as the dataFree function and feeding copied values in here. just create the hash table with NULL as the dataFree function and use the value here directly, no copy.

+ return virHashUpdateEntry(table, name, tmp); +} + +/* + * 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;

There is no actual cleanup to be done in this function, just return -1 directly here...

+ + /* Get the typeinfo out of the class info list */ + if (hypervGetWmiClassInfo(priv, info, &classInfo) < 0) + goto cleanup;

... and here.

+ p = ¶ms->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..0a9fac2 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h

@@ -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 { + char *name; + char *value;

I'd switch both to const char and don't store copies here.

+}; +typedef struct _hypervSimpleParam hypervSimpleParam;

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

2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam@datto.com>:

This commit adds support for invoking methods on remote objects via hypervInvokeMethod. --- src/hyperv/hyperv_wmi.c | 569 ++++++++++++++++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_wmi.h | 3 + src/hyperv/openwsman.h | 4 + 3 files changed, 576 insertions(+)

diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 1960c4c..deea907 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -24,6 +24,7 @@ */

#include <config.h> +#include <wsman-soap.h>

#include "internal.h" #include "virerror.h" @@ -34,11 +35,14 @@ #include "hyperv_private.h" #include "hyperv_wmi.h" #include "virstring.h" +#include "openwsman.h" +#include "virlog.h"

#define WS_SERIALIZER_FREE_MEM_WORKS 0

#define VIR_FROM_THIS VIR_FROM_HYPERV

+VIR_LOG_INIT("hyperv.hyperv_wmi");

static int hypervGetWmiClassInfo(hypervPrivate *priv, hypervWmiClassInfoListPtr list, @@ -406,6 +410,571 @@ hypervAddEmbeddedParam(hypervInvokeParamsListPtr params, hypervPrivate *priv, }

+/* + * Serializing parameters to XML and invoking methods + */ + +static int +hypervGetCimTypeInfo(hypervCimTypePtr typemap, const char *name, + hypervCimTypePtr *property) +{ + size_t i = 0; + while (typemap[i].name[0] != '\0') { + if (STREQ(typemap[i].name, name)) { + *property = &typemap[i]; + return 0; + } + i++; + } + + return -1; +} + + +static int +hypervCreateInvokeXmlDoc(hypervInvokeParamsListPtr params, WsXmlDocH *docRoot) +{ + int result = -1; + char *method = NULL; + WsXmlNodeH xmlNodeMethod = NULL; + + if (virAsprintf(&method, "%s_INPUT", params->method) < 0) + goto error; + + *docRoot = ws_xml_create_doc(NULL, method); + if (*docRoot == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not instantiate XML document")); + goto error; + } + + xmlNodeMethod = xml_parser_get_root(*docRoot); + if (xmlNodeMethod == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get root node of XML document")); + goto error; + } + + /* add resource URI as namespace */ + ws_xml_set_ns(xmlNodeMethod, params->resourceUri, "p"); + + result = 0; + goto cleanup; + + error: + if (*docRoot != NULL) { + ws_xml_destroy_doc(*docRoot); + *docRoot = NULL; + } + cleanup: + VIR_FREE(method); + return result;

The error and cleanup label could be merged: result = 0; cleanup: if (result < 0 && *docRoot != NULL) { ws_xml_destroy_doc(*docRoot); *docRoot = NULL; } VIR_FREE(method); return result; }

+} + +static int +hypervSerializeSimpleParam(hypervParamPtr p, const char *resourceUri, + WsXmlNodeH *methodNode) +{ + int result = -1; + WsXmlNodeH xmlNodeParam = NULL; + + xmlNodeParam = ws_xml_add_child(*methodNode, resourceUri, + p->simple.name, p->simple.value); + if (xmlNodeParam == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not create simple param")); + goto cleanup;

There is no actual cleanup, just return -1 here.

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

And just return 0 here.

+} + +static int +hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, + const char *resourceUri, WsXmlDocH doc, WsXmlNodeH *methodNode) +{ + int result = -1; + WsXmlNodeH xmlNodeParam = NULL, + xmlNodeTemp = NULL, + xmlNodeAddr = NULL, + xmlNodeRef = NULL; + xmlNodePtr xmlNodeAddrPtr = NULL, + 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; + + /* init and set up options */ + options = wsmc_options_init(); + if (!options) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); + goto cleanup; + } + wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR); + + /* Get query and create filter based on it */ + query_string = virBufferContentAndReset(p->epr.query); + filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string); + if (!filter) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create WQL filter")); + goto cleanup; + } + + /* enumerate based on the filter from this query */ + xmlDocResponse = wsmc_action_enumerate(priv->client, p->epr.info->rootUri, + options, filter); + if (hypervVerifyResponse(priv->client, xmlDocResponse, "enumeration") < 0) + goto cleanup; + + /* Get context */ + enumContext = wsmc_get_enum_context(xmlDocResponse); + ws_xml_destroy_doc(xmlDocResponse); + + /* Pull using filter and enum context */ + xmlDocResponse = wsmc_action_pull(priv->client, resourceUri, options, + filter, enumContext); + + if (hypervVerifyResponse(priv->client, xmlDocResponse, "pull") < 0) + goto cleanup; + + /* drill down and extract EPR node children */ + if (!(xmlNodeTemp = ws_xml_get_soap_body(xmlDocResponse))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get SOAP body")); + goto cleanup; + } + + if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, + WSENUM_PULL_RESP))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response")); + goto cleanup; + } + + if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, WSENUM_ITEMS))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response items")); + goto cleanup; + } + + if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_EPR))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR items")); + goto cleanup; + } + + if (!(xmlNodeAddr = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, + WSA_ADDRESS))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR address")); + goto cleanup; + } + + if (!(xmlNodeAddrPtr = xmlDocCopyNode((xmlNodePtr) xmlNodeAddr, docPtr, 1))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not copy EPR address")); + goto cleanup; + } + + if (!(xmlNodeRef = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, + WSA_REFERENCE_PARAMETERS))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not lookup EPR item reference parameters")); + goto cleanup; + } + + if (!(xmlNodeRefPtr = xmlDocCopyNode((xmlNodePtr) xmlNodeRef, docPtr, 1))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not copy EPR item reference parameters")); + goto cleanup; + } + + /* now build a new xml doc with the EPR node children */ + if (!(xmlNodeParam = ws_xml_add_child(*methodNode, resourceUri, + p->epr.name, NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add child node to xmlNodeParam")); + goto cleanup; + } + + if (!(ns = ws_xml_ns_add(xmlNodeParam, + "http://schemas.xmlsoap.org/ws/2004/08/addressing", "a"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not set namespace address for xmlNodeParam")); + goto cleanup; + } + + ns = NULL; + if (!(ns = ws_xml_ns_add(xmlNodeParam, + "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd", "w"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not set wsman namespace address for xmlNodeParam")); + goto cleanup; + } + + if (xmlAddChild((xmlNodePtr) *methodNode, (xmlNodePtr) xmlNodeParam) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add child to xml parent node")); + goto cleanup; + } + + if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeAddrPtr) == 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; + } + + /* we did it! */ + result = 0; + + cleanup: + if (options != NULL) + wsmc_options_destroy(options); + if (filter != NULL) + filter_destroy(filter); + ws_xml_destroy_doc(xmlDocResponse); + VIR_FREE(enumContext); + VIR_FREE(query_string); + return result; +} + +static int +hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, + WsXmlNodeH *methodNode) +{ + int result = -1; + WsXmlNodeH xmlNodeInstance = NULL, + xmlNodeProperty = NULL, + xmlNodeParam = NULL, + xmlNodeArray = NULL; + WsXmlDocH xmlDocTemp = NULL, + xmlDocCdata = NULL; + xmlBufferPtr xmlBufferNode = NULL; + const xmlChar *xmlCharCdataContent = NULL; + xmlNodePtr xmlNodeCdata = NULL; + hypervWmiClassInfoPtr classInfo = p->embedded.info; + virHashKeyValuePairPtr items = NULL; + hypervCimTypePtr property = NULL; + int numKeys = -1; + int len = 0, i = 0;

+ if (!(xmlNodeParam = ws_xml_add_child(*methodNode, resourceUri, p->embedded.name, + NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not add child node %s"), + p->embedded.name); + goto cleanup; + } + + /* create the temp xml doc */ + + /* start with the INSTANCE node */ + if (!(xmlDocTemp = ws_xml_create_doc(NULL, "INSTANCE"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not create temporary xml doc")); + goto cleanup; + } + + if (!(xmlNodeInstance = xml_parser_get_root(xmlDocTemp))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get temp xml doc root")); + goto cleanup; + } + + /* add CLASSNAME node to INSTANCE node */ + if (ws_xml_add_node_attr(xmlNodeInstance, NULL, "CLASSNAME", + classInfo->name) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add attribute to node")); + goto cleanup; + } + + /* retrieve parameters out of hash table */ + numKeys = virHashSize(p->embedded.table); + items = virHashGetItems(p->embedded.table, NULL); + if (!items) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not read embedded param hash table")); + goto cleanup; + } + + /* Add the parameters */ + if (numKeys > 0) {

This if is unnecessary.

+ for (i = 0; i < numKeys; i++) { + const char *name = items[i].key; + const char *value = items[i].value; + + if (value != NULL) { + if (hypervGetCimTypeInfo(classInfo->propertyInfo, name, + &property) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not read type information")); + goto cleanup; + } + + if (!(xmlNodeProperty = ws_xml_add_child(xmlNodeInstance, NULL, + property->isArray ? "PROPERTY.ARRAY" : "PROPERTY", + NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add child to XML node")); + goto cleanup; + } + + if (ws_xml_add_node_attr(xmlNodeProperty, NULL, "NAME", name) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add attribute to XML node")); + goto cleanup; + } + + if (ws_xml_add_node_attr(xmlNodeProperty, NULL, "TYPE", property->type) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add attribute to XML node")); + goto cleanup; + } + + /* If this attribute is an array, add VALUE.ARRAY node */ + if (property->isArray) { + if (!(xmlNodeArray = ws_xml_add_child(xmlNodeProperty, NULL, + "VALUE.ARRAY", NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add child to XML node")); + goto cleanup; + } + } + + /* add the child */ + if (ws_xml_add_child(property->isArray ? xmlNodeArray : xmlNodeProperty, + NULL, "VALUE", value) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add child to XML node")); + goto cleanup; + } + + xmlNodeArray = NULL; + xmlNodeProperty = NULL; + } + } + } + + /* 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 temp XML doc")); + goto cleanup; + } + + len = xmlBufferLength(xmlBufferNode); + xmlCharCdataContent = xmlBufferContent(xmlBufferNode); + if (!(xmlNodeCdata = xmlNewCDataBlock((xmlDocPtr) xmlDocCdata, + xmlCharCdataContent, len))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not create CDATA element")); + goto cleanup; + } + + /* Add CDATA node to the doc root */ + if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeCdata) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not add CDATA to doc root")); + goto cleanup; + } + + /* we did it! */ + result = 0; + + cleanup: + VIR_FREE(items); + ws_xml_destroy_doc(xmlDocCdata); + ws_xml_destroy_doc(xmlDocTemp); + if (xmlBufferNode) + xmlBufferFree(xmlBufferNode); + return result; +} + + +/* + * hypervInvokeMethod: + * @priv: hypervPrivate object associated with the connection + * @params: object containing the all necessary information for method + * invocation + * @res: Optional out parameter to contain the response XML. + * + * Performs an invocation described by @params, and optionally returns the + * XML containing the result. Returns -1 on failure, 0 on success. + */ +int +hypervInvokeMethod(hypervPrivate *priv, hypervInvokeParamsListPtr params, + WsXmlDocH *res) +{ + int result = -1; + size_t i = 0; + int returnCode; + WsXmlDocH paramsDocRoot = NULL; + client_opt_t *options = NULL; + WsXmlDocH response = NULL; + WsXmlNodeH methodNode = NULL; + char *returnValue_xpath = NULL; + char *jobcode_instance_xpath = NULL; + char *returnValue = NULL; + char *instanceID = NULL; + bool completed = false; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_ConcreteJob *job = NULL; + int jobState = -1; + hypervParamPtr p = NULL; + + if (hypervCreateInvokeXmlDoc(params, ¶msDocRoot) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not create XML document")); + goto cleanup; + } + + methodNode = xml_parser_get_root(paramsDocRoot); + if (methodNode == NULL) + goto cleanup; + + /* Serialize parameters */ + for (i = 0; i < params->nbParams; i++) { + p = &(params->params[i]); + + switch (p->type) { + case HYPERV_SIMPLE_PARAM: + if (hypervSerializeSimpleParam(p, params->resourceUri, + &methodNode) < 0) + goto cleanup; + break; + case HYPERV_EPR_PARAM: + if (hypervSerializeEprParam(p, priv, params->resourceUri, + paramsDocRoot, &methodNode) < 0) + goto cleanup; + break; + case HYPERV_EMBEDDED_PARAM: + if (hypervSerializeEmbeddedParam(p, params->resourceUri, + &methodNode) < 0) + goto cleanup; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unknown parameter type")); + goto cleanup; + } + } + + /* Invoke the method and get the response */ + + options = wsmc_options_init(); + + if (!options) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); + goto cleanup; + } + + wsmc_add_selectors_from_str(options, params->selector); + + /* do the invoke */ + response = wsmc_action_invoke(priv->client, params->resourceUri, options, + params->method, paramsDocRoot); + + /* check return code of invocation */ + if (virAsprintf(&returnValue_xpath, "/s:Envelope/s:Body/p:%s_OUTPUT/p:ReturnValue", + params->method) < 0) + goto cleanup; + + returnValue = ws_xml_get_xpath_value(response, returnValue_xpath); + if (!returnValue) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get return value for %s invocation"), + params->method);

Missing goto cleanup here?

+ } + + if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) + goto cleanup; + + if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) { + if (virAsprintf(&jobcode_instance_xpath, + "/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:ReferenceParameters/" + "w:SelectorSet/w:Selector[@Name='InstanceID']", + params->method) < 0) { + goto cleanup; + } + + instanceID = ws_xml_get_xpath_value(response, jobcode_instance_xpath); + if (!instanceID) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get instance ID for %s invocation"), + params->method); + goto cleanup; + } + + /* Poll every 100 ms until the job completes or fails */ + while (!completed) { + virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); + virBufferAsprintf(&query, "where InstanceID = \"%s\"", instanceID); + + if (hypervGetMsvmConcreteJobList(priv, &query, &job) < 0 + || job == NULL) + goto cleanup; + + jobState = job->data.common->JobState; + switch (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 *) job); + job = NULL; + usleep(100 * 1000); /* sleep 100 ms */ + 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, "%s", + _("Unknown invocation state")); + goto cleanup; + } + }

How do you avoid waiting forever for a job to finish?

+ } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Invocation of %s returned an error: %s (%d)"), + params->method, hypervReturnCodeToString(returnCode), + returnCode); + goto cleanup; + } + + if (res != NULL)

Your pointer checking style is inconsistent. libvirt favors the short form "if (ptr)" and "if (!ptr)" for new code over the long format "if (ptr != NULL)" and "if (ptr == NULL)".

+ *res = response; + + result = 0; + + cleanup: + if (options) + wsmc_options_destroy(options); + if (response && (res == NULL)) + ws_xml_destroy_doc(response); + if (paramsDocRoot) + ws_xml_destroy_doc(paramsDocRoot); + VIR_FREE(returnValue_xpath); + VIR_FREE(jobcode_instance_xpath); + VIR_FREE(returnValue); + VIR_FREE(instanceID); + virBufferFreeAndReset(&query); + hypervFreeObject(priv, (hypervObject *) job); + hypervFreeInvokeParams(params); + return result; +}

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

2017-05-05 22:24 GMT+02:00  <sramanujam@datto.com>:
> On Tue, 2017-05-02 at 00:26 +0200, Matthias Bolte wrote:
>> 2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam@datto.com>:
>> > This commit adds support for invoking methods on remote objects
>> > via hypervInvokeMethod.
>> > ---
>> >  src/hyperv/hyperv_wmi.c | 569
>> > ++++++++++++++++++++++++++++++++++++++++++++++++
>> >  src/hyperv/hyperv_wmi.h |   3 +
>> >  src/hyperv/openwsman.h  |   4 +
>> >  3 files changed, 576 insertions(+)
>> >
>> > diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
>> > index 1960c4c..deea907 100644
>> > --- a/src/hyperv/hyperv_wmi.c
>> > +++ b/src/hyperv/hyperv_wmi.c
>> > @@ -24,6 +24,7 @@
>> >   */
>> >
>> >  #include <config.h>
>> > +#include <wsman-soap.h>
>> >
>> >  #include "internal.h"
>> >  #include "virerror.h"
>> > @@ -34,11 +35,14 @@
>> >  #include "hyperv_private.h"
>> >  #include "hyperv_wmi.h"
>> >  #include "virstring.h"
>> > +#include "openwsman.h"
>> > +#include "virlog.h"
>> >
>> >  #define WS_SERIALIZER_FREE_MEM_WORKS 0
>> >
>> >  #define VIR_FROM_THIS VIR_FROM_HYPERV
>> >
>> > +VIR_LOG_INIT("hyperv.hyperv_wmi");
>> >
>> >  static int
>> >  hypervGetWmiClassInfo(hypervPrivate *priv,
>> > hypervWmiClassInfoListPtr list,
>> > @@ -406,6 +410,571 @@
>> > hypervAddEmbeddedParam(hypervInvokeParamsListPtr params,
>> > hypervPrivate *priv,
>> >  }
>> >
>> >
>> > +/*
>> > + * Serializing parameters to XML and invoking methods
>> > + */
>> > +
>> > +static int
>> > +hypervGetCimTypeInfo(hypervCimTypePtr typemap, const char *name,
>> > +        hypervCimTypePtr *property)
>> > +{
>> > +    size_t i = 0;
>> > +    while (typemap[i].name[0] != '\0') {
>> > +        if (STREQ(typemap[i].name, name)) {
>> > +            *property = &typemap[i];
>> > +            return 0;
>> > +        }
>> > +        i++;
>> > +    }
>> > +
>> > +    return -1;
>> > +}
>> > +
>> > +
>> > +static int
>> > +hypervCreateInvokeXmlDoc(hypervInvokeParamsListPtr params,
>> > WsXmlDocH *docRoot)
>> > +{
>> > +    int result = -1;
>> > +    char *method = NULL;
>> > +    WsXmlNodeH xmlNodeMethod = NULL;
>> > +
>> > +    if (virAsprintf(&method, "%s_INPUT", params->method) < 0)
>> > +        goto error;
>> > +
>> > +    *docRoot = ws_xml_create_doc(NULL, method);
>> > +    if (*docRoot == NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not instantiate XML document"));
>> > +        goto error;
>> > +    }
>> > +
>> > +    xmlNodeMethod = xml_parser_get_root(*docRoot);
>> > +    if (xmlNodeMethod == NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not get root node of XML document"));
>> > +        goto error;
>> > +    }
>> > +
>> > +    /* add resource URI as namespace */
>> > +    ws_xml_set_ns(xmlNodeMethod, params->resourceUri, "p");
>> > +
>> > +    result = 0;
>> > +    goto cleanup;
>> > +
>> > + error:
>> > +    if (*docRoot != NULL) {
>> > +        ws_xml_destroy_doc(*docRoot);
>> > +        *docRoot = NULL;
>> > +    }
>> > + cleanup:
>> > +    VIR_FREE(method);
>> > +    return result;
>>
>> The error and cleanup label could be merged:
>>
>>
>>     result = 0;
>>
>> cleanup:
>>     if (result < 0 && *docRoot != NULL) {
>>         ws_xml_destroy_doc(*docRoot);
>>         *docRoot = NULL;
>>     }
>>
>>     VIR_FREE(method);
>>     return result;
>> }
>>
>>
>> > +}
>> > +
>> > +static int
>> > +hypervSerializeSimpleParam(hypervParamPtr p, const char
>> > *resourceUri,
>> > +        WsXmlNodeH *methodNode)
>> > +{
>> > +    int result = -1;
>> > +    WsXmlNodeH xmlNodeParam = NULL;
>> > +
>> > +    xmlNodeParam = ws_xml_add_child(*methodNode, resourceUri,
>> > +            p->simple.name, p->simple.value);
>> > +    if (xmlNodeParam == NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not create simple param"));
>> > +        goto cleanup;
>>
>> There is no actual cleanup, just return -1 here.
>>
>> > +    }
>> > +
>> > +    result = 0;
>> > +
>> > + cleanup:
>> > +    return result;
>>
>> And just return 0 here.
>>
>> > +}
>> > +
>> > +static int
>> > +hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv,
>> > +        const char *resourceUri, WsXmlDocH doc, WsXmlNodeH
>> > *methodNode)
>> > +{
>> > +    int result = -1;
>> > +    WsXmlNodeH xmlNodeParam = NULL,
>> > +               xmlNodeTemp = NULL,
>> > +               xmlNodeAddr = NULL,
>> > +               xmlNodeRef = NULL;
>> > +    xmlNodePtr xmlNodeAddrPtr = NULL,
>> > +               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;
>> > +
>> > +    /* init and set up options */
>> > +    options = wsmc_options_init();
>> > +    if (!options) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > init options"));
>> > +        goto cleanup;
>> > +    }
>> > +    wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR);
>> > +
>> > +    /* Get query and create filter based on it */
>> > +    query_string = virBufferContentAndReset(p->epr.query);
>> > +    filter = filter_create_simple(WSM_WQL_FILTER_DIALECT,
>> > query_string);
>> > +    if (!filter) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > create WQL filter"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* enumerate based on the filter from this query */
>> > +    xmlDocResponse = wsmc_action_enumerate(priv->client, p-
>> > >epr.info->rootUri,
>> > +            options, filter);
>> > +    if (hypervVerifyResponse(priv->client, xmlDocResponse,
>> > "enumeration") < 0)
>> > +        goto cleanup;
>> > +
>> > +    /* Get context */
>> > +    enumContext = wsmc_get_enum_context(xmlDocResponse);
>> > +    ws_xml_destroy_doc(xmlDocResponse);
>> > +
>> > +    /* Pull using filter and enum context */
>> > +    xmlDocResponse = wsmc_action_pull(priv->client, resourceUri,
>> > options,
>> > +            filter, enumContext);
>> > +
>> > +    if (hypervVerifyResponse(priv->client, xmlDocResponse, "pull")
>> > < 0)
>> > +        goto cleanup;
>> > +
>> > +    /* drill down and extract EPR node children */
>> > +    if (!(xmlNodeTemp = ws_xml_get_soap_body(xmlDocResponse))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > get SOAP body"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0,
>> > XML_NS_ENUMERATION,
>> > +            WSENUM_PULL_RESP))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > get response"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0,
>> > XML_NS_ENUMERATION, WSENUM_ITEMS))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > get response items"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0,
>> > XML_NS_ADDRESSING, WSA_EPR))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > get EPR items"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeAddr = ws_xml_get_child(xmlNodeTemp, 0,
>> > XML_NS_ADDRESSING,
>> > +                    WSA_ADDRESS))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > get EPR address"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeAddrPtr = xmlDocCopyNode((xmlNodePtr)
>> > xmlNodeAddr, docPtr, 1))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > copy EPR address"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeRef = ws_xml_get_child(xmlNodeTemp, 0,
>> > XML_NS_ADDRESSING,
>> > +            WSA_REFERENCE_PARAMETERS))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not lookup EPR item reference
>> > parameters"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeRefPtr = xmlDocCopyNode((xmlNodePtr) xmlNodeRef,
>> > docPtr, 1))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not copy EPR item reference
>> > parameters"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* now build a new xml doc with the EPR node children */
>> > +    if (!(xmlNodeParam = ws_xml_add_child(*methodNode,
>> > resourceUri,
>> > +                    p->epr.name, NULL))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not add child node to xmlNodeParam"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(ns = ws_xml_ns_add(xmlNodeParam,
>> > +                    "http://schemas.xmlsoap.org/ws/2004/08/address
>> > ing", "a"))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not set namespace address for
>> > xmlNodeParam"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    ns = NULL;
>> > +    if (!(ns = ws_xml_ns_add(xmlNodeParam,
>> > +                    "http://schemas.dmtf.org/wbem/wsman/1/wsman.xs
>> > d", "w"))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not set wsman namespace address for
>> > xmlNodeParam"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (xmlAddChild((xmlNodePtr) *methodNode, (xmlNodePtr)
>> > xmlNodeParam) == NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not add child to xml parent node"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeAddrPtr) ==
>> > 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;
>> > +    }
>> > +
>> > +    /* we did it! */
>> > +    result = 0;
>> > +
>> > + cleanup:
>> > +    if (options != NULL)
>> > +        wsmc_options_destroy(options);
>> > +    if (filter != NULL)
>> > +        filter_destroy(filter);
>> > +    ws_xml_destroy_doc(xmlDocResponse);
>> > +    VIR_FREE(enumContext);
>> > +    VIR_FREE(query_string);
>> > +    return result;
>> > +}
>> > +
>> > +static int
>> > +hypervSerializeEmbeddedParam(hypervParamPtr p, const char
>> > *resourceUri,
>> > +        WsXmlNodeH *methodNode)
>> > +{
>> > +    int result = -1;
>> > +    WsXmlNodeH xmlNodeInstance = NULL,
>> > +               xmlNodeProperty = NULL,
>> > +               xmlNodeParam = NULL,
>> > +               xmlNodeArray = NULL;
>> > +    WsXmlDocH xmlDocTemp = NULL,
>> > +              xmlDocCdata = NULL;
>> > +    xmlBufferPtr xmlBufferNode = NULL;
>> > +    const xmlChar *xmlCharCdataContent = NULL;
>> > +    xmlNodePtr xmlNodeCdata = NULL;
>> > +    hypervWmiClassInfoPtr classInfo = p->embedded.info;
>> > +    virHashKeyValuePairPtr items = NULL;
>> > +    hypervCimTypePtr property = NULL;
>> > +    int numKeys = -1;
>> > +    int len = 0, i = 0;
>> > +    if (!(xmlNodeParam = ws_xml_add_child(*methodNode,
>> > resourceUri, p->embedded.name,
>> > +                    NULL))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not add
>> > child node %s"),
>> > +                p->embedded.name);
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* create the temp xml doc */
>> > +
>> > +    /* start with the INSTANCE node */
>> > +    if (!(xmlDocTemp = ws_xml_create_doc(NULL, "INSTANCE"))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not create temporary xml doc"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    if (!(xmlNodeInstance = xml_parser_get_root(xmlDocTemp))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not get temp xml doc root"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* add CLASSNAME node to INSTANCE node */
>> > +    if (ws_xml_add_node_attr(xmlNodeInstance, NULL, "CLASSNAME",
>> > +                classInfo->name) == NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not add attribute to node"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* retrieve parameters out of hash table */
>> > +    numKeys = virHashSize(p->embedded.table);
>> > +    items = virHashGetItems(p->embedded.table, NULL);
>> > +    if (!items) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not read embedded param hash table"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* Add the parameters */
>> > +    if (numKeys > 0) {
>>
>> This if is unnecessary.
>>
>> > +        for (i = 0; i < numKeys; i++) {
>> > +            const char *name = items[i].key;
>> > +            const char *value = items[i].value;
>> > +
>> > +            if (value != NULL) {
>> > +                if (hypervGetCimTypeInfo(classInfo->propertyInfo,
>> > name,
>> > +                            &property) < 0) {
>> > +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                            _("Could not read type information"));
>> > +                    goto cleanup;
>> > +                }
>> > +
>> > +                if (!(xmlNodeProperty =
>> > ws_xml_add_child(xmlNodeInstance, NULL,
>> > +                                property->isArray ?
>> > "PROPERTY.ARRAY" : "PROPERTY",
>> > +                                NULL))) {
>> > +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                            _("Could not add child to XML node"));
>> > +                    goto cleanup;
>> > +                }
>> > +
>> > +                if (ws_xml_add_node_attr(xmlNodeProperty, NULL,
>> > "NAME", name) == NULL) {
>> > +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                            _("Could not add attribute to XML
>> > node"));
>> > +                    goto cleanup;
>> > +                }
>> > +
>> > +                if (ws_xml_add_node_attr(xmlNodeProperty, NULL,
>> > "TYPE", property->type) == NULL) {
>> > +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                            _("Could not add attribute to XML
>> > node"));
>> > +                    goto cleanup;
>> > +                }
>> > +
>> > +                /* If this attribute is an array, add VALUE.ARRAY
>> > node */
>> > +                if (property->isArray) {
>> > +                    if (!(xmlNodeArray =
>> > ws_xml_add_child(xmlNodeProperty, NULL,
>> > +                                    "VALUE.ARRAY", NULL))) {
>> > +                        virReportError(VIR_ERR_INTERNAL_ERROR,
>> > "%s",
>> > +                                _("Could not add child to XML
>> > node"));
>> > +                        goto cleanup;
>> > +                    }
>> > +                }
>> > +
>> > +                /* add the child */
>> > +                if (ws_xml_add_child(property->isArray ?
>> > xmlNodeArray : xmlNodeProperty,
>> > +                            NULL, "VALUE", value) == NULL) {
>> > +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                            _("Could not add child to XML node"));
>> > +                    goto cleanup;
>> > +                }
>> > +
>> > +                xmlNodeArray = NULL;
>> > +                xmlNodeProperty = NULL;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> > +    /* 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 temp XML doc"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    len = xmlBufferLength(xmlBufferNode);
>> > +    xmlCharCdataContent = xmlBufferContent(xmlBufferNode);
>> > +    if (!(xmlNodeCdata = xmlNewCDataBlock((xmlDocPtr) xmlDocCdata,
>> > +                    xmlCharCdataContent, len))) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not create CDATA element"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* Add CDATA node to the doc root */
>> > +    if (xmlAddChild((xmlNodePtr) xmlNodeParam, xmlNodeCdata) ==
>> > NULL) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not add CDATA to doc root"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    /* we did it! */
>> > +    result = 0;
>> > +
>> > + cleanup:
>> > +    VIR_FREE(items);
>> > +    ws_xml_destroy_doc(xmlDocCdata);
>> > +    ws_xml_destroy_doc(xmlDocTemp);
>> > +    if (xmlBufferNode)
>> > +        xmlBufferFree(xmlBufferNode);
>> > +    return result;
>> > +}
>> > +
>> > +
>> > +/*
>> > + * hypervInvokeMethod:
>> > + * @priv: hypervPrivate object associated with the connection
>> > + * @params: object containing the all necessary information for
>> > method
>> > + * invocation
>> > + * @res: Optional out parameter to contain the response XML.
>> > + *
>> > + * Performs an invocation described by @params, and optionally
>> > returns the
>> > + * XML containing the result. Returns -1 on failure, 0 on success.
>> > + */
>> > +int
>> > +hypervInvokeMethod(hypervPrivate *priv, hypervInvokeParamsListPtr
>> > params,
>> > +        WsXmlDocH *res)
>> > +{
>> > +    int result = -1;
>> > +    size_t i = 0;
>> > +    int returnCode;
>> > +    WsXmlDocH paramsDocRoot = NULL;
>> > +    client_opt_t *options = NULL;
>> > +    WsXmlDocH response = NULL;
>> > +    WsXmlNodeH methodNode = NULL;
>> > +    char *returnValue_xpath = NULL;
>> > +    char *jobcode_instance_xpath = NULL;
>> > +    char *returnValue = NULL;
>> > +    char *instanceID = NULL;
>> > +    bool completed = false;
>> > +    virBuffer query = VIR_BUFFER_INITIALIZER;
>> > +    Msvm_ConcreteJob *job = NULL;
>> > +    int jobState = -1;
>> > +    hypervParamPtr p = NULL;
>> > +
>> > +    if (hypervCreateInvokeXmlDoc(params, ¶msDocRoot) < 0) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                _("Could not create XML document"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    methodNode = xml_parser_get_root(paramsDocRoot);
>> > +    if (methodNode == NULL)
>> > +        goto cleanup;
>> > +
>> > +    /* Serialize parameters */
>> > +    for (i = 0; i < params->nbParams; i++) {
>> > +        p = &(params->params[i]);
>> > +
>> > +        switch (p->type) {
>> > +            case HYPERV_SIMPLE_PARAM:
>> > +                if (hypervSerializeSimpleParam(p, params-
>> > >resourceUri,
>> > +                            &methodNode) < 0)
>> > +                    goto cleanup;
>> > +                break;
>> > +            case HYPERV_EPR_PARAM:
>> > +                if (hypervSerializeEprParam(p, priv, params-
>> > >resourceUri,
>> > +                            paramsDocRoot, &methodNode) < 0)
>> > +                    goto cleanup;
>> > +                break;
>> > +            case HYPERV_EMBEDDED_PARAM:
>> > +                if (hypervSerializeEmbeddedParam(p, params-
>> > >resourceUri,
>> > +                            &methodNode) < 0)
>> > +                    goto cleanup;
>> > +                break;
>> > +            default:
>> > +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> > +                        _("Unknown parameter type"));
>> > +                goto cleanup;
>> > +        }
>> > +    }
>> > +
>> > +    /* Invoke the method and get the response */
>> > +
>> > +    options = wsmc_options_init();
>> > +
>> > +    if (!options) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not
>> > init options"));
>> > +        goto cleanup;
>> > +    }
>> > +
>> > +    wsmc_add_selectors_from_str(options, params->selector);
>> > +
>> > +    /* do the invoke */
>> > +    response = wsmc_action_invoke(priv->client, params-
>> > >resourceUri, options,
>> > +            params->method, paramsDocRoot);
>> > +
>> > +    /* check return code of invocation */
>> > +    if (virAsprintf(&returnValue_xpath,
>> > "/s:Envelope/s:Body/p:%s_OUTPUT/p:ReturnValue",
>> > +            params->method) < 0)
>> > +        goto cleanup;
>> > +
>> > +    returnValue = ws_xml_get_xpath_value(response,
>> > returnValue_xpath);
>> > +    if (!returnValue) {
>> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> > +                       _("Could not get return value for %s
>> > invocation"),
>> > +                       params->method);
>>
>> Missing goto cleanup here?
>>
>> > +    }
>> > +
>> > +    if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0)
>> > +        goto cleanup;
>> > +
>> > +    if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) {
>> > +        if (virAsprintf(&jobcode_instance_xpath,
>> > +                    "/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:Refere
>> > nceParameters/"
>> > +                    "w:SelectorSet/w:Selector[@Name='InstanceID']"
>> > ,
>> > +                    params->method) < 0) {
>> > +            goto cleanup;
>> > +        }
>> > +
>> > +        instanceID = ws_xml_get_xpath_value(response,
>> > jobcode_instance_xpath);
>> > +        if (!instanceID) {
>> > +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> > +                           _("Could not get instance ID for %s
>> > invocation"),
>> > +                           params->method);
>> > +            goto cleanup;
>> > +        }
>> > +
>> > +        /* Poll every 100 ms until the job completes or fails */
>> > +        while (!completed) {
>> > +            virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT);
>> > +            virBufferAsprintf(&query, "where InstanceID = \"%s\"",
>> > instanceID);
>> > +
>> > +            if (hypervGetMsvmConcreteJobList(priv, &query, &job) <
>> > 0
>> > +                    || job == NULL)
>> > +                goto cleanup;
>> > +
>> > +            jobState = job->data.common->JobState;
>> > +            switch (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 *) job);
>> > +                    job = NULL;
>> > +                    usleep(100 * 1000); /* sleep 100 ms */
>> > +                    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, "%s",
>> > +                                   _("Unknown invocation state"));
>> > +                    goto cleanup;
>> > +            }
>> > +        }
>>
>> How do you avoid waiting forever for a job to finish?
>
> Windows has a timeout on jobs already, by default it's set to 1
> minute. If that timeout is hit, the WMI service will terminate the
> operation on its own and any further queries about that job will
> indicate an error. So, since all we are doing is querying a particular
> job's status and leaving it up to Windows to actually watch over the
> execution of that job, we _should_ always get a response back from the
> server.
>
> Therefore, if we fail to receive a response at all from the server,
> then that should be indicative of a different kind of issue, such as a
> network issue or perhaps the server is overloaded. I'm not sure whether
> to handle that here (and if so, how to do that correctly within
> Libvirt), or if another part of Libvirt will handle such situations.
>
> So, right now the code makes the assumption that the server will always
> respond to the hypervGetConcreteJobList() call. I would definitely
> appreciate ideas/guidance on how best to handle this, it's something
> that I grappled with a bit when implementing this and didn't really
> have a good option for.
>

Okay, you should add a comment that the server should normally timeout
jobs aber 1 minute. As a second safe guard you could add your own
timeout here of say 1.5 minutes to ensure that if all other mechanisms
fail that you still can be sure here that the code cannot wait for
ever on a job.

Regards,
Matthias

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

2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam@datto.com>:

This commit adds support for virDomainSendKey. It also serves as an example of how to use the new method invocation APIs with a single "simple" type parameter. --- src/hyperv/hyperv_driver.c | 85 ++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_wmi.c | 7 +++ src/hyperv/hyperv_wmi.h | 3 +- src/hyperv/hyperv_wmi_generator.input | 86 +++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 0ca5971..9562d5a 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -35,6 +35,7 @@ #include "hyperv_wmi.h" #include "openwsman.h" #include "virstring.h" +#include "virkeycode.h"

#define VIR_FROM_THIS VIR_FROM_HYPERV

@@ -1373,6 +1374,89 @@ hypervConnectListAllDomains(virConnectPtr conn, #undef MATCH

+static int +hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, + unsigned int holdtime ATTRIBUTE_UNUSED, unsigned int *keycodes, + int nkeycodes, unsigned int flags) +{ + int result = -1; + size_t i = 0; + int keycode = 0; + int *translatedKeycodes = NULL; + hypervPrivate *priv = domain->conn->privateData; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + char *selector = NULL; + Msvm_ComputerSystem *computerSystem = NULL; + Msvm_Keyboard *keyboard = NULL; + virBuffer query = VIR_BUFFER_INITIALIZER; + hypervInvokeParamsListPtr params = NULL; + + virCheckFlags(0, -1); + + virUUIDFormat(domain->uuid, uuid_string); + + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) + goto cleanup; + + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where ResultClass = Msvm_Keyboard", + uuid_string); + +

Unnecessary extra empty line.

+ if (hypervGetMsvmKeyboardList(priv, &query, &keyboard) < 0) + goto cleanup; + + /* translate keycodes to xt and generate keyup scancodes. */ + translatedKeycodes = (int *) keycodes;

You cannot translate the keycodes in-place. For the VBox and QEMU drivers this is kind of okay, because they are always behind the remote driver. But for the Hyper-V driver this is different. You're directly modifying user provided memory here. This is not okay, you need to make a copy of the keycodes before translating them.

+ for (i = 0; i < nkeycodes; i++) { + if (codeset != VIR_KEYCODE_SET_WIN32) { + keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_WIN32, + translatedKeycodes[i]); + + if (keycode < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not translate keycode")); + goto cleanup; + } + translatedKeycodes[i] = keycode; + } + } + + if (virAsprintf(&selector, + "CreationClassName=Msvm_Keyboard&DeviceID=%s&" + "SystemCreationClassName=Msvm_ComputerSystem&" + "SystemName=%s", keyboard->data.common->DeviceID, uuid_string) < 0) + goto cleanup; + + /* type the keys */ + for (i = 0; i < nkeycodes; i++) { + char keycodeStr[sizeof(int) * 3 + 2];

Use "char keycodeStr[INT_BUFSIZE_BOUND(int)];" instead.

+ snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]); + + /* params obj takes ownership of selector */ + params = hypervInitInvokeParamsList(priv, "TypeKey", selector, + Msvm_Keyboard_WmiInfo); + if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0) + goto cleanup; + + if (hypervInvokeMethod(priv, params, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not press key %d"), + translatedKeycodes[i]); + goto cleanup; + }

This is not correct. If virDomainSendKey is called with more than one keycode than all those keys have to be pressed at once. This allows to send key combination e.g. Alt+F4. But the way you invoke the TypeKey function here will just send them one after another. Also you currently don't handle holdtime. I think you can make both things work by using PressKey/Sleep/ReleaseKey instead of TypeKey. See how the VBox driver does it: http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/vbox/vbox_common.c#l7551 -- Matthias Bolte http://photron.blogspot.com

2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam@datto.com>:

Introduces support for virDomainSetMemory. This also serves an an example for how to use the new method invocation API with a more complicated method, this time including an EPR and embedded param. --- src/hyperv/hyperv_driver.c | 96 +++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_wmi.c | 51 +++++++++++++++++++ src/hyperv/hyperv_wmi.h | 11 ++++ src/hyperv/hyperv_wmi_generator.input | 30 +++++++++++ 4 files changed, 188 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 9562d5a..104e13f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1459,6 +1459,100 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, }

+static int +hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, + unsigned int flags) +{ + int result = -1; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + char *memory_str = NULL; + hypervInvokeParamsListPtr params = NULL; + unsigned long memory_mb = memory / 1024; + Msvm_VirtualSystemSettingData *vssd = NULL; + Msvm_MemorySettingData *memsd = NULL; + virBuffer eprQuery = VIR_BUFFER_INITIALIZER; + virHashTablePtr memResource = NULL; + + virCheckFlags(0, -1); + + /* memory has to be a multiple of 2; round up if necessary */ + if (memory_mb % 2) memory_mb++; + + if (virAsprintf(&memory_str, "%lu", memory_mb) < 0) + goto cleanup; + + virUUIDFormat(domain->uuid, uuid_string); + + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) + goto cleanup; + + if (hypervGetMsvmMemorySettingDataFromVSSD(priv, vssd->data.common->InstanceID, + &memsd) < 0) + goto cleanup; + + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + params = hypervInitInvokeParamsList(priv, "ModifyVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, + Msvm_VirtualSystemManagementService_WmiInfo); + + virBufferAddLit(&eprQuery, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&eprQuery, "where Name = \"%s\"", uuid_string); + + if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery, + Msvm_ComputerSystem_WmiInfo) < 0) + goto cleanup; + + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) { + params = hypervInitInvokeParamsList(priv, "ModifyResourceSettings", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, + Msvm_VirtualSystemManagementService_WmiInfo); + } + + memResource = hypervCreateEmbeddedParam(priv, Msvm_MemorySettingData_WmiInfo); + if (memResource == NULL) + goto cleanup; + + if (hypervSetEmbeddedProperty(memResource, "VirtualQuantity", memory_str) < 0) + goto cleanup; + + if (hypervSetEmbeddedProperty(memResource, "InstanceID", + memsd->data.common->InstanceID) < 0) + goto cleanup; + +

Unnecessary extra empty line.

+ if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + if (hypervAddEmbeddedParam(params, priv, "ResourceSettingData", + memResource, Msvm_MemorySettingData_WmiInfo) < 0) + goto cleanup; + + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) { + if (hypervAddEmbeddedParam(params, priv, "ResourceSettings", + memResource, Msvm_MemorySettingData_WmiInfo) < 0) + goto cleanup; + } + + if (hypervInvokeMethod(priv, params, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set memory")); + goto cleanup; + } + + result = 0; + cleanup: + if (memory_str) VIR_FREE(memory_str);

VIR_FREE is fine being called with NULL. Actually "make syntax-check" should have told you.

+ hypervFreeObject(priv, (hypervObject *) vssd); + hypervFreeObject(priv, (hypervObject *) memsd); + return result; +}

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