On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

There are two specific WQL queries we're using to get either a list of virtual machines or the hypervisor host itself from Msvm_ComputerSystem. Those queries rely on filtering results based on the "Description" field. Since the "Description" field is locale sensitive, the queries will fail if the Windows host is using a language pack. While the WSMAN spec allows the client to set the requested locale (and it is supported since openwsman 2.6.x), the Windows WinRM service does not respect this setting: it returns non-English strings despite the WSMAN request properly setting the locale to 'en-US'. Therefore, this patch changes the WQL query to make use of the "__SERVER" field to stop relying on English strings in queries and side step the issue.

Co-authored-by: Dawid Zamirski <dzamirski@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi_classes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/hyperv/hyperv_wmi_classes.h b/src/hyperv/hyperv_wmi_classes.h index a19b6a656d..7465684d6e 100644 --- a/src/hyperv/hyperv_wmi_classes.h +++ b/src/hyperv/hyperv_wmi_classes.h @@ -44,10 +44,10 @@ */

#define MSVM_COMPUTERSYSTEM_WQL_VIRTUAL \ - "Description = \"Microsoft Virtual Machine\" " + "Name != __SERVER "

#define MSVM_COMPUTERSYSTEM_WQL_PHYSICAL \ - "Description = \"Microsoft Hosting Computer System\" " + "Name = __SERVER "

#define MSVM_COMPUTERSYSTEM_WQL_ACTIVE \ "(EnabledState != 0 and EnabledState != 3 and EnabledState != 32769) " -- 2.27.0

Should we require a bump to openwsman 2.6 for this? -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

Some CPU model names were too long for _virNodeInfo.model. For example: Intel Xeon CPU E5-2620 v2 @ 2.10GHz This commit removes the clock frequency suffix.

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index b57325f2a5..9bbc2f67fb 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -285,6 +285,10 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) } else if (STRPREFIX(tmp, "(TM)")) { memmove(tmp, tmp + 4, strlen(tmp + 4) + 1); continue; + } else if (STRPREFIX(tmp, " @ ")) { + /* Remove " @ X.YZGHz" from the end. */ + *tmp = '\0'; + break; }

++tmp; -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

This eliminates some duplicate code and simplifies the driver functions.

Co-authored-by: Sri Ramanujam <sramanujam@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 489 ++++++++++++++++++++----------------- 1 file changed, 259 insertions(+), 230 deletions(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 9bbc2f67fb..5e09c8f4af 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -41,6 +41,238 @@

VIR_LOG_INIT("hyperv.hyperv_driver");

+/* + * WMI utility functions + * + * wrapper functions for commonly-accessed WMI objects and interfaces. + */ + +static int +hypervGetProcessorsByName(hypervPrivate *priv, const char *name, + Win32_Processor **processorList) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + "ASSOCIATORS OF {Win32_ComputerSystem.Name=\"%s\"} " + "WHERE AssocClass = Win32_ComputerSystemProcessor " + "ResultClass = Win32_Processor", + name); + + if (hypervGetWin32ProcessorList(priv, &query, processorList) < 0 || + !processorList) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not look up processor(s) on '%s'"), + name); + return -1; + } + + return 0; +} + +static int +hypervGetActiveVirtualSystemList(hypervPrivate *priv, + Msvm_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = { g_string_new(MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND " MSVM_COMPUTERSYSTEM_WQL_ACTIVE), 0 }; + + if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0 || + !*computerSystemList) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not look up active virtual machines")); + return -1; + } + + return 0; +} + +/* gets all the vms including the ones that are marked inactive. */ +static int +hypervGetInactiveVirtualSystemList(hypervPrivate *priv, + Msvm_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = { g_string_new(MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND " MSVM_COMPUTERSYSTEM_WQL_INACTIVE), 0 }; + + if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0 || + !*computerSystemList) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not look up inactive virtual machines")); + return -1; + } + + return 0; +} + +static int +hypervGetPhysicalSystemList(hypervPrivate *priv, + Win32_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = { g_string_new(WIN32_COMPUTERSYSTEM_WQL_SELECT), 0 }; + + if (hypervGetWin32ComputerSystemList(priv, &query, computerSystemList) < 0 || + !*computerSystemList) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not look up Win32_ComputerSystem")); + return -1; + } + + return 0; +} + +static int +hypervGetVirtualSystemByID(hypervPrivate *priv, int id, + Msvm_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferAsprintf(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND ProcessID = %d", + id); + + if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Could not look up virtual system with ID %d"), id); + return -1; + } + + if (*computerSystemList == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, _("No domain with ID %d"), id); + return -1; + } + + return 0; +} + +static int +hypervGetVirtualSystemByUUID(hypervPrivate *priv, const char *uuid, + Msvm_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND Name = \"%s\"", + uuid); + + if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Could not look up virtual system with UUID '%s'"), + uuid); + return -1; + } + + if (*computerSystemList == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with UUID %s"), uuid); + return -1; + } + + return 0; +} + + +static int +hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, + Msvm_ComputerSystem **computerSystemList) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND ElementName = \"%s\"", + name); + + if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Could not look up virtual system named '%s'"), name); + return -1; + } + + if (*computerSystemList == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with name %s"), name); + return -1; + } + + return 0; +} + +static int +hypervGetVSSDFromUUID(hypervPrivate *priv, const char *uuid, + Msvm_VirtualSystemSettingData **data) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\",Name=\"%s\"} " + "WHERE AssocClass = Msvm_SettingsDefineState " + "ResultClass = Msvm_VirtualSystemSettingData", + uuid); + + if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, data) < 0 || + !*data) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not look up virtual system setting data with UUID '%s'"), + uuid); + return -1; + } + + return 0; +} + +static int +hypervGetProcSDByVSSDInstanceId(hypervPrivate *priv, const char *id, + Msvm_ProcessorSettingData **data) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_ProcessorSettingData", + id); + + if (hypervGetMsvmProcessorSettingDataList(priv, &query, data) < 0 || + !*data) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not look up processor setting data with virtual system instance ID '%s'"), + id); + return -1; + } + + return 0; +} + +static int +hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id, + Msvm_MemorySettingData **data) +{ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + virBufferEscapeSQL(&query, + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ResultClass = Msvm_MemorySettingData", + id); + + if (hypervGetMsvmMemorySettingDataList(priv, &query, data) < 0 || + !*data) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not look up memory setting data with virtual system instance ID '%s'"), + id); + return -1; + } + + return 0; +} + + + +/* + * Driver functions + */ + static void hypervFreePrivate(hypervPrivate **priv) { @@ -203,20 +435,10 @@ hypervConnectGetHostname(virConnectPtr conn) { char *hostname = NULL; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Win32_ComputerSystem *computerSystem = NULL;

