On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 106 +++++++++++++++++++++++++++++++++++++++++++- src/hyperv/hyperv_private.h | 2 + 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index b642a02..4a5e80d 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 "virtypedparam.h"

#define VIR_FROM_THIS VIR_FROM_HYPERV

@@ -51,11 +52,15 @@ hypervFreePrivate(hypervPrivate **priv) wsmc_release((*priv)->client); }

+ if ((*priv)->caps != NULL) + virObjectUnref((*priv)->caps); + hypervFreeParsedUri(&(*priv)->parsedUri); VIR_FREE(*priv); }

- +/* Forward declaration of hypervCapsInit */ +static virCapsPtr hypervCapsInit(hypervPrivate *priv);

Ewww.... Why not define the function right here then?

static virDrvOpenStatus hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, @@ -183,6 +188,12 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, goto cleanup; }

+ /* Setup capabilities */ + priv->caps = hypervCapsInit(priv); + if (priv->caps == NULL) { + goto cleanup; + } +

If there's only 1 line, then no need for { } This occurs multiple times in this file. You need to run "make check syntax-check" I would change to : if (!(priv->caps = hypervCapsInit(priv))) goto cleanup;

conn->privateData = priv; priv = NULL; result = VIR_DRV_OPEN_SUCCESS; @@ -1324,7 +1335,19 @@ hypervConnectListAllDomains(virConnectPtr conn, } #undef MATCH

Try to go with 2 blank lines between functions - it's the new normal for our reviews...

+static char*

static char *

+hypervConnectGetCapabilities(virConnectPtr conn) +{ + hypervPrivate *priv = conn->privateData; + char *xml = virCapabilitiesFormatXML(priv->caps);

+ if (xml == NULL) { + virReportOOMError();

This should be unnecessary. If there was a failure to memory allocation it would already be messaged. If there was some other failure, then you're overwriting that error.

+ return NULL; + }

So this simply becomes: return virCapabilitiesFormatXML(priv->caps);

+ + return xml; +}

static virHypervisorDriver hypervHypervisorDriver = { @@ -1361,8 +1384,89 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainHasManagedSaveImage = hypervDomainHasManagedSaveImage, /* 0.9.5 */ .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */ .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */ + .connectGetCapabilities = hypervConnectGetCapabilities, /* 1.2.10 */

s/1.2.10/2.3.0/ (at the very earliest)

};

blank line

+/* Retrieves host system UUID */ +static int +hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) +{ + Win32_ComputerSystemProduct *computerSystem = NULL; + virBuffer query = VIR_BUFFER_INITIALIZER; + int result = -1; + + virBufferAddLit(&query, WIN32_COMPUTERSYSTEMPRODUCT_WQL_SELECT); + + if (hypervGetWin32ComputerSystemProductList(priv, &query, &computerSystem) < 0) { + goto cleanup; + }

Unnecessary {}

+ + if (computerSystem == NULL) { + virReportError(VIR_ERR_NO_DOMAIN,

You need to a "%s, " e.g.: virReportError(VIR_ERR_NO_DOMAIN, "%s", (something syntax-check would complain about)

+ _("Unable to get Win32_ComputerSystemProduct")); + goto cleanup; + } + + if (virUUIDParse(computerSystem->data->UUID, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + computerSystem->data->UUID); + goto cleanup; + } + + result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); + + return result; +} + +

Only 2 blank lines necessary

+ +static virCapsPtr hypervCapsInit(hypervPrivate *priv)

static virCapsPtr hypervCapsInit(hypervPrivate *priv)

+{ + virCapsPtr caps = NULL; + virCapsGuestPtr guest = NULL; + + caps = virCapabilitiesNew(VIR_ARCH_X86_64, 1, 1); + + if (caps == NULL) { + virReportOOMError(); + return NULL; + }

Unnecessary OOM if (!(caps = virCapabilitiesNew(VIR_ARCH_X86_64, 1, 1))) return NULL;

+ + /* virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); */ + + if (hypervLookupHostSystemBiosUuid(priv,caps->host.host_uuid) < 0) {

s/priv,caps/priv, caps/ Need a space between args - syntax-check complaint.

+ goto failure; + }

Unnecessary {} s/failure/error/

+ + /* i686 */ + guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_I686, NULL, NULL, 0, NULL); + if (guest == NULL) { + goto failure; + } + if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV, NULL, NULL, 0, NULL) == NULL) { + goto failure; + } + + /* x86_64 */ + guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_X86_64, NULL, NULL, 0, NULL); + if (guest == NULL) { + goto failure; + } + if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV, NULL, NULL, 0, NULL) == NULL) { + goto failure; + }

These are all really long lines... They all fail syntax-check due to {} issues... Try to have each line be 80 columns, consider if (!(guest = virCapabilitiesAddGuest(...))) goto error; if (!

+ + return caps; + + failure:

We use error for error labels not failure John

+ virObjectUnref(caps); + return NULL; +}

static void diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 574bb5f..d9aa0bd 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -26,6 +26,7 @@ # include "internal.h" # include "virerror.h" # include "hyperv_util.h" +# include "capabilities.h" # include "openwsman.h"

typedef struct _hypervPrivate hypervPrivate; @@ -33,6 +34,7 @@ typedef struct _hypervPrivate hypervPrivate; struct _hypervPrivate { hypervParsedUri *parsedUri; WsManClient *client; + virCapsPtr caps; };

#endif /* __HYPERV_PRIVATE_H__ */

On Tue, 2016-08-09 at 12:39 +0000, Jason Miesionczek wrote:

---  src/hyperv/hyperv_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++  1 file changed, 52 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 4a5e80d..b2d1abf 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1349,6 +1349,57 @@ hypervConnectGetCapabilities(virConnectPtr conn)      return xml;  }   +static int +hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ +    int result = -1; +    hypervPrivate *priv = conn->privateData; +    CIM_DataFile  *datafile = NULL; +    virBuffer query = VIR_BUFFER_INITIALIZER; +    char *p; + +    virBufferAddLit(&query, " Select * from CIM_DataFile where Name='c:\\\\windows\\\\system32\\\\vmms.exe' ");

This hard-coded path might not be correct on some systems. I'd pull ir from Win32_OpereratingSystem->WindowsDirectory: https://msdn.microsoft.com/en-us/library/aa394239(v=vs.85).aspx

+    if (hypervGetCIMDataFileList(priv, &query, &datafile) < 0) { +        goto cleanup; +    } + +    /* Check the result of convertion */ +    if (datafile == NULL) { +        virReportError(VIR_ERR_INTERNAL_ERROR, +                       _("Could not lookup %s for domain %s"), +                       "Msvm_VirtualSystemSettingData",

Do you mean CIM_DataFile here?

+                       datafile->data->Version);

If datafile is NULL then you can't print version like that...

+        goto cleanup; +    } + +    /* Delete release number and last digit of build number 1.1.111x.xxxx */ +    p = strrchr(datafile->data->Version,'.'); +    if (p == NULL) { +        virReportError(VIR_ERR_INTERNAL_ERROR, +                       _("Could not parse version number from '%s'"), +                       datafile->data->Version); +        goto cleanup; +    } +    p--; +    *p = '\0'; + +    /* Parse Version String to Long */ +    if (virParseVersionString(datafile->data->Version, +                              version, true) < 0) { +        virReportError(VIR_ERR_INTERNAL_ERROR, +                       _("Could not parse version number from '%s'"), +                       datafile->data->Version); +        goto cleanup; +    } + +    result = 0; + + cleanup: +    hypervFreeObject(priv, (hypervObject *)datafile); +    virBufferFreeAndReset(&query); + +    return result; +}    static virHypervisorDriver hypervHypervisorDriver = {      .name = "Hyper-V", @@ -1385,6 +1436,7 @@ static virHypervisorDriver hypervHypervisorDriver = {      .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */      .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */      .connectGetCapabilities = hypervConnectGetCapabilities, /* 1.2.10 */ +    .connectGetVersion = hypervConnectGetVersion, /* 1.2.10 */  };    /* Retrieves host system UUID  */

2016-08-09 14:39 GMT+02:00 Jason Miesionczek <jmiesionczek@datto.com>:

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;

These types are not properly named. The all need a hyperv prefix and should not have _t suffixed.

+ int nb_params; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + + 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); + + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &queryVssd, &virtualSystemSettingData) < 0) + 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;

Why do you do a VIR_ALLOC_N here? You know the number of elements at compile time and the array doesn't have to life after the call to hypervDomainSetAutostart. You can turn this into a stack allocation. Also, as pointed out earlier, the names should be converted from strings to enum/int values. Those can then be used for an O(1) index based lookup instead of an O(n) string based lookup. -- Matthias Bolte http://photron.blogspot.com

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/Makefile.am | 1 + src/hyperv/hyperv_driver.c | 2 + src/hyperv/hyperv_network_driver.c | 124 +++++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_network_driver.h | 30 +++++++++ src/hyperv/hyperv_wmi.c | 39 ++++++++++++ src/hyperv/hyperv_wmi.h | 17 +++++ 6 files changed, 213 insertions(+) create mode 100644 src/hyperv/hyperv_network_driver.c create mode 100644 src/hyperv/hyperv_network_driver.h

Hopefully by this point in the review I've shown the "need" to run "syntax-check" - I'm not going to point out those things for the rest of the review as my time to spend on this is getting shorter. The "consistency" in formatting makes it easier to review/read libvirt code regardless of the subsystem. Many people use 80 column displays, so keeping your lines within that limit is beneficial.

diff --git a/src/Makefile.am b/src/Makefile.am index 922b9b8..d03e6b0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -888,6 +888,7 @@ ESX_DRIVER_EXTRA_DIST = \ HYPERV_DRIVER_SOURCES = \ hyperv/hyperv_private.h \ hyperv/hyperv_driver.c hyperv/hyperv_driver.h \ + hyperv/hyperv_network_driver.c hyperv/hyperv_network_driver.h \ hyperv/hyperv_util.c hyperv/hyperv_util.h \ hyperv/hyperv_wmi.c hyperv/hyperv_wmi.h \ hyperv/hyperv_wmi_classes.c hyperv/hyperv_wmi_classes.h \ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index aea7837..4c094e7 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -30,6 +30,7 @@ #include "virlog.h" #include "viruuid.h" #include "hyperv_driver.h" +#include "hyperv_network_driver.h" #include "hyperv_private.h" #include "hyperv_util.h" #include "hyperv_wmi.h" @@ -1853,6 +1854,7 @@ hypervDebugHandler(const char *message, debug_level_e level,

static virConnectDriver hypervConnectDriver = { .hypervisorDriver = &hypervHypervisorDriver, + .networkDriver = &hypervNetworkDriver, };

