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