- virBufferAddLit(&query, WIN32_COMPUTERSYSTEM_WQL_SELECT); - - if (hypervGetWin32ComputerSystemList(priv, &query, &computerSystem) < 0) - goto cleanup; - - if (computerSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s"), - "Win32_ComputerSystem"); + if (hypervGetPhysicalSystemList(priv, &computerSystem) < 0) goto cleanup; - }

hostname = g_strdup(computerSystem->data.common->DNSHostName);

@@ -233,7 +455,6 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { int result = -1; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Win32_ComputerSystem *computerSystem = NULL; Win32_Processor *processorList = NULL; Win32_Processor *processor = NULL; @@ -241,34 +462,11 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)

memset(info, 0, sizeof(*info));

- virBufferAddLit(&query, WIN32_COMPUTERSYSTEM_WQL_SELECT); - - /* Get Win32_ComputerSystem */ - if (hypervGetWin32ComputerSystemList(priv, &query, &computerSystem) < 0) + if (hypervGetPhysicalSystemList(priv, &computerSystem) < 0) goto cleanup;

- if (computerSystem == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s"), - "Win32_ComputerSystem"); - goto cleanup; - } - - /* Get Win32_Processor list */ - virBufferEscapeSQL(&query, - "associators of " - "{Win32_ComputerSystem.Name=\"%s\"} " - "where AssocClass = Win32_ComputerSystemProcessor " - "ResultClass = Win32_Processor", - computerSystem->data.common->Name); - - if (hypervGetWin32ProcessorList(priv, &query, &processorList) < 0) - goto cleanup; - - if (processorList == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s"), - "Win32_Processor"); + if (hypervGetProcessorsByName(priv, computerSystem->data.common->Name, + &processorList) < 0) { goto cleanup; }

@@ -332,7 +530,6 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) { bool success = false; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; @@ -340,16 +537,8 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) if (maxids == 0) return 0;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAddLit(&query, "and "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_ACTIVE); - - if (hypervGetMsvmComputerSystemList(priv, &query, - &computerSystemList) < 0) { + if (hypervGetActiveVirtualSystemList(priv, &computerSystemList) < 0) goto cleanup; - }

for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { @@ -374,21 +563,12 @@ hypervConnectNumOfDomains(virConnectPtr conn) { bool success = false; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAddLit(&query, "and "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_ACTIVE); - - if (hypervGetMsvmComputerSystemList(priv, &query, - &computerSystemList) < 0) { + if (hypervGetActiveVirtualSystemList(priv, &computerSystemList) < 0) goto cleanup; - }

for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { @@ -410,22 +590,11 @@ hypervDomainLookupByID(virConnectPtr conn, int id) { virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAsprintf(&query, "and ProcessID = %d", id); - - if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) + if (hypervGetVirtualSystemByID(priv, id, &computerSystem) < 0) goto cleanup;

- if (computerSystem == NULL) { - virReportError(VIR_ERR_NO_DOMAIN, _("No domain with ID %d"), id); - goto cleanup; - } - hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain);

cleanup: @@ -442,24 +611,12 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL;

virUUIDFormat(uuid, uuid_string);

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferEscapeSQL(&query, "and Name = \"%s\"", 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); + if (hypervGetVirtualSystemByUUID(priv, uuid_string, &computerSystem) < 0) goto cleanup; - }

hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain);

@@ -476,23 +633,11 @@ hypervDomainLookupByName(virConnectPtr conn, const char *name) { virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferEscapeSQL(&query, "and ElementName = \"%s\"", name); - - if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) + if (hypervGetVirtualSystemByName(priv, name, &computerSystem) < 0) goto cleanup;

- if (computerSystem == NULL) { - virReportError(VIR_ERR_NO_DOMAIN, - _("No domain with name %s"), name); - goto cleanup; - } - hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain);

cleanup: @@ -615,7 +760,6 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL; Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; Msvm_ProcessorSettingData *processorSettingData = NULL; @@ -629,68 +773,20 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) goto cleanup;