int diff --git a/src/hyperv/hyperv_network_driver.c b/src/hyperv/hyperv_network_driver.c new file mode 100644 index 0000000..00037ae --- /dev/null +++ b/src/hyperv/hyperv_network_driver.c @@ -0,0 +1,124 @@ +/* + * hyperv_network_driver.c: network driver functions for managing + * Microsoft Hyper-V host networks + * + * Copyright (C) 2011 Matthias Bolte <matthias.bolte@googlemail.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "internal.h" +#include "virerror.h" +#include "datatypes.h" +#include "viralloc.h" +#include "viruuid.h" +#include "virstring.h" +#include "hyperv_network_driver.h" +#include "hyperv_wmi.h" +#include "network_conf.h" + +#define VIR_FROM_THIS VIR_FROM_HYPERV + +static virNetworkPtr +hypervNetworkLookupByName(virConnectPtr conn, const char *name) +{ + virNetworkPtr network = NULL; + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where Description = \"%s\" and ElementName = \"%s\"", + "Microsoft Virtual Switch", name); + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitch) < 0) { + goto cleanup; + } + if (virtualSwitch == NULL) { + virReportError(VIR_ERR_NO_NETWORK, + _("No network found with name %s"), name); + goto cleanup; + } + + hypervMsvmVirtualSwitchToNetwork(conn, virtualSwitch, &network); + + cleanup: + hypervFreeObject(priv, (hypervObject *) virtualSwitch); + virBufferFreeAndReset(&query); + + return network; +} + +static char * +hypervNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) +{ + char *xml = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = network->conn->privateData; + virNetworkDefPtr def = NULL; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + /* Flags checked by virNetworkDefFormat */ + + if (VIR_ALLOC(def) < 0) { + virReportOOMError();

unnecessary OOM - that's done for you as part of VIR_ALLOC failure

+ goto cleanup; + } + + virUUIDFormat(network->uuid, uuid_string); + + /* Get Msvm_VirtualSwitch */ + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string); + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitch) < 0) { + goto cleanup; + } + if (virtualSwitch == NULL) { + virReportError(VIR_ERR_NO_NETWORK, + _("No network found with UUID %s"), uuid_string); + goto cleanup; + } + + /* Fill struct */ + if (virUUIDParse(virtualSwitch->data->Name, def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + virtualSwitch->data->Name); + return NULL; + } + + if (VIR_STRDUP(def->name, virtualSwitch->data->ElementName) < 0) + goto cleanup; + + xml = virNetworkDefFormat(def, flags); + + cleanup: + virNetworkDefFree(def); + hypervFreeObject(priv, (hypervObject *)virtualSwitch); + virBufferFreeAndReset(&query); + + return xml; +} + + + +virNetworkDriver hypervNetworkDriver = { + .name = "Hyper-V", + .networkLookupByName = hypervNetworkLookupByName, /* 1.2.10 */ + .networkGetXMLDesc = hypervNetworkGetXMLDesc, /* 1.2.10 */ +};

At least 2.3.0

diff --git a/src/hyperv/hyperv_network_driver.h b/src/hyperv/hyperv_network_driver.h new file mode 100644 index 0000000..54a4792 --- /dev/null +++ b/src/hyperv/hyperv_network_driver.h @@ -0,0 +1,30 @@ +/* + * hyperv_network_driver.h: network driver functions for managing + * Microsoft Hyper-V host networks + * + * Copyright (C) 2011 Matthias Bolte <matthias.bolte@googlemail.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __HYPERV_NETWORK_DRIVER_H__ +# define __HYPERV_NETWORK_DRIVER_H__ + +int hypervNetworkRegister(void); + +extern virNetworkDriver hypervNetworkDriver; + +#endif /* __HYPERV_NETWORK_DRIVER_H__ */ diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 51d9b43..2334c49 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1326,5 +1326,44 @@ hypervInvokeMethod(hypervPrivate *priv, invokeXmlParam *param_t, int nbParameter }

+int +hypervMsvmVirtualSwitchToNetwork(virConnectPtr conn, + Msvm_VirtualSwitch *virtualSwitch, virNetworkPtr *network) +{ + unsigned char uuid[VIR_UUID_BUFLEN]; + char *rawUuid = NULL; + char *uuidBuf; + + if (network == NULL || *network != NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + // Switch-SM-91b305f3-9a9e-408b-8b04-0ecff33aaba8-0 + // need to parse out the 'Switch-SM-' and '-0' so that its a proper UUID + rawUuid = virtualSwitch->data->Name; + uuidBuf = (char *)calloc(37, sizeof(char)); + strncpy(uuidBuf, rawUuid+10, 36);

Why not if (VIR_STRNDUP(uuidBuf, rawUuid+10, 36) < 0) What if virtualSwitch->data->Name isn't prefixed with "Switch-SM-"? What if it's not postfixed with "-0" Are these safe assumptions? BTW: There's a STRPREFIX that could probably help you out here.

+ + if (virUUIDParse(uuidBuf, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + virtualSwitch->data->Name); + free(uuidBuf);

VIR_FREE

+ return -1; + } + + *network = virGetNetwork(conn, virtualSwitch->data->ElementName, uuid); + + if (*network == NULL) { + free(uuidBuf);

VIR_FREE

+ return -1; + } + + free(uuidBuf);

VIR_FREE

+ return 0;

This is a function that should: 1. Initialize "int ret = -1;" 2. Utilize goto cleanup 3. Set ret = 0 just before the cleanup label cleanup: VIR_FREE(uuidBuf) return ret; After reading this and thinking about it, this function should be: virNetworkPtr * hypervMsvmVirtualSwitchToNetwork(virConnectPtr conn, Msvm_VirtualSwitch *virtualSwitch) { char *rawUuid = virtualSwitch->data->Name; char *uuidBuf = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; virNetworkPtr network = NULL; if (strlen(rawUuid) != 40) { virReportError(..., "unexpected length") return NULL; } if (!STRPREFIX(rawUuid, "Switch-SM-")) { virReportError(..., "unexpected prefix") return NULL; } if (STRNEQ(rawUuid+38, "-0")) { virReportError(..., "unexpected postfix") return NULL } if (VIR_STRNDUP(uuidBuf, rawUuid + 10, 36) < 0) return NULL; if (virUUIDParse(uuidBuf, uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse UUID from string '%s'"), rawUuid); goto cleanup; } network = virGetNetwork(conn, virtualSwitch->data->ElementName, uuid); cleanup: VIR_FREE(uuidBuf); return network; } John

+} + +

#include "hyperv_wmi.generated.c" diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index c14d229..67f45fb 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -172,6 +172,23 @@ int hypervMsvmComputerSystemFromDomain(virDomainPtr domain, Msvm_ComputerSystem **computerSystem);

+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Msvm_VirtualSwitch + */ + +int hypervMsvmVirtualSwitchToNetwork(virConnectPtr conn, + Msvm_VirtualSwitch *virtualSwitch, virNetworkPtr *network); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * Msvm_VirtualSwitch + */ + +int hypervMsvmVirtualSwitchToNetwork(virConnectPtr conn, + Msvm_VirtualSwitch *virtualSwitch, virNetworkPtr *network); + +

# include "hyperv_wmi.generated.h"

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 4c094e7..2623916 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1704,6 +1704,39 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart) return result; }

+static int +hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) +{ + int result = -1; + hypervPrivate *priv = domain->conn->privateData; + Msvm_ComputerSystem *computerSystem = NULL; + bool in_transition = false; + + virCheckFlags(0, -1); + + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { + goto cleanup; + } + + if (!hypervIsMsvmComputerSystemActive(computerSystem, &in_transition) || in_transition) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Domain is not active or is in state transition")); + goto cleanup; + } + + result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); + + cleanup: + hypervFreeObject(priv, (hypervObject *) computerSystem); + return result; +}

Beyond the long lines - it strikes me that shouldn't a DomainShutdown do some sort of shutdown? From just the name, it would seem that hypervInvokeMsvmComputerSystemRequestStateChange doesn't do that shutdown, but rather inhibits changing state. Functionally, it's just not clear this is doing all it needs to do.

+ +static int +hypervDomainShutdown(virDomainPtr dom) +{ + return hypervDomainShutdownFlags(dom, 0); +} + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -1747,6 +1780,8 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainGetVcpus = hypervDomainGetVcpus, /* 1.2.10 */ .domainSetAutostart = hypervDomainSetAutostart, /* 1.2.10 */ .domainGetAutostart = hypervDomainGetAutostart, /* 1.2.10 */ + .domainShutdownFlags = hypervDomainShutdownFlags, /* 1.2.10 */ + .domainShutdown = hypervDomainShutdown, /* 1.2.10 */

2.3.0 at the earliest John

};

/* Retrieves host system UUID */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 127 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2623916..348b39c 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1737,6 +1737,130 @@ hypervDomainShutdown(virDomainPtr dom) return hypervDomainShutdownFlags(dom, 0); }

+static int +hypervDomainGetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, + int *nparams, unsigned int flags) +{ + hypervPrivate *priv = dom->conn->privateData; + Msvm_ComputerSystem *computerSystem = NULL; + Msvm_ProcessorSettingData *processorSettingData = NULL; + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + virBuffer query = VIR_BUFFER_INITIALIZER; + int saved_nparams = 0; + int result = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG |VIR_TYPED_PARAM_STRING_OKAY, -1); + + /* We don't return strings, and thus trivially support this flag */ + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + + virUUIDFormat(dom->uuid, uuid_string); + + /* Get Msvm_ComputerSystem */ + if (hypervMsvmComputerSystemFromDomain(dom, &computerSystem) < 0) { + goto cleanup; + } + + /* Get Msvm_VirtualSystemSettingData */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid_string); + + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingData) < 0) { + goto cleanup; + } + + if (virtualSystemSettingData == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not lookup %s for domain %s"), + "Msvm_VirtualSystemSettingData", + computerSystem->data->ElementName); + goto cleanup; + } + + /* Get Msvm_ProcessorSettingData */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_ProcessorSettingData", + virtualSystemSettingData->data->InstanceID); + + if (hypervGetMsvmProcessorSettingDataList(priv, &query, &processorSettingData) < 0) { + goto cleanup; + } + + if (processorSettingData == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for domain %s"), + "Msvm_ProcessorSettingData",computerSystem->data->ElementName); + goto cleanup; + } +

There's no other parameters? Strange to me I guess, but it would seem there could be others...

+ if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_LIMIT, + VIR_TYPED_PARAM_LLONG, processorSettingData->data->Limit) < 0) + goto cleanup; + saved_nparams++; + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[1],VIR_DOMAIN_SCHEDULER_RESERVATION, + VIR_TYPED_PARAM_LLONG, processorSettingData->data->Reservation) < 0) + goto cleanup; + saved_nparams++; + } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[2],VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, processorSettingData->data->Weight) < 0) + goto cleanup; + saved_nparams++; + } + + *nparams = saved_nparams; + + result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); + hypervFreeObject(priv, (hypervObject *)processorSettingData); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervDomainGetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params, int *nparams) +{ + return hypervDomainGetSchedulerParametersFlags(dom, params, nparams, VIR_DOMAIN_AFFECT_CURRENT); +} + + + +static char*

