[PATCH 0/6] Hyper-V code cleanup

Here's a draft GitLab MR if you'd prefer to review the changes there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs Matt Coleman (6): hyperv: reformat WQL query strings hyperv: remove duplicate function hypervGetVSSDFromUUID() hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc() hyperv: reduce duplicate code for Msvm_ComputerSystem lookups hyperv: do not overwrite errors from hypervInvokeMethod() src/hyperv/hyperv_driver.c | 169 +++++++++---------------------------- src/hyperv/hyperv_wmi.c | 57 +++++++------ src/hyperv/hyperv_wmi.h | 4 + 3 files changed, 75 insertions(+), 155 deletions(-) -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 50 +++++++++++++++++--------------------- src/hyperv/hyperv_wmi.c | 29 +++++++++++----------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 002434c56a..dbc864b6fa 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -68,7 +68,7 @@ hypervGetProcessorsByName(hypervPrivate *priv, const char *name, { g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Win32_ComputerSystem.Name=\"%s\"} " + "ASSOCIATORS OF {Win32_ComputerSystem.Name='%s'} " "WHERE AssocClass = Win32_ComputerSystemProcessor " "ResultClass = Win32_Processor", name); @@ -180,7 +180,7 @@ hypervGetVirtualSystemByUUID(hypervPrivate *priv, const char *uuid, virBufferEscapeSQL(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL - "AND Name = \"%s\"", + "AND Name = '%s'", uuid); if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) @@ -204,7 +204,7 @@ hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, virBufferEscapeSQL(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL - "AND ElementName = \"%s\"", + "AND ElementName = '%s'", name); if (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) @@ -226,7 +226,7 @@ hypervGetVSSDFromUUID(hypervPrivate *priv, const char *uuid, { g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\",Name=\"%s\"} " + "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName='Msvm_ComputerSystem',Name='%s'} " "WHERE AssocClass = Msvm_SettingsDefineState " "ResultClass = Msvm_VirtualSystemSettingData", uuid); @@ -251,7 +251,7 @@ hypervGetProcSDByVSSDInstanceId(hypervPrivate *priv, const char *id, { g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " "ResultClass = Msvm_ProcessorSettingData", id); @@ -276,7 +276,7 @@ hypervGetMemSDByVSSDInstanceId(hypervPrivate *priv, const char *id, { g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " "ResultClass = Msvm_MemorySettingData", id); @@ -346,10 +346,9 @@ static int hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) { Win32_ComputerSystemProduct *computerSystem = NULL; - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) query = { g_string_new(WIN32_COMPUTERSYSTEMPRODUCT_WQL_SELECT), 0 }; int result = -1; - virBufferAddLit(&query, WIN32_COMPUTERSYSTEMPRODUCT_WQL_SELECT); if (hypervGetWmiClass(Win32_ComputerSystemProduct, &computerSystem) < 0) goto cleanup; @@ -459,18 +458,18 @@ hypervInitConnection(virConnectPtr conn, hypervPrivate *priv, wqlQuery.info = Msvm_ComputerSystem_WmiInfo; wqlQuery.query = &query; - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "WHERE "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_PHYSICAL); + virBufferAddLit(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_PHYSICAL); /* try query using V2 namespace (for Hyper-V 2012+) */ priv->wmiVersion = HYPERV_WMI_VERSION_V2; if (hypervEnumAndPull(priv, &wqlQuery, &computerSystem) < 0) { /* rebuild query because hypervEnumAndPull consumes it */ - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "WHERE "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_PHYSICAL); + virBufferAddLit(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_PHYSICAL); /* fall back to V1 namespace (for Hyper-V 2008) */ priv->wmiVersion = HYPERV_WMI_VERSION_V1; @@ -1390,7 +1389,7 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart) if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { virBufferEscapeSQL(&query, MSVM_VIRTUALSYSTEMGLOBALSETTINGDATA_WQL_SELECT - "WHERE SystemName = \"%s\"", uuid_string); + "WHERE SystemName = '%s'", uuid_string); if (hypervGetWmiClass(Msvm_VirtualSystemGlobalSettingData, &vsgsd) < 0) goto cleanup; @@ -1722,21 +1721,17 @@ hypervConnectListAllDomains(virConnectPtr conn, goto cleanup; } - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferAddLit(&query, "where "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE " 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); + virBufferAddLit(&query, "AND " MSVM_COMPUTERSYSTEM_WQL_ACTIVE); } if (MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { - virBufferAddLit(&query, "and "); - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_INACTIVE); + virBufferAddLit(&query, "AND " MSVM_COMPUTERSYSTEM_WQL_INACTIVE); } } @@ -1840,10 +1835,8 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, goto cleanup; virBufferEscapeSQL(&query, - "associators of " - "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," - "Name=\"%s\"} " - "where ResultClass = Msvm_Keyboard", + "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName='Msvm_ComputerSystem',Name='%s'} " + "WHERE ResultClass = Msvm_Keyboard", uuid_string); if (hypervGetWmiClass(Msvm_Keyboard, &keyboard) < 0) @@ -1961,8 +1954,9 @@ hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, if (!params) goto cleanup; - virBufferAddLit(&eprQuery, MSVM_COMPUTERSYSTEM_WQL_SELECT); - virBufferEscapeSQL(&eprQuery, "where Name = \"%s\"", uuid_string); + virBufferEscapeSQL(&eprQuery, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE Name = '%s'", uuid_string); if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery, Msvm_ComputerSystem_WmiInfo) < 0) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 421cbfa31b..d804558fe4 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -914,8 +914,9 @@ hypervInvokeMethod(hypervPrivate *priv, * side! That is up to Windows to control, we don't do anything about it. */ while (!completed && timeout >= 0) { - virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); - virBufferEscapeSQL(&query, "where InstanceID = \"%s\"", instanceID); + virBufferEscapeSQL(&query, + MSVM_CONCRETEJOB_WQL_SELECT + "WHERE InstanceID = '%s'", instanceID); if (hypervGetWmiClassList(priv, Msvm_ConcreteJob_WmiInfo, &query, (hypervObject **)&job) < 0 || job == NULL) @@ -1328,8 +1329,9 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, /* FIXME: Poll every 100ms until the job completes or fails. There * seems to be no other way than polling. */ while (!completed) { - virBufferAddLit(&query, MSVM_CONCRETEJOB_WQL_SELECT); - virBufferAsprintf(&query, "where InstanceID = \"%s\"", instanceID); + virBufferAsprintf(&query, + MSVM_CONCRETEJOB_WQL_SELECT + "WHERE InstanceID = '%s'", instanceID); if (hypervGetWmiClassList(priv, Msvm_ConcreteJob_WmiInfo, &query, (hypervObject **)&concreteJob) < 0) @@ -1520,10 +1522,10 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain, 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); + virBufferAsprintf(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND Name = '%s'", uuid_string); if (hypervGetWmiClassList(priv, Msvm_ComputerSystem_WmiInfo, &query, (hypervObject **)computerSystem) < 0) @@ -1550,10 +1552,8 @@ hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv, g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferAsprintf(&query, - "associators of " - "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," - "Name=\"%s\"} " - "where AssocClass = Msvm_SettingsDefineState " + "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName='Msvm_ComputerSystem',Name='%s'} " + "WHERE AssocClass = Msvm_SettingsDefineState " "ResultClass = Msvm_VirtualSystemSettingData", uuid_string); @@ -1576,9 +1576,8 @@ hypervGetMsvmMemorySettingDataFromVSSD(hypervPrivate *priv, g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; virBufferAsprintf(&query, - "associators of " - "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " - "where AssocClass = Msvm_VirtualSystemSettingDataComponent " + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " + "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " "ResultClass = Msvm_MemorySettingData", vssd_instanceid); -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 42 ++++++++------------------------------ 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index dbc864b6fa..3935320ea9 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -220,30 +220,6 @@ hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, } -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 (hypervGetWmiClass(Msvm_VirtualSystemSettingData, data) < 0) - return -1; - - if (!*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, @@ -1096,10 +1072,9 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) goto cleanup; - if (hypervGetVSSDFromUUID(priv, uuid_string, - &virtualSystemSettingData) < 0) { + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, + &virtualSystemSettingData) < 0) goto cleanup; - } if (hypervGetProcSDByVSSDInstanceId(priv, virtualSystemSettingData->data.common->InstanceID, @@ -1182,10 +1157,9 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) goto cleanup; - if (hypervGetVSSDFromUUID(priv, uuid_string, - &virtualSystemSettingData) < 0) { + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, + &virtualSystemSettingData) < 0) goto cleanup; - } if (hypervGetProcSDByVSSDInstanceId(priv, virtualSystemSettingData->data.common->InstanceID, @@ -1397,7 +1371,7 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart) *autostart = vsgsd->data.common->AutomaticStartupAction == 2; result = 0; } else { - if (hypervGetVSSDFromUUID(priv, uuid_string, &vssd) < 0) + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) goto cleanup; *autostart = vssd->data.v2->AutomaticStartupAction == 4; @@ -1446,7 +1420,7 @@ hypervDomainSetAutostart(virDomainPtr domain, int autostart) virUUIDFormat(domain->uuid, uuid_string); - if (hypervGetVSSDFromUUID(priv, uuid_string, &vssd) < 0) + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) goto cleanup; params = hypervCreateInvokeParamsList(priv, methodName, @@ -1721,7 +1695,9 @@ hypervConnectListAllDomains(virConnectPtr conn, goto cleanup; } - virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); + virBufferAddLit(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL); /* construct query with filter depending on flags */ if (!(MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE) && -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 39 ++++++-------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 3935320ea9..6d5e899535 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -246,31 +246,6 @@ hypervGetProcSDByVSSDInstanceId(hypervPrivate *priv, const char *id, } -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 (hypervGetWmiClass(Msvm_MemorySettingData, data) < 0) - return -1; - - if (!*data) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not look up memory setting data with virtual system instance ID '%s'"), - id); - return -1; - } - - return 0; -} - - static int hypervRequestStateChange(virDomainPtr domain, int state) { @@ -1082,11 +1057,10 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) goto cleanup; } - if (hypervGetMemSDByVSSDInstanceId(priv, - virtualSystemSettingData->data.common->InstanceID, - &memorySettingData) < 0) { + if (hypervGetMsvmMemorySettingDataFromVSSD(priv, + virtualSystemSettingData->data.common->InstanceID, + &memorySettingData) < 0) goto cleanup; - } /* Fill struct */ info->state = hypervMsvmComputerSystemEnabledStateToDomainState(computerSystem); @@ -1167,11 +1141,10 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) goto cleanup; } - if (hypervGetMemSDByVSSDInstanceId(priv, - virtualSystemSettingData->data.common->InstanceID, - &memorySettingData) < 0) { + if (hypervGetMsvmMemorySettingDataFromVSSD(priv, + virtualSystemSettingData->data.common->InstanceID, + &memorySettingData) < 0) goto cleanup; - } /* Fill struct */ def->virtType = VIR_DOMAIN_VIRT_HYPERV; -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 6d5e899535..68835cad91 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1053,9 +1053,8 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) if (hypervGetProcSDByVSSDInstanceId(priv, virtualSystemSettingData->data.common->InstanceID, - &processorSettingData) < 0) { + &processorSettingData) < 0) goto cleanup; - } if (hypervGetMsvmMemorySettingDataFromVSSD(priv, virtualSystemSettingData->data.common->InstanceID, @@ -1137,9 +1136,8 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (hypervGetProcSDByVSSDInstanceId(priv, virtualSystemSettingData->data.common->InstanceID, - &processorSettingData) < 0) { + &processorSettingData) < 0) goto cleanup; - } if (hypervGetMsvmMemorySettingDataFromVSSD(priv, virtualSystemSettingData->data.common->InstanceID, -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 26 +------------------------- src/hyperv/hyperv_wmi.c | 36 ++++++++++++++++++++++-------------- src/hyperv/hyperv_wmi.h | 4 ++++ 3 files changed, 27 insertions(+), 39 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 68835cad91..fba1e355db 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -172,30 +172,6 @@ hypervGetVirtualSystemByID(hypervPrivate *priv, int id, } -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 (hypervGetWmiClass(Msvm_ComputerSystem, computerSystemList) < 0) - 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) @@ -806,7 +782,7 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virUUIDFormat(uuid, uuid_string); - if (hypervGetVirtualSystemByUUID(priv, uuid_string, &computerSystem) < 0) + if (hypervMsvmComputerSystemFromUUID(priv, uuid_string, &computerSystem) < 0) goto cleanup; hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index d804558fe4..66aed01832 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1508,32 +1508,27 @@ hypervMsvmComputerSystemToDomain(virConnectPtr conn, int -hypervMsvmComputerSystemFromDomain(virDomainPtr domain, - Msvm_ComputerSystem **computerSystem) +hypervMsvmComputerSystemFromUUID(hypervPrivate *priv, const char *uuid, + Msvm_ComputerSystem **computerSystem) { - hypervPrivate *priv = domain->conn->privateData; - char uuid_string[VIR_UUID_STRING_BUFLEN]; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - if (computerSystem == NULL || *computerSystem != NULL) { + if (!computerSystem || *computerSystem) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } - virUUIDFormat(domain->uuid, uuid_string); - - virBufferAsprintf(&query, - MSVM_COMPUTERSYSTEM_WQL_SELECT - "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL - "AND Name = '%s'", uuid_string); + virBufferEscapeSQL(&query, + MSVM_COMPUTERSYSTEM_WQL_SELECT + "WHERE " MSVM_COMPUTERSYSTEM_WQL_VIRTUAL + "AND Name = '%s'", uuid); if (hypervGetWmiClassList(priv, Msvm_ComputerSystem_WmiInfo, &query, (hypervObject **)computerSystem) < 0) return -1; - if (*computerSystem == NULL) { - virReportError(VIR_ERR_NO_DOMAIN, - _("No domain with UUID %s"), uuid_string); + if (!*computerSystem) { + virReportError(VIR_ERR_NO_DOMAIN, _("No domain with UUID %s"), uuid); return -1; } @@ -1541,6 +1536,19 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain, } +int +hypervMsvmComputerSystemFromDomain(virDomainPtr domain, + Msvm_ComputerSystem **computerSystem) +{ + hypervPrivate *priv = domain->conn->privateData; + char uuidString[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(domain->uuid, uuidString); + + return hypervMsvmComputerSystemFromUUID(priv, uuidString, computerSystem); +} + + /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Msvm_VirtualSystemSettingData */ diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index 570aa07eb8..5b97ab3db9 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -233,5 +233,9 @@ int hypervMsvmComputerSystemToDomain(virConnectPtr conn, Msvm_ComputerSystem *computerSystem, virDomainPtr *domain); +int +hypervMsvmComputerSystemFromUUID(hypervPrivate *priv, const char *uuid, + Msvm_ComputerSystem **computerSystem); + int hypervMsvmComputerSystemFromDomain(virDomainPtr domain, Msvm_ComputerSystem **computerSystem); -- 2.27.0

Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fba1e355db..a71d0d6261 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1800,11 +1800,8 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0) goto cleanup; - if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not press key %d"), - translatedKeycodes[i]); + if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) goto cleanup; - } } /* simulate holdtime by sleeping */ @@ -1823,11 +1820,8 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0) goto cleanup; - if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not release key %s"), keycodeStr); + if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) goto cleanup; - } } result = 0; @@ -1919,10 +1913,8 @@ hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, } } - if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set memory")); + if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) goto cleanup; - } result = 0; -- 2.27.0