- /* Get Msvm_VirtualSystemSettingData */ - virBufferEscapeSQL(&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.common->ElementName); - goto cleanup; - } - - /* Get Msvm_ProcessorSettingData */ - virBufferEscapeSQL(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " - "where AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_ProcessorSettingData", - virtualSystemSettingData->data.common->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.common->ElementName); + if (hypervGetVSSDFromUUID(priv, uuid_string, + &virtualSystemSettingData) < 0) { goto cleanup; }

- /* Get Msvm_MemorySettingData */ - virBufferEscapeSQL(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " - "where AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_MemorySettingData", - virtualSystemSettingData->data.common->InstanceID); - - if (hypervGetMsvmMemorySettingDataList(priv, &query, - &memorySettingData) < 0) { + if (hypervGetProcSDByVSSDInstanceId(priv, + virtualSystemSettingData->data.common->InstanceID, + &processorSettingData) < 0) { goto cleanup; }

- - if (memorySettingData == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s for domain %s"), - "Msvm_MemorySettingData", - computerSystem->data.common->ElementName); + if (hypervGetMemSDByVSSDInstanceId(priv, + virtualSystemSettingData->data.common->InstanceID, + &memorySettingData) < 0) { goto cleanup; }

@@ -749,7 +845,6 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) hypervPrivate *priv = domain->conn->privateData; virDomainDefPtr def = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN]; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystem = NULL; Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; Msvm_ProcessorSettingData *processorSettingData = NULL; @@ -766,68 +861,20 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) goto cleanup;

- /* Get Msvm_VirtualSystemSettingData */ - virBufferEscapeSQL(&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.common->ElementName); - goto cleanup; - } - - /* Get Msvm_ProcessorSettingData */ - virBufferEscapeSQL(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " - "where AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_ProcessorSettingData", - virtualSystemSettingData->data.common->InstanceID); - - if (hypervGetMsvmProcessorSettingDataList(priv, &query, - &processorSettingData) < 0) { + if (hypervGetVSSDFromUUID(priv, uuid_string, + &virtualSystemSettingData) < 0) { goto cleanup; }

- if (processorSettingData == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s for domain %s"), - "Msvm_ProcessorSettingData", - computerSystem->data.common->ElementName); + if (hypervGetProcSDByVSSDInstanceId(priv, + virtualSystemSettingData->data.common->InstanceID, + &processorSettingData) < 0) { goto cleanup; }

- /* Get Msvm_MemorySettingData */ - virBufferEscapeSQL(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " - "where AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_MemorySettingData", - virtualSystemSettingData->data.common->InstanceID); - - if (hypervGetMsvmMemorySettingDataList(priv, &query, - &memorySettingData) < 0) { - goto cleanup; - } - - - if (memorySettingData == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not lookup %s for domain %s"), - "Msvm_MemorySettingData", - computerSystem->data.common->ElementName); + if (hypervGetMemSDByVSSDInstanceId(priv, + virtualSystemSettingData->data.common->InstanceID, + &memorySettingData) < 0) { goto cleanup; }

@@ -909,7 +956,6 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn { bool success = false; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; @@ -918,16 +964,8 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn if (maxnames == 0) return 0;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAddLit(&query, "and "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_INACTIVE); - - if (hypervGetMsvmComputerSystemList(priv, &query, - &computerSystemList) < 0) { + if (hypervGetInactiveVirtualSystemList(priv, &computerSystemList) < 0) goto cleanup; - }

for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { @@ -961,21 +999,12 @@ hypervConnectNumOfDefinedDomains(virConnectPtr conn) { bool success = false; hypervPrivate *priv = conn->privateData; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; Msvm_ComputerSystem *computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0;

- virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); - virBufferAddLit(&query, "and "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_INACTIVE); - - if (hypervGetMsvmComputerSystemList(priv, &query, - &computerSystemList) < 0) { + if (hypervGetInactiveVirtualSystemList(priv, &computerSystemList) < 0) goto cleanup; - }

for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 39 ++++++++++++------ src/hyperv/hyperv_wmi.c | 83 ++++---------------------------------- src/hyperv/hyperv_wmi.h | 28 ++----------- 3 files changed, 36 insertions(+), 114 deletions(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 5e09c8f4af..3e4563252e 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -47,6 +47,20 @@ VIR_LOG_INIT("hyperv.hyperv_driver"); * wrapper functions for commonly-accessed WMI objects and interfaces. */

+/** + * hypervGetWmiClass: + * @type: the type of the class being retrieved from WMI + * @class: double pointer where the class data will be stored + * + * Retrieve one or more classes from WMI. + * + * The following variables must exist in the caller: + * 1. hypervPrivate *priv + * 2. virBuffer query + */ +#define hypervGetWmiClass(type, class) \ + hypervGetWmiClassList(priv, type ## _WmiInfo, &query, (hypervObject **)class) + static int hypervGetProcessorsByName(hypervPrivate *priv, const char *name, Win32_Processor **processorList) @@ -58,7 +72,7 @@ hypervGetProcessorsByName(hypervPrivate *priv, const char *name, "ResultClass = Win32_Processor", name);

- if (hypervGetWin32ProcessorList(priv, &query, processorList) < 0 || + if (hypervGetWmiClass(Win32_Processor, processorList) < 0 || !processorList) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not look up processor(s) on '%s'"), @@ -77,7 +91,7 @@ hypervGetActiveVirtualSystemList(hypervPrivate *priv, "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL "AND " MSVM_COMPUTERSYSTEM_WQL_ACTIVE), 0 };

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0 || + if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0 || !*computerSystemList) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not look up active virtual machines")); @@ -96,7 +110,7 @@ hypervGetInactiveVirtualSystemList(hypervPrivate *priv, "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL "AND " MSVM_COMPUTERSYSTEM_WQL_INACTIVE), 0 };

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0 || + if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0 || !*computerSystemList) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not look up inactive virtual machines")); @@ -112,7 +126,7 @@ hypervGetPhysicalSystemList(hypervPrivate *priv, { g_auto(virBuffer) query = { g_string_new(WIN32_COMPUTERSYSTEM_WQL_SELECT), 0 };