char *

+hypervDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED, int *nparams) +{ + char *type; + + if (VIR_STRDUP(type, "allocation") < 0) { + virReportOOMError();

Unnecessary OOM

+ return NULL; + }

Or ignore_value(VIR_STRDUP(type, "allocation)); if (nparams) *params = 3; /* reservation, limit, weight */ return type;

+ + if (nparams != NULL) { + *nparams = 3; /* reservation, limit, weight */ + } + + return type; +} + + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -1782,6 +1906,9 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainGetAutostart = hypervDomainGetAutostart, /* 1.2.10 */ .domainShutdownFlags = hypervDomainShutdownFlags, /* 1.2.10 */ .domainShutdown = hypervDomainShutdown, /* 1.2.10 */ + .domainGetSchedulerParametersFlags = hypervDomainGetSchedulerParametersFlags, /* 1.2.10 */ + .domainGetSchedulerParameters = hypervDomainGetSchedulerParameters, /* 1.2.10 */ + .domainGetSchedulerType = hypervDomainGetSchedulerType, /* 1.2.10 */

2.3.0 at the earliest Trying my best not to mention long lines, {} usage, but it's hard John

};

/* Retrieves host system UUID */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 232 ++++++++++++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_private.h | 2 + 2 files changed, 234 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 348b39c..daae371 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -56,6 +56,9 @@ hypervFreePrivate(hypervPrivate **priv) if ((*priv)->caps != NULL) virObjectUnref((*priv)->caps);

+ if ((*priv)->xmlopt != NULL) + virObjectUnref((*priv)->xmlopt); + hypervFreeParsedUri(&(*priv)->parsedUri); VIR_FREE(*priv); } @@ -195,6 +198,9 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, goto cleanup; }

+ /* Init xmlopt to parse Domain XML */ + priv->xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL); +

Strange to see NULL, NULL, NULL No MAC prefix needs? No desire/need to have post parse callbacks? IOW: Things to check after all the parsing is done so that you can verify things work together? Guess I'm unclear why this is being done here. Looking forward - I see patch 14 makes use of it, so that's where it should be moved to.

conn->privateData = priv; priv = NULL; result = VIR_DRV_OPEN_SUCCESS; @@ -1860,6 +1866,229 @@ hypervDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED, int *nparams) return type; }

+/* Format a number as a string value */ +static char *num2str(unsigned long value) +{ + int sz; + char *result; + + sz = snprintf (NULL, 0, "%lu", value); + if (VIR_ALLOC_N(result, sz + 1) < 0) { + return NULL; + } + + sprintf(result, "%lu", value); + return result;

open coding virAsprintf() I think...

+} + + + +static int +hypervDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) +{ + int result = -1; + invokeXmlParam *params = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + properties_t *tab_props = NULL; + virBuffer query = VIR_BUFFER_INITIALIZER; + virBuffer query2 = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; + Msvm_MemorySettingData *memorySettingData = NULL; + eprParam eprparam; + embeddedParam embeddedparam; + int nb_params; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + unsigned long memory_mb = memory/1024; + char *memory_str = NULL; + + /* Memory value must be a multiple of 2 MB; round up it accordingly if necessary */ + if (memory_mb % 2) memory_mb++;

There's VIR_ROUND_UP or VIR_ROUND_UP_POWER_OF_TWO Not sure if you meant power of 2 instead multiple of 2MB

+ + /* Convert the memory value as a string */ + memory_str = num2str(memory_mb); + if (memory_str == NULL) + goto cleanup;

if (virAsprintf(memory_str, "%lu", memory_mb) < 0) goto cleanup;

+ + virUUIDFormat(domain->uuid, uuid_string); + + VIR_DEBUG("memory=%sMb, uuid=%s", memory_str, 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 1 */ + /* Get Msvm_VirtualSystemSettingData */ + virBufferAsprintf(&query2, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid_string); + + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query2, &virtualSystemSettingData) < 0) + goto cleanup; + + /* Get Msvm_MemorySettingData */ + virBufferFreeAndReset(&query2); + virBufferAsprintf(&query2, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_MemorySettingData", + virtualSystemSettingData->data->InstanceID); + + if (hypervGetMsvmMemorySettingDataList(priv, &query2, &memorySettingData) < 0) + goto cleanup; + + embeddedparam.nbProps = 2; + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0) + goto cleanup; + (*tab_props).name = "Limit"; + (*tab_props).val = memory_str; + (*(tab_props+1)).name = "InstanceID"; + (*(tab_props+1)).val = memorySettingData->data->InstanceID; + embeddedparam.instanceName = "Msvm_MemorySettingData"; + embeddedparam.prop_t = tab_props; + embeddedparam.nbProps = 2; + + /* Create invokeXmlParam */ + 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 = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam; +

More of these strange constructs *tab_props, *params... consider tab_props[0]-> tab_props[1]-> params[0]-> params[1]->

+ result = hypervInvokeMethod(priv, params, nb_params, "ModifyVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector); + + cleanup: + VIR_FREE(tab_props); + VIR_FREE(params); + VIR_FREE(memory_str); + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); + hypervFreeObject(priv, (hypervObject *)memorySettingData); + virBufferFreeAndReset(&query); + virBufferFreeAndReset(&query2); + + return result; +} +

Since these two are so similar - check out what the qemu driver code does: static int qemuDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) { return qemuDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM); } Then in qemuDomainSetMemoryFlags, there's a check: if (flags & VIR_DOMAIN_MEM_MAXIMUM) { } else { } Of course there's also an extra flags check for VIR_DOMAIN_MEM_MAXIMUM in qemuDomainSetMemoryFlags It's up to you though to determine how similar and whether any merging of the two could be done...

+static int +hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, + unsigned int flags ATTRIBUTE_UNUSED) +{ + int result = -1, nb_params; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + invokeXmlParam *params = NULL; + properties_t *tab_props = NULL; + eprParam eprparam; + embeddedParam embeddedparam; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; + Msvm_MemorySettingData *memorySettingData = NULL; + unsigned long memory_mb = memory / 1024; /* Memory converted in MB */ + char *memory_str = NULL; + + /* Memory value must be a multiple of 2 MB; round up it accordingly if necessary */ + if (memory_mb % 2) memory_mb++; + + /* Convert the memory value as a string */ + memory_str = num2str(memory_mb); + if (memory_str == NULL) + goto cleanup;

Simi

+ + virUUIDFormat(domain->uuid, uuid_string); + + VIR_DEBUG("memory=%sMb, uuid=%s", memory_str, uuid_string); + + /* Get Msvm_VirtualSystemSettingData */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid_string); + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingData) < 0) + goto cleanup; + + /* Get Msvm_MemorySettingData */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_MemorySettingData", + virtualSystemSettingData->data->InstanceID); + if (hypervGetMsvmMemorySettingDataList(priv, &query, &memorySettingData) < 0) + goto cleanup; + + /* Prepare EPR param */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"",uuid_string); + eprparam.query = &query; + eprparam.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare EMBEDDED param */ + embeddedparam.nbProps = 2; + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0) + goto cleanup; + (*tab_props).name = "VirtualQuantity"; + (*tab_props).val = memory_str; + (*(tab_props+1)).name = "InstanceID"; + (*(tab_props+1)).val = memorySettingData->data->InstanceID; + embeddedparam.instanceName = "Msvm_MemorySettingData"; + embeddedparam.prop_t = tab_props; + + /* Create invokeXmlParam */ + 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 = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam; + + if (hypervInvokeMethod(priv, params, nb_params, "ModifyVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not set domain memory")); + goto cleanup; + } + + result = 0; + + cleanup: + VIR_FREE(tab_props); + VIR_FREE(params); + VIR_FREE(memory_str); + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); + hypervFreeObject(priv, (hypervObject *)memorySettingData); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervDomainSetMemory(virDomainPtr domain, unsigned long memory) +{ + return hypervDomainSetMemoryFlags(domain, memory, 0); +}

static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", @@ -1909,6 +2138,9 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainGetSchedulerParametersFlags = hypervDomainGetSchedulerParametersFlags, /* 1.2.10 */ .domainGetSchedulerParameters = hypervDomainGetSchedulerParameters, /* 1.2.10 */ .domainGetSchedulerType = hypervDomainGetSchedulerType, /* 1.2.10 */ + .domainSetMaxMemory = hypervDomainSetMaxMemory, /* 1.2.10 */ + .domainSetMemory = hypervDomainSetMemory, /* 1.2.10 */ + .domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* 1.2.10 */

2.3.0 at the earliest

};

/* Retrieves host system UUID */ diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index d9aa0bd..2dfce6e 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -27,6 +27,7 @@ # include "virerror.h" # include "hyperv_util.h" # include "capabilities.h" +# include "domain_conf.h"

Will this also need to move to patch 14?

# include "openwsman.h"

typedef struct _hypervPrivate hypervPrivate; @@ -35,6 +36,7 @@ struct _hypervPrivate { hypervParsedUri *parsedUri; WsManClient *client; virCapsPtr caps; + virDomainXMLOptionPtr xmlopt;

Move to patch 14 John

};

#endif /* __HYPERV_PRIVATE_H__ */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 117 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index daae371..db59ce1 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2090,6 +2090,121 @@ hypervDomainSetMemory(virDomainPtr domain, unsigned long memory) return hypervDomainSetMemoryFlags(domain, memory, 0); }

+ +static int +hypervDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, + unsigned int flags ATTRIBUTE_UNUSED) +{ + int result = -1; + invokeXmlParam *params = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + properties_t *tab_props = NULL; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; + Msvm_ProcessorSettingData *processorSettingData = NULL; + eprParam eprparam; + embeddedParam embeddedparam; + int nb_params; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + char *nvcpus_str = NULL; + + /* Convert nvcpus as a string value */ + nvcpus_str = num2str(nvcpus); + if (nvcpus_str == NULL) + goto cleanup;

Again virAsprintf()... and your indents are off

+ + virUUIDFormat(domain->uuid, uuid_string); + + VIR_DEBUG("nvcpus=%s, uuid=%s", nvcpus_str, uuid_string); + + /* Get Msvm_VirtualSystemSettingData */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid_string);

The more I see these the more I wonder if there couldn't be a way to "merge" the syntax in a common function which could take uncommon parameters for specific fields... Then those would be called properly and errors handled... IOW: Don't cut-n-paste - create a generic function to formulate the queries...

+ + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingData) < 0) + goto cleanup;

The common code could even include this an be called such as: if (!(vssd = hypervGetMsvmVSSDList(priv, uuid_string, xxx))) where xxx would be required only if the CreationClassName, AssocClass, or ResultClass would changed - and be name appropriately using constants.

+ + /* Get Msvm_ProcessorSettingData */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_ProcessorSettingData", + virtualSystemSettingData->data->InstanceID); + + if (hypervGetMsvmProcessorSettingDataList(priv, &query, &processorSettingData) < 0) + goto cleanup; + + if (processorSettingData == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not lookup Msvm_ProcessorSettingData for domain %s"), + virtualSystemSettingData->data->ElementName); + goto cleanup; + }