On Thu, Oct 22, 2020 at 12:38 PM Matt Coleman <mcoleman@datto.com> wrote:
Here's a draft GitLab MR if you'd prefer to review the changes there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs
Matt Coleman (6): hyperv: reformat WQL query strings hyperv: remove duplicate function hypervGetVSSDFromUUID() hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc() hyperv: reduce duplicate code for Msvm_ComputerSystem lookups hyperv: do not overwrite errors from hypervInvokeMethod()
src/hyperv/hyperv_driver.c | 169 +++++++++---------------------------- src/hyperv/hyperv_wmi.c | 57 +++++++------ src/hyperv/hyperv_wmi.h | 4 + 3 files changed, 75 insertions(+), 155 deletions(-)
-- 2.27.0
Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ!/ Always, there's only one truth!

On 10/22/20 6:38 PM, Matt Coleman wrote:
Here's a draft GitLab MR if you'd prefer to review the changes there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs
Matt Coleman (6): hyperv: reformat WQL query strings hyperv: remove duplicate function hypervGetVSSDFromUUID() hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc() hyperv: reduce duplicate code for Msvm_ComputerSystem lookups hyperv: do not overwrite errors from hypervInvokeMethod()
src/hyperv/hyperv_driver.c | 169 +++++++++---------------------------- src/hyperv/hyperv_wmi.c | 57 +++++++------ src/hyperv/hyperv_wmi.h | 4 + 3 files changed, 75 insertions(+), 155 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> but I don't think these qualify for the freeze, so let me queue them locally and push after the release is over. Michal

On 10/27/20 11:58 AM, Michal Privoznik wrote:
On 10/22/20 6:38 PM, Matt Coleman wrote:
Here's a draft GitLab MR if you'd prefer to review the changes there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs
Matt Coleman (6): hyperv: reformat WQL query strings hyperv: remove duplicate function hypervGetVSSDFromUUID() hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc() hyperv: reduce duplicate code for Msvm_ComputerSystem lookups hyperv: do not overwrite errors from hypervInvokeMethod()
src/hyperv/hyperv_driver.c | 169 +++++++++---------------------------- src/hyperv/hyperv_wmi.c | 57 +++++++------ src/hyperv/hyperv_wmi.h | 4 + 3 files changed, 75 insertions(+), 155 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
but I don't think these qualify for the freeze, so let me queue them locally and push after the release is over.
Pushed now. Michal
participants (3)
-
Matt Coleman
-
Michal Privoznik
-
Neal Gompa