- if (hypervGetWin32ComputerSystemList(priv, &query, computerSystemList) < 0 || + if (hypervGetWmiClass(Win32_ComputerSystem, computerSystemList) < 0 || !*computerSystemList) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not look up Win32_ComputerSystem")); @@ -133,7 +147,7 @@ hypervGetVirtualSystemByID(hypervPrivate *priv, int id, "AND ProcessID = %d", id);

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("Could not look up virtual system with ID %d"), id); return -1; @@ -158,7 +172,7 @@ hypervGetVirtualSystemByUUID(hypervPrivate *priv, const char *uuid, "AND Name = \"%s\"", uuid);

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("Could not look up virtual system with UUID '%s'"), uuid); @@ -186,7 +200,7 @@ hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, "AND ElementName = \"%s\"", name);

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystemList) < 0) { + if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("Could not look up virtual system named '%s'"), name); return -1; @@ -212,7 +226,7 @@ hypervGetVSSDFromUUID(hypervPrivate *priv, const char *uuid, "ResultClass = Msvm_VirtualSystemSettingData", uuid);

- if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, data) < 0 || + if (hypervGetWmiClass(Msvm_VirtualSystemSettingData, data) < 0 || !*data) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not look up virtual system setting data with UUID '%s'"), @@ -234,7 +248,7 @@ hypervGetProcSDByVSSDInstanceId(hypervPrivate *priv, const char *id, "ResultClass = Msvm_ProcessorSettingData", id);