Same goes here - some sort of helper function would be *most* advantageous for review and the code bloat reduction act.

+ + /* Prepare EPR param */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"",uuid_string); + eprparam.query = &query; + eprparam.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare EMBEDDED param */ + embeddedparam.nbProps = 2; + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0) + goto cleanup; + (*tab_props).name = "VirtualQuantity"; + (*tab_props).val = nvcpus_str; + (*(tab_props+1)).name = "InstanceID"; + (*(tab_props+1)).val = processorSettingData->data->InstanceID; + embeddedparam.instanceName = "Msvm_ProcessorSettingData"; + embeddedparam.prop_t = tab_props; + + /* Create invokeXmlParam */ + 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 = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam; +

Ewww *tab_probs, *params* Something tells me this sequence could also use a good helper routine. I just seems there's too much cut-n-paste or repetitive code...

+ if (hypervInvokeMethod(priv, params, nb_params, "ModifyVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not set domain vcpus")); + goto cleanup; + } + + result = 0; + + cleanup: + VIR_FREE(tab_props); + VIR_FREE(params); + VIR_FREE(nvcpus_str); + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); + hypervFreeObject(priv, (hypervObject *)processorSettingData); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) +{ + return hypervDomainSetVcpusFlags(domain, nvcpus, 0); +} + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -2141,6 +2256,8 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainSetMaxMemory = hypervDomainSetMaxMemory, /* 1.2.10 */ .domainSetMemory = hypervDomainSetMemory, /* 1.2.10 */ .domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* 1.2.10 */ + .domainSetVcpus = hypervDomainSetVcpus, /* 1.2.10 */ + .domainSetVcpusFlags = hypervDomainSetVcpusFlags, /* 1.2.10 */

2.3.0 at the earliest John

};

/* Retrieves host system UUID */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index db59ce1..bd028ed 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2205,6 +2205,76 @@ hypervDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) return hypervDomainSetVcpusFlags(domain, nvcpus, 0); }

+ +static int +hypervDomainUndefineFlags(virDomainPtr domain, unsigned int flags ATTRIBUTE_UNUSED) +{ + int result = -1, nb_params; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + invokeXmlParam *params = NULL; + eprParam eprparam; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_ComputerSystem *computerSystem = NULL; + + virCheckFlags(0, -1); + + virUUIDFormat(domain->uuid, uuid_string); + + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { + goto cleanup; + } + + /* Shutdown the VM if not disabled */ + if (computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED) { + if (hypervDomainShutdown(domain) < 0) { + goto cleanup; + } + } +

This seems to go against the virDomainUndefineFlags API description: * Undefine a domain. If the domain is running, it's converted to * transient domain, without stopping it. If the domain is inactive, * the domain configuration is removed. besides it wasn't clear to me earlier if Shutdown did anything.

+ /* Deleting the VM */ + + /* Prepare EPR param */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string); + eprparam.query = &query; + eprparam.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Create invokeXmlParam tab */ + nb_params = 1; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "ComputerSystem"; + (*params).type = EPR_PARAM; + (*params).param = &eprparam;

Like before is this a repeatable sequence with something else..

+ + /* Destroy VM */ + if (hypervInvokeMethod(priv, params, nb_params, "DestroyVirtualSystem", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not delete domain")); + goto cleanup; + } + + result = 0; + + cleanup: + VIR_FREE(params); + hypervFreeObject(priv, (hypervObject *) computerSystem); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervDomainUndefine(virDomainPtr domain) +{ + return hypervDomainUndefineFlags(domain, 0); +} + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -2258,6 +2328,8 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* 1.2.10 */ .domainSetVcpus = hypervDomainSetVcpus, /* 1.2.10 */ .domainSetVcpusFlags = hypervDomainSetVcpusFlags, /* 1.2.10 */ + .domainUndefine = hypervDomainUndefine, /* 1.2.10 */ + .domainUndefineFlags = hypervDomainUndefineFlags, /* 1.2.10 */

2.3.0 at the earliest John

};

/* Retrieves host system UUID */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_driver.c | 807 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 807 insertions(+)

This is where the optxml first shows up and where it should be defined. Although I'm still a bit unclear why it's being used with the NULL parameters...

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index bd028ed..716fadb 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2275,6 +2275,809 @@ hypervDomainUndefine(virDomainPtr domain) return hypervDomainUndefineFlags(domain, 0); }

+/* + * Creates the attribute __PATH for the RASD object + * The attribute is build like this:

s/build/built

+ * \\<host_name>\root\virtualization:Msvm_ResourceAllocationSettingData.InstanceID="<rasdInstanceID>" + * where backslashes in rasdInstanceID are doubled + */ +static int +hypervGetResourceAllocationSettingDataPATH(virDomainPtr domain, char *rasdInstanceID, char **__path) +{ + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_ComputerSystem *computerSystem = NULL; + char *strTemp = NULL; + int result = -1, i = 0, j = 0, n = 0; + + virUUIDFormat(domain->uuid, uuid_string); + + /* Get host name */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_HostedDependency " + "ResultClass = Msvm_ComputerSystem", + uuid_string); + if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) { + goto cleanup; + } + if (computerSystem == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, _("No domain with UUID %s"), uuid_string); + goto cleanup; + }

Repeatable sequence (comment from earlier patch 12 I think)

+ + /* Count the number of backslash character */ + strTemp = strchr(rasdInstanceID, '\\'); + while (strTemp != NULL) { + n++; + strTemp = strchr(++strTemp, '\\'); + } + /* Double the blackslashes */ + if (VIR_ALLOC_N(strTemp, strlen(rasdInstanceID) + 1 + n) < 0) + goto cleanup; + while (rasdInstanceID[i] != '\0') { + strTemp[j] = rasdInstanceID[i]; + if (rasdInstanceID[i] == '\\') { + j++; + strTemp[j] = '\\'; + } + i++; + j++; + } + strTemp[j] = '\0';

I think you may want to look at virBufferEscape

+ + /* Create the attribute __PATH */ + /* FIXME: *__path allocated with 255 characters (static value) */ + if (VIR_ALLOC_N(*__path, 255) < 0) + goto cleanup; + sprintf(*__path, "\\\\"); + strcat(*__path, computerSystem->data->ElementName); + strcat(*__path, "\\root\\virtualization:Msvm_ResourceAllocationSettingData.InstanceID=\""); + strcat(*__path, strTemp); + strcat(*__path, "\"");

Why not return __path instead of passing char **_path and returning where the caller would then being able succeed/fail based on NULL return. Perhaps even virAsprintf will help you out with your quadary. There's even a virBufferStrcat

+ + result = 0; + + cleanup: + VIR_FREE(strTemp); + hypervFreeObject(priv, (hypervObject *)computerSystem); + virBufferFreeAndReset(&query); + + return result; +} + + + +/* hypervDomainAttachDisk + * FIXME: + * - added ressources must me removed in case of error + * - allow attaching disks on iSCSI (implemented only on IDE) + * - allow attaching ISO images (on DVD devices) + * - implement associated detach method + */ +ATTRIBUTE_UNUSED static int +hypervDomainAttachDisk(virDomainPtr domain, virDomainDiskDefPtr disk) +{ + int result = -1, nb_params; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + char *ideRasdPath = NULL, *newDiskDrivePath = NULL; + char ideControler[2], ideControlerAddr[2];

A type A person would tell you it's "controller" ;-)

+ hypervPrivate *priv = domain->conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData = NULL; + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData2 = NULL; + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData3 = NULL; + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData4 = NULL; + Msvm_ResourceAllocationSettingData *ideRasd = NULL; /* resourceAllocationSettingData subtree -> do not disallocate */ + Msvm_ResourceAllocationSettingData *diskRasd = NULL; /* resourceAllocationSettingData2 subtree -> do not disallocate */ + Msvm_ResourceAllocationSettingData *newDiskDrive = NULL; /* resourceAllocationSettingData3 subtree -> do not disallocate */ + Msvm_AllocationCapabilities *allocationCapabilities = NULL; + Msvm_AllocationCapabilities *allocationCapabilities2 = NULL; + invokeXmlParam *params = NULL; + properties_t *tab_props = NULL; + eprParam eprparam1, eprparam2; + embeddedParam embeddedparam1, embeddedparam2; + + /* Initialization */ + virUUIDFormat(domain->uuid, uuid_string); + + /* Set IDE Controler 0 or 1 and address 0 or 1 */ + if (STREQ(disk->dst, "hda")) { + sprintf(ideControler, "%d", 0); + sprintf(ideControlerAddr, "%d", 0); + } else if (STREQ(disk->dst, "hdb")) { + sprintf(ideControler, "%d", 0); + sprintf(ideControlerAddr, "%d", 1); + } else if (STREQ(disk->dst, "hdc")) { + sprintf(ideControler, "%d", 1); + sprintf(ideControlerAddr, "%d", 0); + } else if (STREQ(disk->dst, "hdd")) { + sprintf(ideControler, "%d", 1); + sprintf(ideControlerAddr, "%d", 1); + } else { + /* IDE Controler 0 and address 0 choosen by default */ + sprintf(ideControler, "%d", 0); + sprintf(ideControlerAddr, "%d", 0); + }

That's an odd set of lines to read... So no such thing as "hde" - just 4 recognized types. What if someone tries 'hda' and 'hde'?

+ + VIR_DEBUG("src=%s, dst=IDE Controller %s:%s, uuid=%s", + disk->src->path, ideControler, ideControlerAddr, uuid_string); + + /* Get the current VM settings object */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid_string); + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, &virtualSystemSettingData) < 0) { + goto cleanup; + } + + /* Get the settings for IDE Controller on the VM */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_ResourceAllocationSettingData", + virtualSystemSettingData->data->InstanceID); + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData) < 0) { + goto cleanup; + }

While sometimes it takes a few moments to figure out the acronym, it sure beats typing ResourceAllocationSettingData (or copy/cut - paste).

+ ideRasd = resourceAllocationSettingData; + while (ideRasd != NULL) { + if (ideRasd->data->ResourceType == 5 && STREQ(ideRasd->data->Address, ideControler)) {

5 is a magic number?

+ /* IDE Controller 0 or 1 */ + break; + } + ideRasd = ideRasd->next; + } + if (ideRasd == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find IDE Controller %s"), ideControler); + goto cleanup; + } + + /* Get the settings for 'Microsoft Synthetic Disk Drive' */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_ALLOCATIONCAPABILITIES_WQL_SELECT); + virBufferAddLit(&query, "WHERE ResourceSubType = 'Microsoft Synthetic Disk Drive'"); + if (hypervGetMsvmAllocationCapabilitiesList(priv, &query, &allocationCapabilities) < 0) { + goto cleanup; + } + + /* Get default values for 'Microsoft Synthetic Disk Drive' */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_AllocationCapabilities.InstanceID=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineCapabilities " + "ResultClass = Msvm_ResourceAllocationSettingData", + allocationCapabilities->data->InstanceID); + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData2) < 0) { + goto cleanup; + } + diskRasd = resourceAllocationSettingData2; + while (diskRasd != NULL) { + if (strstr(diskRasd->data->InstanceID, "Default") != NULL) { + /* Default values */ + break; + } + diskRasd = diskRasd->next; + } + if (diskRasd == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get default values for 'Microsoft Synthetic Disk Drive'")); + goto cleanup; + } + + /* Create the attribute _PATH for the RASD object */ + if (hypervGetResourceAllocationSettingDataPATH(domain, ideRasd->data->InstanceID, &ideRasdPath) < 0) { + goto cleanup; + } + + /* Add default disk drive */ + /* Prepare EPR param */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string); + eprparam1.query = &query; + eprparam1.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare EMBEDDED param 1 */ + embeddedparam1.nbProps = 4; + if (VIR_ALLOC_N(tab_props, embeddedparam1.nbProps) < 0) + goto cleanup; + (*tab_props).name = "Parent"; + (*tab_props).val = ideRasdPath; + (*(tab_props+1)).name = "Address"; + (*(tab_props+1)).val = ideControlerAddr; + (*(tab_props+2)).name = "ResourceType"; + (*(tab_props+2)).val = "22";

22 is a magic number

+ (*(tab_props+3)).name = "ResourceSubType"; + (*(tab_props+3)).val = diskRasd->data->ResourceSubType; + embeddedparam1.instanceName = MSVM_RESOURCEALLOCATIONSETTINGDATA_CLASSNAME; + embeddedparam1.prop_t = tab_props; +

I still cannot get used to the construct (*tab_props)

+ /* Create invokeXmlParam tab */ + nb_params = 2; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "TargetSystem"; + (*params).type = EPR_PARAM; + (*params).param = &eprparam1; + (*(params+1)).name = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam1; + + if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add default disk drive")); + goto cleanup; + } + + /* Get the instance of the new default drive disk */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_ResourceAllocationSettingData", + virtualSystemSettingData->data->InstanceID); + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData3) < 0) { + goto cleanup; + } + newDiskDrive = resourceAllocationSettingData3; + while (newDiskDrive != NULL) { + if (newDiskDrive->data->ResourceType == 22 && + STREQ(newDiskDrive->data->ResourceSubType, "Microsoft Synthetic Disk Drive") && + STREQ(newDiskDrive->data->Parent, ideRasdPath) && + STREQ(newDiskDrive->data->Address, ideControlerAddr)) { + break; + } + newDiskDrive = newDiskDrive->next; + } + if (newDiskDrive == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find 'Microsoft Synthetic Disk Drive'")); + goto cleanup; + } + + /* Get the settings for 'Microsoft Virtual Hard Disk' */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_ALLOCATIONCAPABILITIES_WQL_SELECT); + virBufferAddLit(&query, "WHERE ResourceSubType = 'Microsoft Virtual Hard Disk'"); + if (hypervGetMsvmAllocationCapabilitiesList(priv, &query, &allocationCapabilities2) < 0) { + goto cleanup; + } + + /* Get default values for 'Microsoft Virtual Hard Drive' */ + virBufferFreeAndReset(&query); + virBufferAsprintf(&query, + "associators of " + "{Msvm_AllocationCapabilities.InstanceID=\"%s\"} " + "where AssocClass = Msvm_SettingsDefineCapabilities " + "ResultClass = Msvm_ResourceAllocationSettingData", + allocationCapabilities2->data->InstanceID); + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData4) < 0) { + goto cleanup; + } + diskRasd = resourceAllocationSettingData4; + while (diskRasd != NULL) { + if (strstr(diskRasd->data->InstanceID, "Default") != NULL) { + /* Default values */ + break; + } + diskRasd = diskRasd->next; + } + if (diskRasd == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get default values for 'Microsoft Virtual Hard Drive'")); + goto cleanup; + } + + /* Create the attribute _PATH for the RASD object */ + if (hypervGetResourceAllocationSettingDataPATH(domain, newDiskDrive->data->InstanceID, &newDiskDrivePath) < 0) { + goto cleanup; + } + + /* Add the new VHD */ + /* Prepare EPR param 2 */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string); + eprparam2.query = &query; + eprparam2.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare EMBEDDED param 2 */ + VIR_FREE(tab_props); + embeddedparam2.nbProps = 4; + if (VIR_ALLOC_N(tab_props, embeddedparam2.nbProps) < 0) + goto cleanup; + (*tab_props).name = "Parent"; + (*tab_props).val = newDiskDrivePath; + (*(tab_props+1)).name = "Connection"; + (*(tab_props+1)).val = disk->src->path; + (*(tab_props+2)).name = "ResourceType"; + (*(tab_props+2)).val = "21"; + (*(tab_props+3)).name = "ResourceSubType"; + (*(tab_props+3)).val = diskRasd->data->ResourceSubType; + embeddedparam2.instanceName = MSVM_RESOURCEALLOCATIONSETTINGDATA_CLASSNAME; + embeddedparam2.prop_t = tab_props; + + /* Create invokeXmlParam tab */ + VIR_FREE(params); + nb_params = 2; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "TargetSystem"; + (*params).type = EPR_PARAM; + (*params).param = &eprparam2; + (*(params+1)).name = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam2; + + if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not attach hard disk drive")); + goto cleanup; + } + + result = 0;

Oh my eyes and brain hurt badly by this point. I think this needs to be broken up into multiple helpers 1. To do that IDE stuff 2. To do that Microsoft Synthetic Disk Drive stuff 3. To do that Microsoft Virtual Hard Disk Combining everything in one function with all those consonants and vowels is painful.

+ + cleanup: + VIR_FREE(ideRasdPath); + VIR_FREE(newDiskDrivePath); + VIR_FREE(tab_props); + VIR_FREE(params); + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData); + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData2); + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData3); + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData4); + hypervFreeObject(priv, (hypervObject *)allocationCapabilities); + hypervFreeObject(priv, (hypervObject *)allocationCapabilities2); + virBufferFreeAndReset(&query); + + return result; +} +

OK so far the above is DISK... Can we make it it's own patch separate from the following NETWORK stuff?

+ +/* + * Create the attribute __PATH for the SwitchPort object. + * The attribute is build like this: + * \\<host_name>\root\virtualization:Msvm_SwitchPort.CreationClassName="Msvm_SwitchPort", + * Name="<switchPortName>",SystemCreationClassName="Msvm_VirtualSwitch", + * SystemName="<virtualSwitchSystemName>" + */ +static int +hypervGetSwitchPortPATH(virDomainPtr domain, char *switchPortName, char *virtualSwitchSystemName, char **__path) +{ + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_ComputerSystem *computerSystem = NULL; + int result = -1; + + virUUIDFormat(domain->uuid, uuid_string); + + /* Get host name */ + virBufferAsprintf(&query, + "associators of " + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," + "Name=\"%s\"} " + "where AssocClass = Msvm_HostedDependency " + "ResultClass = Msvm_ComputerSystem", + uuid_string); + if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) { + goto cleanup; + } + if (computerSystem == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with UUID %s"), uuid_string); + goto cleanup; + } + + /* Create the attribute __PATH */ + /* FIXME: *__path is allocated with 512 characters (static value) */ + if (VIR_ALLOC_N(*__path, 512) < 0) + goto cleanup; + sprintf(*__path, + "\\\\%s\\root\\virtualization:Msvm_SwitchPort.CreationClassName=\"Msvm_SwitchPort\"," + "Name=\"%s\",SystemCreationClassName=\"Msvm_VirtualSwitch\",SystemName=\"%s\"", + computerSystem->data->ElementName, switchPortName, virtualSwitchSystemName); +

virAsprintf Holy Shinola that is one fugly construct.

+ result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *) computerSystem); + virBufferFreeAndReset(&query); + return result; +} + + + +/* hypervDomainAttachNetwork + * FIXME: + * - implement associated detach method + */ +ATTRIBUTE_UNUSED static int +hypervDomainAttachNetwork(virDomainPtr domain, virDomainNetDefPtr net) +{ + int result = -1, nb_params; + const char *selector1 = "CreationClassName=Msvm_VirtualSwitchManagementService"; + const char *selector2 = "CreationClassName=Msvm_VirtualSystemManagementService"; + char uuid_string[VIR_UUID_STRING_BUFLEN], guid_string[VIR_UUID_STRING_BUFLEN]; + unsigned char guid[VIR_UUID_BUFLEN]; + char *virtualSystemIdentifiers = NULL, *switchPortPATH = NULL; + hypervPrivate *priv = domain->conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + eprParam eprparam1, eprparam2; + simpleParam simpleparam1, simpleparam2, simpleparam3; + embeddedParam embeddedparam; + properties_t *tab_props = NULL; + invokeXmlParam *params = NULL; + Msvm_SwitchPort *switchPort = NULL; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + /* Initialization */ + virUUIDFormat(domain->uuid, uuid_string); + + VIR_DEBUG("network=%s, uuid=%s", net->data.network.name, uuid_string); + + /* Create virtual switch port */ + /* Prepare EPR param 1 */ + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where ElementName = \"%s\"", net->data.network.name); + eprparam1.query = &query; + eprparam1.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare SIMPLE params */ + virUUIDGenerate(guid);

Coverity notes, this needs to check the status like other such calls