- if (hypervGetMsvmProcessorSettingDataList(priv, &query, data) < 0 || + if (hypervGetWmiClass(Msvm_ProcessorSettingData, data) < 0 || !*data) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not look up processor setting data with virtual system instance ID '%s'"), @@ -256,7 +270,7 @@ hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id, "ResultClass = Msvm_MemorySettingData", id);

- if (hypervGetMsvmMemorySettingDataList(priv, &query, data) < 0 || + if (hypervGetWmiClass(Msvm_MemorySettingData, data) < 0 || !*data) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not look up memory setting data with virtual system instance ID '%s'"), @@ -1278,8 +1292,7 @@ hypervConnectListAllDomains(virConnectPtr conn, } }

- if (hypervGetMsvmComputerSystemList(priv, &query, - &computerSystemList) < 0) + if (hypervGetWmiClass(Msvm_ComputerSystem, &computerSystemList) < 0) goto cleanup;

if (domains) { @@ -1385,7 +1398,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, "where ResultClass = Msvm_Keyboard", uuid_string);

- if (hypervGetMsvmKeyboardList(priv, &query, &keyboard) < 0) + if (hypervGetWmiClass(Msvm_Keyboard, &keyboard) < 0) goto cleanup;

translatedKeycodes = g_new0(int, nkeycodes); diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 809f68a844..b233dab58d 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -83,7 +83,7 @@ hypervGetWmiClassInfo(hypervPrivate *priv, hypervWmiClassInfoListPtr list, return -1; }

-static int +int hypervGetWmiClassList(hypervPrivate *priv, hypervWmiClassInfoListPtr wmiInfo, virBufferPtr query, hypervObject **wmiClass) { @@ -878,8 +878,8 @@ hypervInvokeMethod(hypervPrivate *priv, hypervInvokeParamsListPtr params, virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); virBufferEscapeSQL(&query, "where InstanceID = \"%s\"", instanceID);

- if (hypervGetMsvmConcreteJobList(priv, &query, &job) < 0 - || job == NULL) + if (hypervGetWmiClassList(priv, Msvm_ConcreteJob_WmiInfo, &query, + (hypervObject **)&job) < 0 || job == NULL) goto cleanup;

jobState = job->data.common->JobState; @@ -1218,77 +1218,6 @@ hypervReturnCodeToString(int returnCode)

-/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * - * Generic "Get WMI class list" helpers - */ - -int -hypervGetMsvmComputerSystemList(hypervPrivate *priv, virBufferPtr query, - Msvm_ComputerSystem **list) -{ - return hypervGetWmiClassList(priv, Msvm_ComputerSystem_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetMsvmConcreteJobList(hypervPrivate *priv, virBufferPtr query, - Msvm_ConcreteJob **list) -{ - return hypervGetWmiClassList(priv, Msvm_ConcreteJob_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetWin32ComputerSystemList(hypervPrivate *priv, virBufferPtr query, - Win32_ComputerSystem **list) -{ - return hypervGetWmiClassList(priv, Win32_ComputerSystem_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetWin32ProcessorList(hypervPrivate *priv, virBufferPtr query, - Win32_Processor **list) -{ - return hypervGetWmiClassList(priv, Win32_Processor_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetMsvmVirtualSystemSettingDataList(hypervPrivate *priv, - virBufferPtr query, - Msvm_VirtualSystemSettingData **list) -{ - return hypervGetWmiClassList(priv, Msvm_VirtualSystemSettingData_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetMsvmProcessorSettingDataList(hypervPrivate *priv, - virBufferPtr query, - Msvm_ProcessorSettingData **list) -{ - return hypervGetWmiClassList(priv, Msvm_ProcessorSettingData_WmiInfo, query, - (hypervObject **)list); -} - -int -hypervGetMsvmMemorySettingDataList(hypervPrivate *priv, virBufferPtr query, - Msvm_MemorySettingData **list) -{ - return hypervGetWmiClassList(priv, Msvm_MemorySettingData_WmiInfo, query, - (hypervObject **)list); -} - -int hypervGetMsvmKeyboardList(hypervPrivate *priv, virBufferPtr query, - Msvm_Keyboard **list) -{ - return hypervGetWmiClassList(priv, Msvm_Keyboard_WmiInfo, query, - (hypervObject **)list); -} - - - /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Msvm_ComputerSystem */ @@ -1371,7 +1300,8 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); virBufferAsprintf(&query, "where InstanceID = \"%s\"", instanceID);

- if (hypervGetMsvmConcreteJobList(priv, &query, &concreteJob) < 0) + if (hypervGetWmiClassList(priv, Msvm_ConcreteJob_WmiInfo, &query, + (hypervObject **)&concreteJob) < 0) goto cleanup;

if (concreteJob == NULL) { @@ -1560,7 +1490,8 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain, virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); virBufferAsprintf(&query, "and Name = \"%s\"", uuid_string);

- if (hypervGetMsvmComputerSystemList(priv, &query, computerSystem) < 0) + if (hypervGetWmiClassList(priv, Msvm_ComputerSystem_WmiInfo, &query, + (hypervObject **)computerSystem) < 0) return -1;

if (*computerSystem == NULL) { diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index 74a74e0e09..8c9c5ed9c1 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -198,40 +198,18 @@ const char *hypervReturnCodeToString(int returnCode); * Generic "Get WMI class list" helpers */

-int hypervGetMsvmComputerSystemList(hypervPrivate *priv, virBufferPtr query, - Msvm_ComputerSystem **list); - -int hypervGetMsvmConcreteJobList(hypervPrivate *priv, virBufferPtr query, - Msvm_ConcreteJob **list); - -int hypervGetWin32ComputerSystemList(hypervPrivate *priv, virBufferPtr query, - Win32_ComputerSystem **list); - -int hypervGetWin32ProcessorList(hypervPrivate *priv, virBufferPtr query, - Win32_Processor **list); - -int hypervGetMsvmVirtualSystemSettingDataList(hypervPrivate *priv, - virBufferPtr query, - Msvm_VirtualSystemSettingData **list); +int hypervGetWmiClassList(hypervPrivate *priv, + hypervWmiClassInfoListPtr wmiInfo, virBufferPtr query, + hypervObject **wmiClass);

int hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv, const char *uuid_string, Msvm_VirtualSystemSettingData **list);

-int hypervGetMsvmProcessorSettingDataList(hypervPrivate *priv, - virBufferPtr query, - Msvm_ProcessorSettingData **list); - -int hypervGetMsvmMemorySettingDataList(hypervPrivate *priv, virBufferPtr query, - Msvm_MemorySettingData **list); - int hypervGetMsvmMemorySettingDataFromVSSD(hypervPrivate *priv, const char *vssd_instanceid, Msvm_MemorySettingData **list);

-int hypervGetMsvmKeyboardList(hypervPrivate *priv, virBufferPtr query, - Msvm_Keyboard **list); - /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Msvm_ComputerSystem */ -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

Co-authored-by: Sri Ramanujam <sramanujam@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 90 +++++++++++++++++++++++++++++++++++++ src/hyperv/hyperv_private.h | 2 + 2 files changed, 92 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 3e4563252e..93e08c54c0 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -36,6 +36,7 @@ #include "openwsman.h" #include "virstring.h" #include "virkeycode.h" +#include "domain_conf.h"

#define VIR_FROM_THIS VIR_FROM_HYPERV

@@ -283,6 +284,76 @@ hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id,

+/* + * API-specific utility functions + */ + +static int +hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) +{ + Win32_ComputerSystemProduct *computerSystem = NULL; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + int result = -1; + + virBufferAddLit(&query, WIN32_COMPUTERSYSTEMPRODUCT_WQL_SELECT); + if (hypervGetWmiClass(Win32_ComputerSystemProduct, &computerSystem) < 0) + goto cleanup; + + if (virUUIDParse(computerSystem->data.common->UUID, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + computerSystem->data.common->UUID); + goto cleanup; + } + result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *) computerSystem); + + return result; +} + +static virCapsPtr +hypervCapsInit(hypervPrivate *priv) +{ + virCapsPtr caps = NULL; + virCapsGuestPtr guest = NULL; + + caps = virCapabilitiesNew(VIR_ARCH_X86_64, 1, 1); + + if (!caps) + return NULL; + + if (hypervLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) + goto error; + + /* i686 caps */ + guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_I686, + NULL, NULL, 0, NULL); + if (!guest) + goto error; + + if (!virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV, NULL, NULL, 0, NULL)) + goto error; + + /* x86_64 caps */ + guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_X86_64, + NULL, NULL, 0, NULL); + if (!guest) + goto error; + + if (!virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV, NULL, NULL, 0, NULL)) + goto error; + + return caps; + + error: + virObjectUnref(caps); + return NULL; +} + + + /* * Driver functions */ @@ -298,6 +369,9 @@ hypervFreePrivate(hypervPrivate **priv) wsmc_release((*priv)->client); }

+ if ((*priv)->caps) + virObjectUnref((*priv)->caps); + hypervFreeParsedUri(&(*priv)->parsedUri); VIR_FREE(*priv); } @@ -408,6 +482,11 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, if (hypervInitConnection(conn, priv, username, password) < 0) goto cleanup;

+ /* set up capabilities */ + priv->caps = hypervCapsInit(priv); + if (!priv->caps) + goto cleanup; + conn->privateData = priv; priv = NULL; result = VIR_DRV_OPEN_SUCCESS; @@ -464,6 +543,16 @@ hypervConnectGetHostname(virConnectPtr conn)

+static char* +hypervConnectGetCapabilities(virConnectPtr conn) +{ + hypervPrivate *priv = conn->privateData; + + return virCapabilitiesFormatXML(priv->caps); +} + + + static int hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { @@ -1601,6 +1690,7 @@ static virHypervisorDriver hypervHypervisorDriver = { .connectGetType = hypervConnectGetType, /* 0.9.5 */ .connectGetHostname = hypervConnectGetHostname, /* 0.9.5 */ .nodeGetInfo = hypervNodeGetInfo, /* 0.9.5 */ + .connectGetCapabilities = hypervConnectGetCapabilities, /* 6.9.0 */ .connectListDomains = hypervConnectListDomains, /* 0.9.5 */ .connectNumOfDomains = hypervConnectNumOfDomains, /* 0.9.5 */ .connectListAllDomains = hypervConnectListAllDomains, /* 0.10.2 */ diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index b502e71d83..cf08bf542b 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -26,6 +26,7 @@ #include "virerror.h" #include "hyperv_util.h" #include "openwsman.h" +#include "capabilities.h"

typedef enum _hypervWmiVersion hypervWmiVersion; enum _hypervWmiVersion { @@ -38,4 +39,5 @@ struct _hypervPrivate { hypervParsedUri *parsedUri; WsManClient *client; hypervWmiVersion wmiVersion; + virCapsPtr caps; }; -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

Co-authored-by: Sri Ramanujam <sramanujam@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 93e08c54c0..bbe892fd62 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -553,6 +553,39 @@ hypervConnectGetCapabilities(virConnectPtr conn)

+static int +hypervConnectGetMaxVcpus(virConnectPtr conn, const char *type G_GNUC_UNUSED) +{ + int result = -1; + hypervPrivate *priv = conn->privateData; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + Msvm_ProcessorSettingData *processorSettingData = NULL; + + /* Get max processors definition */ + virBufferAddLit(&query, + MSVM_PROCESSORSETTINGDATA_WQL_SELECT + "WHERE InstanceID LIKE 'Microsoft:Definition%Maximum'"); + + if (hypervGetWmiClass(Msvm_ProcessorSettingData, &processorSettingData) < 0) + goto cleanup; + + if (!processorSettingData) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get maximum definition of Msvm_ProcessorSettingData for host %s"), + conn->uri->server); + goto cleanup; + } + + result = processorSettingData->data.common->VirtualQuantity; + + cleanup: + hypervFreeObject(priv, (hypervObject *) processorSettingData); + + return result; +} + + + static int hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { @@ -1689,6 +1722,7 @@ static virHypervisorDriver hypervHypervisorDriver = { .connectClose = hypervConnectClose, /* 0.9.5 */ .connectGetType = hypervConnectGetType, /* 0.9.5 */ .connectGetHostname = hypervConnectGetHostname, /* 0.9.5 */ + .connectGetMaxVcpus = hypervConnectGetMaxVcpus, /* 6.9.0 */ .nodeGetInfo = hypervNodeGetInfo, /* 0.9.5 */ .connectGetCapabilities = hypervConnectGetCapabilities, /* 6.9.0 */ .connectListDomains = hypervConnectListDomains, /* 0.9.5 */ -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

CurrentTimeZone's type is a signed integer, not unsigned.

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi_generator.input | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_wmi_generator.input b/src/hyperv/hyperv_wmi_generator.input index 77543cf6ef..bbca550790 100644 --- a/src/hyperv/hyperv_wmi_generator.input +++ b/src/hyperv/hyperv_wmi_generator.input @@ -673,7 +673,7 @@ class Win32_OperatingSystem string CSCreationClassName string CSDVersion string CSName - uint16 CurrentTimeZone + int16 CurrentTimeZone boolean DataExecutionPrevention_Available boolean DataExecutionPrevention_32BitApplications boolean DataExecutionPrevention_Drivers -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:22 PM Matt Coleman <mcoleman@datto.com> wrote:

Hyper-V version numbers are not compatible with the encoding in virParseVersionString(): https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virutil.c#L246

For example, the Windows Server 2016 Hyper-V version is 10.0.14393: its micro is over 14 times larger than the encoding allows.

This commit repacks the Hyper-V version number in order to preserve all of the digits. The major and minor are concatenated (with minor zero- padded to two digits) to form the repacked major value. This works because Microsoft's major and minor versions numbers are unlikely to exceed 99. The repacked minor value is derived from the digits in the thousands, ten-thousands, and hundred-thousands places of Hyper-V's micro. The repacked micro is derived from the digits in the ones, tens, and hundreds places of Hyper-V's micro.

Co-authored-by: Sri Ramanujam <sramanujam@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- docs/drvhyperv.html.in | 50 +++++++++++++++++++++++ src/hyperv/hyperv_driver.c | 81 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+)

diff --git a/docs/drvhyperv.html.in b/docs/drvhyperv.html.in index 78f61aaee2..e8e2113f70 100644 --- a/docs/drvhyperv.html.in +++ b/docs/drvhyperv.html.in @@ -112,4 +112,54 @@ winrm set winrm/config/service @{AllowUnencrypted="true"} </pre>

+ <h2><a id="versions">Version Numbers</a></h2> + <p> + Since Microsoft's build numbers are almost always over 1000, this driver + needs to pack the value differently compared to the format defined by + <code>virConnectGetVersion</code>. + To preserve all of the digits, the following format is used: + </p> + <pre>major * 100000000 + minor * 1000000 + micro</pre> + <p> + This results in <code>virsh version</code> producing unexpected output. + </p> + <table class="top_table"> + <thead> + <th>Windows Release</th> + <th>Kernel Version</th> + <th>libvirt Representation</th> + </thead> + <tr> + <td>Windows Server 2008</td> + <td>6.0.6001</td> + <td>600.6.1</td> + </tr> + <tr> + <td>Windows Server 2008 R2</td> + <td>6.1.7600</td> + <td>601.7.600</td> + </tr> + <tr> + <td>Windows Server 2012</td> + <td>6.2.9200</td> + <td>602.9.200</td> + </tr> + <tr> + <td>Windows Server 2012 R2</td> + <td>6.3.9600</td> + <td>603.9.600</td> + </tr> + <tr> + <td>Windows Server 2016</td> + <td>10.0.14393</td> + <td>1000.14.393</td> + </tr> + <tr> + <td>Windows Server 2019</td> + <td>10.0.17763</td> + <td>1000.17.763</td> + </tr> + </table> + + </body></html> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index bbe892fd62..528c826e16 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -29,6 +29,7 @@ #include "viralloc.h" #include "virlog.h" #include "viruuid.h" +#include "virutil.h" #include "hyperv_driver.h" #include "hyperv_private.h" #include "hyperv_util.h" @@ -288,6 +289,24 @@ hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id, * API-specific utility functions */

+static int +hypervParseVersionString(const char *str, unsigned int *major, + unsigned int *minor, unsigned int *micro) +{ + char *suffix = NULL; + + if (virStrToLong_ui(str, &suffix, 10, major) < 0) + return -1; + + if (virStrToLong_ui(suffix + 1, &suffix, 10, minor) < 0) + return -1; + + if (virStrToLong_ui(suffix + 1, NULL, 10, micro) < 0) + return -1; + + return 0; +} + static int hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) { @@ -523,6 +542,67 @@ hypervConnectGetType(virConnectPtr conn G_GNUC_UNUSED)

+static int +hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) +{ + int result = -1; + hypervPrivate *priv = conn->privateData; + Win32_OperatingSystem *os = NULL; + g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 }; + unsigned int major, minor, micro; + + if (hypervGetWmiClass(Win32_OperatingSystem, &os) < 0 || + !os) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get version information for host %s"), + conn->uri->server); + goto cleanup; + } + + if (hypervParseVersionString(os->data.common->Version, + &major, &minor, µ) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not parse version from '%s'"), + os->data.common->Version); + goto cleanup; + } + + /* + * Pack the version into an unsigned long while retaining all the digits. + * + * Since Microsoft's build numbers are almost always over 1000, this driver + * needs to pack the value differently compared to the format defined by + * virConnectGetVersion(). + * + * This results in `virsh version` producing unexpected output. + * + * For example... + * 2008: 6.0.6001 => 600.6.1 + * 2008 R2: 6.1.7600 => 601.7.600 + * 2012: 6.2.9200 => 602.9.200 + * 2012 R2: 6.3.9600 => 603.9.600 + * 2016: 10.0.14393 => 1000.14.393 + * 2019: 10.0.17763 => 1000.17.763 + */ + if (major > 99 || minor > 99 || micro > 999999) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not produce packed version number from '%s'"), + os->data.common->Version); + goto cleanup; + } + + *version = major * 100000000 + minor * 1000000 + micro; + + result = 0; + + cleanup: + hypervFreeObject(priv, (hypervObject *) os); + + return result; +} + + + static char * hypervConnectGetHostname(virConnectPtr conn) { @@ -1721,6 +1801,7 @@ static virHypervisorDriver hypervHypervisorDriver = { .connectOpen = hypervConnectOpen, /* 0.9.5 */ .connectClose = hypervConnectClose, /* 0.9.5 */ .connectGetType = hypervConnectGetType, /* 0.9.5 */ + .connectGetVersion = hypervConnectGetVersion, /* 6.9.0 */ .connectGetHostname = hypervConnectGetHostname, /* 0.9.5 */ .connectGetMaxVcpus = hypervConnectGetMaxVcpus, /* 6.9.0 */ .nodeGetInfo = hypervNodeGetInfo, /* 0.9.5 */ -- 2.27.0

Ugh, this is kind of gross, but it makes sense... Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:21 PM Matt Coleman <mcoleman@datto.com> wrote:

Co-authored-by: Sri Ramanujam <sramanujam@datto.com> Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 528c826e16..dcde469442 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1274,6 +1274,45 @@ hypervDomainCreate(virDomainPtr domain)

+static int +hypervDomainGetAutostart(virDomainPtr domain, int *autostart) +{ + int result = -1; + char uuid_string[VIR_UUID_STRING_BUFLEN]; + hypervPrivate *priv = domain->conn->privateData; + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + Msvm_VirtualSystemGlobalSettingData *vsgsd = NULL; + Msvm_VirtualSystemSettingData *vssd = NULL; + + virUUIDFormat(domain->uuid, uuid_string); + + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + virBufferEscapeSQL(&query, + MSVM_VIRTUALSYSTEMGLOBALSETTINGDATA_WQL_SELECT + "WHERE SystemName = \"%s\"", uuid_string); + + if (hypervGetWmiClass(Msvm_VirtualSystemGlobalSettingData, &vsgsd) < 0) + goto cleanup; + + *autostart = vsgsd->data.common->AutomaticStartupAction == 2; + result = 0; + } else { + if (hypervGetVSSDFromUUID(priv, uuid_string, &vssd) < 0) + goto cleanup; + + *autostart = vssd->data.v2->AutomaticStartupAction == 4; + result = 0; + } + + cleanup: + hypervFreeObject(priv, (hypervObject *) vsgsd); + hypervFreeObject(priv, (hypervObject *) vssd); + + return result; +} + + + static int hypervConnectIsEncrypted(virConnectPtr conn) { @@ -1824,6 +1863,7 @@ static virHypervisorDriver hypervHypervisorDriver = { .connectNumOfDefinedDomains = hypervConnectNumOfDefinedDomains, /* 0.9.5 */ .domainCreate = hypervDomainCreate, /* 0.9.5 */ .domainCreateWithFlags = hypervDomainCreateWithFlags, /* 0.9.5 */ + .domainGetAutostart = hypervDomainGetAutostart, /* 6.9.0 */ .connectIsEncrypted = hypervConnectIsEncrypted, /* 0.9.5 */ .connectIsSecure = hypervConnectIsSecure, /* 0.9.5 */ .domainIsActive = hypervDomainIsActive, /* 0.9.5 */ -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!

On Mon, Oct 5, 2020 at 12:22 PM Matt Coleman <mcoleman@datto.com> wrote:

Signed-off-by: Matt Coleman <matt@datto.com> --- NEWS.rst | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst index b2ed661c8e..e708f06e9e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -19,10 +19,20 @@ v6.9.0 (unreleased) local file-backed disks to configure a disk which discards changes made to it while the VM was active.

+ * hyperv: implement new APIs + + The ``virConnectGetCapabilities()``, ``virConnectGetMaxVcpus()``, + ``virConnectGetVersion()``, and ``virDomainGetAutostart()`` APIs have been + implemented in the Hyper-V driver. + * **Improvements**

* **Bug fixes**

+ * hyperv: ensure WQL queries work in all locales + + Relying on the "Description" field caused queries to fail on non-"en-US" + systems. The queries have been updated to avoid using localized strings.

v6.8.0 (2020-10-01) =================== -- 2.27.0

Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!