+ virUUIDFormat(guid, guid_string); + simpleparam1.value = guid_string; + simpleparam2.value = "Dynamic Ethernet Switch Port"; + simpleparam3.value = ""; + + /* Create invokeXmlParam tab */ + nb_params = 4; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "VirtualSwitch"; + (*params).type = EPR_PARAM; + (*params).param = &eprparam1; + (*(params+1)).name = "Name"; + (*(params+1)).type = SIMPLE_PARAM; + (*(params+1)).param = &simpleparam1; + (*(params+2)).name = "FriendlyName"; + (*(params+2)).type = SIMPLE_PARAM; + (*(params+2)).param = &simpleparam2; + (*(params+3)).name = "ScopeOfResidence"; + (*(params+3)).type = SIMPLE_PARAM; + (*(params+3)).param = &simpleparam3; + + if (hypervInvokeMethod(priv, params, nb_params, "CreateSwitchPort", + MSVM_VIRTUALSWITCHMANAGEMENTSERVICE_RESOURCE_URI, selector1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not create port for virtual switch '%s'"), net->data.network.name); + goto cleanup; + } + + /* Get a reference of the switch port created previously */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_SWITCHPORT_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", guid_string); + if (hypervGetMsvmSwitchPortList(priv, &query, &switchPort) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Method hypervGetMsvmSwitchPortList failed with query=%s"), query.e); + goto cleanup; + } + if (switchPort == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get switch port with Name=%s"), guid_string); + goto cleanup; + } + + /* Get a reference of the given virtual switch */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where ElementName = \"%s\"", net->data.network.name); + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitch) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Method hypervGetMsvmVirtualSwitchList failed with query=%s"), query.e); + goto cleanup; + } + if (virtualSwitch == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get virtual switch '%s'"), net->data.network.name); + goto cleanup; + } + + /* Add the synthetic ethernet port to the VM */ + /* Prepare EPR param 2 */ + virBufferFreeAndReset(&query); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string); + eprparam2.query = &query; + eprparam2.wmiProviderURI = ROOT_VIRTUALIZATION; + + /* Prepare EMBEDDED param */ + virUUIDGenerate(guid); + virUUIDFormat(guid, guid_string); + virtualSystemIdentifiers = (char *) malloc((strlen(guid_string)+3) * sizeof(char)); + sprintf(virtualSystemIdentifiers, "{%s}", guid_string); + if (hypervGetSwitchPortPATH(domain, switchPort->data->Name, virtualSwitch->data->Name, &switchPortPATH) < 0) { + goto cleanup; + } + + embeddedparam.nbProps = 5; + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0) + goto cleanup; + (*tab_props).name = "Connection"; + (*tab_props).val = switchPortPATH; + (*(tab_props+1)).name = "ElementName"; + (*(tab_props+1)).val = "Network Adapter"; + (*(tab_props+2)).name = "VirtualSystemIdentifiers"; + (*(tab_props+2)).val = virtualSystemIdentifiers; + (*(tab_props+3)).name = "ResourceType"; + (*(tab_props+3)).val = "10"; + (*(tab_props+4)).name = "ResourceSubType"; + (*(tab_props+4)).val = "Microsoft Synthetic Ethernet Port"; + embeddedparam.instanceName = MSVM_SYNTHETICETHERNETPORTSETTINGDATA_CLASSNAME; + embeddedparam.prop_t = tab_props; + + /* Create invokeXmlParam tab */ + VIR_FREE(params); + nb_params = 2; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "TargetSystem"; + (*params).type = EPR_PARAM; + (*params).param = &eprparam2; + (*(params+1)).name = "ResourceSettingData"; + (*(params+1)).type = EMBEDDED_PARAM; + (*(params+1)).param = &embeddedparam; +

The drum broke from the continual beating...

+ if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector2) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach the network")); + goto cleanup; + } + + result = 0; + + cleanup: + VIR_FREE(virtualSystemIdentifiers); + VIR_FREE(switchPortPATH); + VIR_FREE(tab_props); + VIR_FREE(params); + hypervFreeObject(priv, (hypervObject *)switchPort); + hypervFreeObject(priv, (hypervObject *)virtualSwitch); + virBufferFreeAndReset(&query); + + return result; +} +

So the above is NETWORK and below is DEVICE - separate patches.

+ +static int +hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, + unsigned int flags ATTRIBUTE_UNUSED) +{ + int result = -1; + hypervPrivate *priv = domain->conn->privateData; + virDomainDefPtr def = NULL; + virDomainDeviceDefPtr dev = NULL; + char *xmlDomain = NULL; + + /* Get domain definition */ + if ((xmlDomain = hypervDomainGetXMLDesc(domain, 0)) == NULL) { + goto cleanup; + } + if ((def = virDomainDefParseString(xmlDomain, priv->caps, priv->xmlopt, + 1 << VIR_DOMAIN_VIRT_HYPERV | VIR_DOMAIN_XML_INACTIVE)) == NULL) { + goto cleanup; + } + + /* Get domain device definition */ + if ((dev = virDomainDeviceDefParse(xml, def, priv->caps, + priv->xmlopt, VIR_DOMAIN_XML_INACTIVE)) == NULL) { + goto cleanup; + } + + switch (dev->type) { + /* Device = disk */ + case VIR_DOMAIN_DEVICE_DISK: + if (hypervDomainAttachDisk(domain, dev->data.disk) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach disk")); + goto cleanup; + } + VIR_DEBUG("Disk attached"); + break; + + /* Device = network */ + case VIR_DOMAIN_DEVICE_NET: + if (hypervDomainAttachNetwork(domain, dev->data.net) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach network")); + goto cleanup; + } + VIR_DEBUG("Network attached"); + break; + + /* Unsupported device type */ + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Device attachment of type %d is not implemented"), dev->type); + goto cleanup; + } + + result = 0; + + cleanup: + virDomainDefFree(def); + virDomainDeviceDefFree(dev); + VIR_FREE(xmlDomain); + + return result; +} + + + +static int +hypervDomainAttachDevice(virDomainPtr domain, const char *xml) +{ + return hypervDomainAttachDeviceFlags(domain, xml, 0); +} +

Above is AttachDevice (without any DetachDevice, but let's not take that fork in the road yet)... Below is DomainDefine which is a separate patch.

+static virDomainPtr +hypervDomainDefineXML(virConnectPtr conn, const char *xml) +{ + hypervPrivate *priv = conn->privateData; + virDomainDefPtr def = NULL; + virDomainPtr domain = NULL; + invokeXmlParam *params = NULL; + properties_t *tab_props = NULL; + embeddedParam embeddedparam; + int nb_params, i; + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService"; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + + /* Parse XML domain description */ + if ((def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, + 1 << VIR_DOMAIN_VIRT_HYPERV | VIR_DOMAIN_XML_INACTIVE)) == NULL) { + goto cleanup; + } + + /* Create the domain if does not exist */ + if (def->uuid == NULL || (domain = hypervDomainLookupByUUID(conn, def->uuid)) == NULL) {

Coverity notes " (1) Event array_null: Comparing an array to null is not useful: "def->uuid == NULL", since the test will always evaluate as true. (2) Event remediation: Was "def->uuid" formerly declared as a pointer? IOW: uuid is defined as "unsigned char uuid[VIR_UUID_BUFLEN];" so def->uuid will always be true

+ /* Prepare EMBEDDED param */ + /* Edit only VM name */ + /* FIXME: cannot edit VM UUID */ + embeddedparam.nbProps = 1; + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0) + goto cleanup; + (*tab_props).name = "ElementName"; + (*tab_props).val = def->name; + embeddedparam.instanceName = "Msvm_VirtualSystemGlobalSettingData"; + embeddedparam.prop_t = tab_props; + + /* Create invokeXmlParam */ + nb_params = 1; + if (VIR_ALLOC_N(params, nb_params) < 0) + goto cleanup; + (*params).name = "SystemSettingData"; + (*params).type = EMBEDDED_PARAM; + (*params).param = &embeddedparam; + + /* Create VM */ + if (hypervInvokeMethod(priv, params, nb_params, "DefineVirtualSystem", + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not create new domain %s"), def->name); + goto cleanup; + } + + /* Get domain pointer */ + domain = hypervDomainLookupByName(conn, def->name); + + VIR_DEBUG("Domain created: name=%s, uuid=%s", + domain->name, virUUIDFormat(domain->uuid, uuid_string)); + } + + /* Set VM maximum memory */ + if (def->mem.max_memory > 0) { + if (hypervDomainSetMaxMemory(domain, def->mem.max_memory) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not set VM maximum memory")); + } + } + + /* Set VM memory */ + if (def->mem.cur_balloon > 0) { + if (hypervDomainSetMemory(domain, def->mem.cur_balloon) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not set VM memory")); + } + } + + /* Set VM vcpus */ + /*if ((int)def->vcpus > 0) {*/ + /*if (hypervDomainSetVcpus(domain, def->vcpus) < 0) {*/ + /*virReportError(VIR_ERR_INTERNAL_ERROR,*/ + /*_("Could not set VM vCPUs"));*/ + /*}*/ + /*}*/

Ugly...

+ + /* Attach networks */ + for (i = 0; i < def->nnets; i++) { + if (hypervDomainAttachNetwork(domain, def->nets[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach network")); + } + } + + /* Attach disks */ + for (i = 0; i < def->ndisks; i++) { + if (hypervDomainAttachDisk(domain, def->disks[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach disk")); + } + } +

Define generally means define the domain, save it somewhere, so that in the future we can edit/start/undefine it. This would mean some amount of persistence. Doesn't seem you're quite there yet.

+ cleanup: + virDomainDefFree(def); + VIR_FREE(tab_props); + VIR_FREE(params); + + return domain; +} + +

Having Create/Define in the same is OK, but really should separate.

+ +static virDomainPtr +hypervDomainCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) +{ + virDomainPtr domain; + + virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); + + /* Create the new domain */ + domain = hypervDomainDefineXML(conn, xmlDesc); + if (domain == NULL) + return NULL;

Hmmm... Well Create is more for a transient domains - ones that will be created and started, but not saved. I think you may need to rethink how you're handling define/create

+ + /* Start the domain */ + if (hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start the domain %s"), domain->name); + return domain; + } + + /* If the VIR_DOMAIN_START_PAUSED flag is set, + the guest domain will be started, but its CPUs will remain paused */ + if (flags & VIR_DOMAIN_START_PAUSED) { + if (hypervDomainSuspend(domain) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not pause the domain %s"), domain->name); + } + } + + /* FIXME: process autodestroy flag */ + + return domain; +} + + static virHypervisorDriver hypervHypervisorDriver = { .name = "Hyper-V", .connectOpen = hypervConnectOpen, /* 0.9.5 */ @@ -2330,6 +3133,10 @@ static virHypervisorDriver hypervHypervisorDriver = { .domainSetVcpusFlags = hypervDomainSetVcpusFlags, /* 1.2.10 */ .domainUndefine = hypervDomainUndefine, /* 1.2.10 */ .domainUndefineFlags = hypervDomainUndefineFlags, /* 1.2.10 */ + .domainAttachDevice = hypervDomainAttachDevice, /* 1.2.10 */ + .domainAttachDeviceFlags = hypervDomainAttachDeviceFlags, /* 1.2.10 */ + .domainDefineXML = hypervDomainDefineXML, /* 1.2.10 */ + .domainCreateXML = hypervDomainCreateXML, /* 1.2.10 */

2.3.0 at the earliest and will need to be separate patches as indicated above. John I've really run out of steam on this effort.

};

/* Retrieves host system UUID */

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/hyperv/hyperv_network_driver.c | 156 +++++++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+)

diff --git a/src/hyperv/hyperv_network_driver.c b/src/hyperv/hyperv_network_driver.c index 00037ae..68f03e5 100644 --- a/src/hyperv/hyperv_network_driver.c +++ b/src/hyperv/hyperv_network_driver.c @@ -115,10 +115,166 @@ hypervNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) return xml; }

+static int +hypervConnectNumOfNetworks(virConnectPtr conn) +{ + int result = -1, count = 0; + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitchList = NULL; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where HealthState = %d", 5);

5 is a magic number

+ if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitchList) < 0) { + goto cleanup; + } + + for (virtualSwitch = virtualSwitchList; virtualSwitch != NULL; + virtualSwitch = virtualSwitch->next) { + count++; + } + + result = count; + + cleanup: + hypervFreeObject(priv, (hypervObject *) virtualSwitchList); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervConnectListNetworks(virConnectPtr conn, char **const names, int maxnames) +{ + int i, count = 0; + bool success = false; + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitchList = NULL; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + if (maxnames <= 0) + return 0; + + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where HealthState = %d", 5); + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitchList) < 0) { + goto cleanup; + }

Hmmm this looks familiar - consider a helper instead of cut-copy-paste

+ + for (virtualSwitch = virtualSwitchList; virtualSwitch != NULL; + virtualSwitch = virtualSwitch->next) { + if (VIR_STRDUP(names[count], virtualSwitch->data->ElementName) < 0) { + goto cleanup; + } + count++; + if (count >= maxnames) {

Well I hope count is never > maxnames!

+ break; + } + } + + success = true; + + cleanup: + if (!success) { + for (i = 0; i < count; ++i) { + VIR_FREE(names[i]); + } + count = -1; + } + + hypervFreeObject(priv, (hypervObject *) virtualSwitchList); + virBufferFreeAndReset(&query); + + return count; +}

The hunks above and below could be separate patches, although I do see some sense in combining them.

+static int +hypervConnectNumOfDefinedNetworks(virConnectPtr conn) +{ + int result = -1, count = 0; + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitchList = NULL; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where HealthState <> %d", 5);

So defined networks don't include ones with the magic #5 I would think a "defined"

+ if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitchList) < 0) { + goto cleanup; + } + + for (virtualSwitch = virtualSwitchList; virtualSwitch != NULL; + virtualSwitch = virtualSwitch->next) { + count++; + } + + result = count; + + cleanup: + hypervFreeObject(priv, (hypervObject *) virtualSwitchList); + virBufferFreeAndReset(&query); + + return result; +} + + + +static int +hypervConnectListDefinedNetworks(virConnectPtr conn, char **const names, int maxnames) +{ + int i, count = 0; + bool success = false; + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSwitch *virtualSwitchList = NULL; + Msvm_VirtualSwitch *virtualSwitch = NULL; + + if (maxnames <= 0) + return 0; + + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT); + virBufferAsprintf(&query, "where HealthState <> %d", 5); + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitchList) < 0) { + goto cleanup; + }

Common code.

+ + for (virtualSwitch = virtualSwitchList; virtualSwitch != NULL; + virtualSwitch = virtualSwitch->next) { + if (VIR_STRDUP(names[count], virtualSwitch->data->ElementName) < 0) { + goto cleanup; + } + count++; + if (count >= maxnames) { + break; + } + } + + success = true; + + cleanup: + if (!success) { + for (i = 0; i < count; ++i) { + VIR_FREE(names[i]); + } + count = -1; + } + + hypervFreeObject(priv, (hypervObject *) virtualSwitchList); + virBufferFreeAndReset(&query); + + return count; +}

virNetworkDriver hypervNetworkDriver = { .name = "Hyper-V", .networkLookupByName = hypervNetworkLookupByName, /* 1.2.10 */ .networkGetXMLDesc = hypervNetworkGetXMLDesc, /* 1.2.10 */ + .connectNumOfNetworks = hypervConnectNumOfNetworks, /* 1.2.10 */ + .connectListNetworks = hypervConnectListNetworks, /* 1.2.10 */ + .connectNumOfDefinedNetworks = hypervConnectNumOfDefinedNetworks, /* 1.2.10 */ + .connectListDefinedNetworks = hypervConnectListDefinedNetworks, /* 1.2.10 */

2.3.0 at the earliest John

};

On 08/09/2016 08:39 AM, Jason Miesionczek wrote:

--- src/Makefile.am | 1 + src/hyperv/hyperv_driver.c | 62 ++++++- src/hyperv/hyperv_driver_2012.c | 299 ++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_driver_2012.h | 55 +++++++ src/hyperv/hyperv_private.h | 4 + src/hyperv/hyperv_wmi.h | 3 + src/hyperv/hyperv_wmi_generator.input | 35 +++- src/hyperv/hyperv_wmi_generator.py | 11 +- 8 files changed, 462 insertions(+), 8 deletions(-) create mode 100644 src/hyperv/hyperv_driver_2012.c create mode 100644 src/hyperv/hyperv_driver_2012.h

diff --git a/src/Makefile.am b/src/Makefile.am index d03e6b0..edc2db3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -888,6 +888,7 @@ ESX_DRIVER_EXTRA_DIST = \ HYPERV_DRIVER_SOURCES = \ hyperv/hyperv_private.h \ hyperv/hyperv_driver.c hyperv/hyperv_driver.h \ + hyperv/hyperv_driver_2012.c hyperv/hyperv_driver_2012.h \ hyperv/hyperv_network_driver.c hyperv/hyperv_network_driver.h \ hyperv/hyperv_util.c hyperv/hyperv_util.h \ hyperv/hyperv_wmi.c hyperv/hyperv_wmi.h \ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 716fadb..63baef8 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -30,6 +30,7 @@ #include "virlog.h" #include "viruuid.h" #include "hyperv_driver.h" +#include "hyperv_driver_2012.h" #include "hyperv_network_driver.h" #include "hyperv_private.h" #include "hyperv_util.h" @@ -65,6 +66,35 @@ hypervFreePrivate(hypervPrivate **priv)

/* Forward declaration of hypervCapsInit */ static virCapsPtr hypervCapsInit(hypervPrivate *priv); +static virHypervisorDriver hypervHypervisorDriver; + +static char * +hypervNodeGetWindowsVersion(hypervPrivate *priv) +{ + virBuffer query = VIR_BUFFER_INITIALIZER; + Win32_OperatingSystem *operatingSystem = NULL; + + /* Get Win32_OperatingSystem */ + virBufferAddLit(&query, WIN32_OPERATINGSYSTEM_WQL_SELECT); + + if (hypervGetWin32OperatingSystemList(priv, &query, &operatingSystem) < 0) { + goto cleanup; + } + + if (operatingSystem == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get Win32_OperatingSystem")); + goto cleanup; + } + + return operatingSystem->data->Version; + + cleanup: + hypervFreeObject(priv, (hypervObject *) operatingSystem); + virBufferFreeAndReset(&query); + + return NULL; +}

static virDrvOpenStatus hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, @@ -78,6 +108,9 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, char *password = NULL; virBuffer query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL; + Msvm_ComputerSystem_2012 *computerSystem2012 = NULL; + char *windowsVersion = NULL; + char *hypervVersion = (char *)calloc(4, sizeof(char));

Coverity notes hypervVersion would not be free'd if virCheckFlags returns failure

virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);

In fact each "return" after if statement prior to the VIR_ALLOC(priv) will elicit the same memory leak So, let's just "char *hyperVersion = NULL;" then later on...

@@ -175,6 +208,16 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, /* FIXME: Currently only basic authentication is supported */ wsman_transport_set_auth_method(priv->client, "basic");

+ windowsVersion = hypervNodeGetWindowsVersion(priv); + if (windowsVersion == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not determine Windows version")); + goto cleanup; + } + + strncpy(hypervVersion, windowsVersion, 3); + priv->hypervVersion = hypervVersion; +

Consider: if (VIR_STRNDUP(priv->hyperVersion, windowsVersion, 3) < 0) and remove hyperVersion altogether.

/* Check if the connection can be established and if the server has the * Hyper-V role installed. If the call to hypervGetMsvmComputerSystemList * succeeds than the connection has been established. If the returned list @@ -183,15 +226,25 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, virBufferAddLit(&query, "where "); virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_PHYSICAL);

- if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) - goto cleanup; + if (strcmp(priv->hypervVersion, HYPERV_VERSION_2008) == 0) {

You could use STREQ instead

+ if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) + goto cleanup; + } else if (strcmp(priv->hypervVersion, HYPERV_VERSION_2012) == 0) {

STREQ

+ if (hypervGetMsvmComputerSystem2012List(priv, &query, &computerSystem2012) < 0) + goto cleanup; + }

- if (computerSystem == NULL) { + if (computerSystem == NULL && computerSystem2012 == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s is not a Hyper-V server"), conn->uri->server); goto cleanup; }

+ if (computerSystem2012 != NULL) { + hypervHypervisorDriver.connectListAllDomains = hypervConnectListAllDomains2012; + hypervHypervisorDriver.domainGetState = hypervDomainGetState2012; + } + /* Setup capabilities */ priv->caps = hypervCapsInit(priv); if (priv->caps == NULL) { @@ -209,8 +262,9 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, hypervFreePrivate(&priv); VIR_FREE(username); VIR_FREE(password); + free(hypervVersion);

VIR_FREE

hypervFreeObject(priv, (hypervObject *)computerSystem); - + hypervFreeObject(priv, (hypervObject *)computerSystem2012); return result; }

diff --git a/src/hyperv/hyperv_driver_2012.c b/src/hyperv/hyperv_driver_2012.c new file mode 100644 index 0000000..6c2b3b6 --- /dev/null +++ b/src/hyperv/hyperv_driver_2012.c @@ -0,0 +1,299 @@

There's no copyright I did't check what/if the difference was with the non-2012 code, but instead of cut-copy-paste of entire functions - you really need to consider what the differences are and set some sort of flag so that you can make those intelligent decisions at the inflection points... The whole point of a "priv" buffer is it's private you can set flags in there you can do hypervisor specific stuff in there... I think you need to figure a better mechanism than different functions doing relatively the same thing except for well who knows what because I didn't compare in depth. John

+#include "hyperv_driver_2012.h" + +static int +hypervMsvmComputerSystemEnabledStateToDomainState2012( + Msvm_ComputerSystem_2012 *computerSystem) +{ + switch (computerSystem->data->EnabledState) { + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_UNKNOWN: + return VIR_DOMAIN_NOSTATE; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED: + return VIR_DOMAIN_RUNNING; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED: + return VIR_DOMAIN_SHUTOFF; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSED: + return VIR_DOMAIN_PAUSED; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED: /* managed save */ + return VIR_DOMAIN_SHUTOFF; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_STARTING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SNAPSHOTTING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SAVING: + return VIR_DOMAIN_RUNNING; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_STOPPING: + return VIR_DOMAIN_SHUTDOWN; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_RESUMING: + return VIR_DOMAIN_RUNNING; + + default: + return VIR_DOMAIN_NOSTATE; + } +} + +static int +hypervMsvmComputerSystemFromDomain2012(virDomainPtr domain, + Msvm_ComputerSystem_2012 **computerSystem) +{ + hypervPrivate *priv = domain->conn->privateData; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + virBuffer query = VIR_BUFFER_INITIALIZER; + + if (computerSystem == NULL || *computerSystem != NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + virUUIDFormat(domain->uuid, uuid_string); + + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAddLit(&query, "where "); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); + virBufferAsprintf(&query, "and Name = \"%s\"", uuid_string); + + if (hypervGetMsvmComputerSystem2012List(priv, &query, computerSystem) < 0) + return -1; + + if (*computerSystem == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with UUID %s"), uuid_string); + return -1; + } + + return 0; +} + +static bool +hypervIsMsvmComputerSystemActive2012(Msvm_ComputerSystem_2012 *computerSystem, + bool *in_transition) +{ + if (in_transition != NULL) + *in_transition = false; + + switch (computerSystem->data->EnabledState) { + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_UNKNOWN: + return false; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED: + return true; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED: + return false; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSED: + return true; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED: /* managed save */ + return false; + + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_STARTING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SNAPSHOTTING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SAVING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_STOPPING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_PAUSING: + case MSVM_COMPUTERSYSTEM_ENABLEDSTATE_RESUMING: + if (in_transition != NULL) + *in_transition = true; + + return true; + + default: + return false; + } +} + +static int +hypervMsvmComputerSystemToDomain2012(virConnectPtr conn, + Msvm_ComputerSystem_2012 *computerSystem, + virDomainPtr *domain) +{ + unsigned char uuid[VIR_UUID_BUFLEN]; + + if (domain == NULL || *domain != NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (virUUIDParse(computerSystem->data->Name, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + computerSystem->data->Name); + return -1; + } + + *domain = virGetDomain(conn, computerSystem->data->ElementName, uuid); + + if (*domain == NULL) + return -1; + + if (hypervIsMsvmComputerSystemActive2012(computerSystem, NULL)) { + (*domain)->id = computerSystem->data->ProcessID; + } else { + (*domain)->id = -1; + } + + return 0; +} + +int +hypervDomainGetState2012(virDomainPtr domain, int *state, int *reason, + unsigned int flags) +{ + int result = -1; + hypervPrivate *priv = domain->conn->privateData; + Msvm_ComputerSystem_2012 *computerSystem = NULL; + + virCheckFlags(0, -1); + + if (hypervMsvmComputerSystemFromDomain2012(domain, &computerSystem) < 0) + goto cleanup; + + *state = hypervMsvmComputerSystemEnabledStateToDomainState2012(computerSystem); + + if (reason != NULL) + *reason = 0; + + result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *)computerSystem); + + return result; +} + +#define MATCH(FLAG) (flags & (FLAG)) +int +hypervConnectListAllDomains2012(virConnectPtr conn, + virDomainPtr **domains, + unsigned int flags) +{ + hypervPrivate *priv = conn->privateData; + virBuffer query = VIR_BUFFER_INITIALIZER; + Msvm_ComputerSystem_2012 *computerSystemList = NULL; + Msvm_ComputerSystem_2012 *computerSystem = NULL; + size_t ndoms; + virDomainPtr domain; + virDomainPtr *doms = NULL; + int count = 0; + int ret = -1; + size_t i; + + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); + + /* check for filter combinations that return no results: + * persistent: all hyperv guests are persistent + * snapshot: the driver does not support snapshot management + * autostart: the driver does not support autostarting guests + */ + if ((MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) && + !MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT)) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) && + !MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART)) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) && + !MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT))) { + if (domains && VIR_ALLOC_N(*domains, 1) < 0) + goto cleanup; + + ret = 0; + goto cleanup; + } + + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); + virBufferAddLit(&query, "where "); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); + + /* construct query with filter depending on flags */ + if (!(MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE) && + MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE))) { + if (MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE)) { + virBufferAddLit(&query, "and "); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_ACTIVE); + } + + if (MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { + virBufferAddLit(&query, "and "); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_INACTIVE); + } + } + + if (hypervGetMsvmComputerSystem2012List(priv, &query, + &computerSystemList) < 0) + goto cleanup; + + if (domains) { + if (VIR_ALLOC_N(doms, 1) < 0) + goto cleanup; + ndoms = 1; + } + + for (computerSystem = computerSystemList; computerSystem != NULL; + computerSystem = computerSystem->next) { + + /* filter by domain state */ + if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) { + int st = hypervMsvmComputerSystemEnabledStateToDomainState2012(computerSystem); + if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) && + st == VIR_DOMAIN_RUNNING) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) && + st == VIR_DOMAIN_PAUSED) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_SHUTOFF) && + st == VIR_DOMAIN_SHUTOFF) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_OTHER) && + (st != VIR_DOMAIN_RUNNING && + st != VIR_DOMAIN_PAUSED && + st != VIR_DOMAIN_SHUTOFF)))) + continue; + } + + /* managed save filter */ + if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_MANAGEDSAVE)) { + bool mansave = computerSystem->data->EnabledState == + MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED; + + if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) && mansave) || + (MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE) && !mansave))) + continue; + } + + if (!doms) { + count++; + continue; + } + + if (VIR_RESIZE_N(doms, ndoms, count, 2) < 0) + goto cleanup; + + domain = NULL; + + if (hypervMsvmComputerSystemToDomain2012(conn, computerSystem, + &domain) < 0) + goto cleanup; + + doms[count++] = domain; + } + + if (doms) + *domains = doms; + doms = NULL; + ret = count; + + cleanup: + if (doms) { + for (i = 0; i < count; ++i) + virObjectUnref(doms[i]); + + VIR_FREE(doms); + } + + hypervFreeObject(priv, (hypervObject *)computerSystemList); + + return ret; +} + +#undef MATCH diff --git a/src/hyperv/hyperv_driver_2012.h b/src/hyperv/hyperv_driver_2012.h new file mode 100644 index 0000000..45eb15b --- /dev/null +++ b/src/hyperv/hyperv_driver_2012.h @@ -0,0 +1,55 @@ +/* + * hyperv_driver.h: core driver functions for managing Microsoft Hyper-V hosts + * + * Copyright (C) 2011 Matthias Bolte <matthias.bolte@googlemail.com> + * Copyright (C) 2009 Michael Sievers <msievers83@googlemail.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __HYPERV_DRIVER_2012_H__ +# define __HYPERV_DRIVER_2012_H__ + +#include <config.h> + +#include "internal.h" +#include "datatypes.h" +#include "virdomainobjlist.h" +#include "virauth.h" +#include "viralloc.h" +#include "virlog.h" +#include "viruuid.h" +#include "hyperv_driver.h" +#include "hyperv_network_driver.h" +#include "hyperv_private.h" +#include "hyperv_util.h" +#include "hyperv_wmi.h" +#include "openwsman.h" +#include "virstring.h" +#include "virtypedparam.h" + +#define VIR_FROM_THIS VIR_FROM_HYPERV + +int +hypervConnectListAllDomains2012(virConnectPtr conn, + virDomainPtr **domains, + unsigned int flags); + +int +hypervDomainGetState2012(virDomainPtr domain, int *state, int *reason, + unsigned int flags); + +#endif /* __HYPERV_DRIVER_2012_H__ */ diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 2dfce6e..db4e15a 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -37,6 +37,10 @@ struct _hypervPrivate { WsManClient *client; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; + char *hypervVersion; };

+#define HYPERV_VERSION_2008 "6.1" +#define HYPERV_VERSION_2012 "6.3" + #endif /* __HYPERV_PRIVATE_H__ */ diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index 67f45fb..be2f429 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -35,6 +35,9 @@ #define ROOT_VIRTUALIZATION \ "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/*"

+#define ROOT_VIRTUALIZATION_V2 \ + "http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/v2/*" + typedef struct _hypervObject hypervObject;

int hyperyVerifyResponse(WsManClient *client, WsXmlDocH response, diff --git a/src/hyperv/hyperv_wmi_generator.input b/src/hyperv/hyperv_wmi_generator.input index 28a5bdc..cf136ca 100644 --- a/src/hyperv/hyperv_wmi_generator.input +++ b/src/hyperv/hyperv_wmi_generator.input @@ -55,6 +55,37 @@ class Msvm_ComputerSystem uint16 AssignedNumaNodeList[] end

+class Msvm_ComputerSystem_2012 + string Caption + string Description + string ElementName + datetime InstallDate + uint16 OperationalStatus[] + string StatusDescriptions[] + string Status + uint16 HealthState + uint16 EnabledState + string OtherEnabledState + uint16 RequestedState + uint16 EnabledDefault + datetime TimeOfLastStateChange + string CreationClassName + string Name + string PrimaryOwnerName + string PrimaryOwnerContact + string Roles[] + string NameFormat + string OtherIdentifyingInfo[] + string IdentifyingDescriptions[] + uint16 Dedicated[] + string OtherDedicatedDescriptions[] + uint16 ResetCapability + uint16 PowerManagementCapabilities[] + uint64 OnTimeInMilliseconds + datetime TimeOfLastConfigurationChange + uint32 ProcessID + uint16 AssignedNumaNodeList[] +end

class Msvm_ConcreteJob string Caption @@ -196,7 +227,7 @@ class Win32_ComputerSystem string Caption uint16 ChassisBootupState string CreationClassName - int16 CurrentTimeZone +# uint16 CurrentTimeZone boolean DaylightInEffect string Description string DNSHostName @@ -466,7 +497,7 @@ class Win32_OperatingSystem string CSCreationClassName string CSDVersion string CSName - uint16 CurrentTimeZone +# uint16 CurrentTimeZone boolean DataExecutionPrevention_Available boolean DataExecutionPrevention_32BitApplications boolean DataExecutionPrevention_Drivers diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py index 8384634..ed40a79 100755 --- a/src/hyperv/hyperv_wmi_generator.py +++ b/src/hyperv/hyperv_wmi_generator.py @@ -61,6 +61,9 @@ class Class:

def generate_classes_header(self): name_upper = self.name.upper() + class_name = self.name + if self.name.endswith("_2012"): + class_name = class_name[:-5]

header = separator header += " * %s\n" % self.name @@ -70,15 +73,17 @@ class Class:

if self.name.startswith("Win32_") or self.name.startswith("CIM_DataFile"): header += " \"http://schemas.microsoft.com/wbem/wsman/1/wmi/root/cimv2/%s\"\n" % self.name + elif self.name.endswith("_2012"): + header += " \"http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/v2/%s\"\n" % class_name else: header += " \"http://schemas.microsoft.com/wbem/wsman/1/wmi/root/virtualization/%s\"\n" % self.name

header += "\n" header += "#define %s_CLASSNAME \\\n" % name_upper - header += " \"%s\"\n" % self.name + header += " \"%s\"\n" % class_name header += "\n" header += "#define %s_WQL_SELECT \\\n" % name_upper - header += " \"select * from %s \"\n" % self.name + header += " \"select * from %s \"\n" % class_name header += "\n" header += "struct _%s_Data {\n" % self.name

@@ -134,6 +139,8 @@ class Class:

if self.name.startswith("Win32_") or self.name.startswith("CIM_DataFile"): source += " return hypervEnumAndPull(priv, query, ROOT_CIMV2,\n" + elif self.name.endswith("_2012"): + source += " return hypervEnumAndPull(priv, query, ROOT_VIRTUALIZATION_V2,\n" else: source += " return hypervEnumAndPull(priv, query, ROOT_VIRTUALIZATION,\n"