[PATCH 00/55] Hyper-V: code cleanup & prep for future changes
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit. Simplifications: * add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types Changes: * store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016 Matt Coleman (55): hyperv: add a macro for retrieving setting data hyperv: store the Hyper-V version when connecting hyperv: add inheritance to the WMI generator hyperv: store hypervPrivate in hypervObject hyperv: enable use of g_autoptr for hypervObject hyperv: enable use of g_autoptr for the rest of the CIM/WMI classes hyperv: enable automatic cleanup for OpenWSMAN types hyperv: use g_autoptr for Win32_OperatingSystem in hypervConnectOpen hyperv: use g_autoptr for Win32_ComputerSystem in hypervConnectGetHostname hyperv: use g_autoptr for Msvm_ProcessorSettingData in hypervConnectGetMaxVcpus hyperv: use g_autoptr for WMI classes in hypervNodeGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByUUID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByName hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainResume hyperv: use g_autoptr for WMI classes in hypervDomainShutdownFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainDestroyFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetMaxMemory hyperv: use g_autoptr for WMI classes in hypervDomainSetMemoryProperty hyperv: use g_autoptr for Msvm_ComputerSystem in hypervRequestStateChange hyperv: use g_autoptr for Win32_ComputerSystemProduct in hypervLookupHostSystemBiosUuid hyperv: use g_autoptr for Msvm_ResourceAllocationSettingData in hypervDomainAttachPhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainAttachStorage hyperv: use g_autoptr for Msvm_DiskDrive in hypervDomainDefParsePhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainGetState hyperv: use g_autoptr for WMI classes in hypervDomainSetVcpusFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetVcpusFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainCreateWithFlags hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainGetAutostart hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainSetAutostart hyperv: use g_autoptr for WMI classes in hypervDomainGetSchedulerParametersFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainIsActive hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSave hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainHasManagedSaveImage hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSaveRemove hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListAllDomains hyperv: use GLib auto-cleanup in hypervDomainSendKey hyperv: use GLib auto-cleanup in hypervInvokeMethod hyperv: use GLib auto-cleanup in hypervInvokeMsvmComputerSystemRequestStateChange hyperv: use GLib auto-cleanup in hypervMsvmVSMSAddResourceSettings and hypervMsvmVSMSModifyResourceSettings hyperv: use g_autoptr for Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor in hypervDomainGetVcpus hyperv: use g_autoptr for Win32_OperatingSystem in hypervNodeGetFreeMemory hyperv: use GLib auto-cleanup in hypervDomainGetXMLDesc hyperv: use g_autoptr for WMI classes in hypervDomainAttachDeviceFlags hyperv: use GLib auto-cleanup in hypervSerializeEprParam hyperv: use GLib auto-cleanup in hypervEnumAndPull hyperv: use GLib auto-cleanup in hypervSerializeEmbeddedParam hyperv: use GLib auto-cleanup in hypervCreateInvokeXmlDoc hyperv: use g_auto for WsXmlDocH in hypervDomainAttachVirtualDisk hyperv: use g_auto for WsXmlDocH in hypervDomainAttachCDROM scripts/hyperv_wmi_generator.py | 16 +- src/hyperv/hyperv_driver.c | 755 +++++++++++--------------------- src/hyperv/hyperv_private.h | 4 +- src/hyperv/hyperv_wmi.c | 408 +++++++---------- src/hyperv/hyperv_wmi.h | 4 +- src/hyperv/hyperv_wsman.h | 28 ++ 6 files changed, 457 insertions(+), 758 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 52 +++++++++++++---------------------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 4bace10874..d89aeb1874 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1490,20 +1490,23 @@ hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv, } +#define hypervGetSettingData(type, id, out) \ + g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; \ + virBufferEscapeSQL(&query, \ + "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " \ + "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " \ + "ResultClass = " #type, \ + id); \ + if (hypervGetWmiClass(type, out) < 0) \ + return -1 + + int hypervGetResourceAllocationSD(hypervPrivate *priv, const char *id, Msvm_ResourceAllocationSettingData **data) { - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " - "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_ResourceAllocationSettingData", - id); - - if (hypervGetWmiClass(Msvm_ResourceAllocationSettingData, data) < 0) - return -1; + hypervGetSettingData(Msvm_ResourceAllocationSettingData, id, data); if (!*data) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1521,15 +1524,7 @@ hypervGetProcessorSD(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 (hypervGetWmiClass(Msvm_ProcessorSettingData, data) < 0) - return -1; + hypervGetSettingData(Msvm_ProcessorSettingData, id, data); if (!*data) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1547,15 +1542,9 @@ hypervGetMemorySD(hypervPrivate *priv, const char *vssd_instanceid, Msvm_MemorySettingData **list) { - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; + hypervGetSettingData(Msvm_MemorySettingData, vssd_instanceid, list); - virBufferAsprintf(&query, - "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " - "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_MemorySettingData", - vssd_instanceid); - - if (hypervGetWmiClass(Msvm_MemorySettingData, list) < 0 || !*list) + if (!*list) return -1; return 0; @@ -1567,16 +1556,7 @@ hypervGetStorageAllocationSD(hypervPrivate *priv, const char *id, Msvm_StorageAllocationSettingData **data) { - g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - virBufferEscapeSQL(&query, - "ASSOCIATORS OF {Msvm_VirtualSystemSettingData.InstanceID='%s'} " - "WHERE AssocClass = Msvm_VirtualSystemSettingDataComponent " - "ResultClass = Msvm_StorageAllocationSettingData", - id); - - if (hypervGetWmiClass(Msvm_StorageAllocationSettingData, data) < 0) - return -1; - + hypervGetSettingData(Msvm_StorageAllocationSettingData, id, data); return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 62 +++++++++++++++++++++---------------- src/hyperv/hyperv_private.h | 1 + 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2399b5df7d..1ac379c14f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -182,6 +182,18 @@ hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, } +static int +hypervGetOperatingSystem(hypervPrivate *priv, Win32_OperatingSystem **operatingSystem) +{ + g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 }; + + if (hypervGetWmiClass(Win32_OperatingSystem, operatingSystem) < 0) + return -1; + + return 0; +} + + static int hypervRequestStateChange(virDomainPtr domain, int state) { @@ -1300,6 +1312,9 @@ hypervFreePrivate(hypervPrivate **priv) if ((*priv)->xmlopt) virObjectUnref((*priv)->xmlopt); + if ((*priv)->version) + g_free((*priv)->version); + hypervFreeParsedUri(&(*priv)->parsedUri); VIR_FREE(*priv); } @@ -1343,6 +1358,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, hypervPrivate *priv = NULL; g_autofree char *username = NULL; g_autofree char *password = NULL; + Win32_OperatingSystem *os = NULL; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -1389,11 +1405,24 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, /* init xmlopt for domain XML */ priv->xmlopt = virDomainXMLOptionNew(&hypervDomainDefParserConfig, NULL, NULL, NULL, NULL); + if (hypervGetOperatingSystem(priv, &os) < 0) + goto cleanup; + + if (!os) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get version information for host %s"), + conn->uri->server); + goto cleanup; + } + + priv->version = g_strdup(os->data->Version); + conn->privateData = priv; priv = NULL; result = VIR_DRV_OPEN_SUCCESS; cleanup: + hypervFreeObject(priv ? priv : conn->privateData, (hypervObject *)os); hypervFreePrivate(&priv); return result; @@ -1423,27 +1452,14 @@ 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) - goto cleanup; - - if (!os) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not get version information for host %s"), - conn->uri->server); - goto cleanup; - } - - if (hypervParseVersionString(os->data->Version, &major, &minor, µ) < 0) { + if (hypervParseVersionString(priv->version, &major, &minor, µ) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse version from '%s'"), - os->data->Version); - goto cleanup; + priv->version); + return -1; } /* @@ -1466,18 +1482,13 @@ hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) if (major > 99 || minor > 99 || micro > 999999) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not produce packed version number from '%s'"), - os->data->Version); - goto cleanup; + priv->version); + return -1; } *version = major * 100000000 + minor * 1000000 + micro; - result = 0; - - cleanup: - hypervFreeObject(priv, (hypervObject *)os); - - return result; + return 0; } @@ -2865,9 +2876,8 @@ hypervNodeGetFreeMemory(virConnectPtr conn) unsigned long long freeMemoryBytes = 0; hypervPrivate *priv = conn->privateData; Win32_OperatingSystem *operatingSystem = NULL; - g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 }; - if (hypervGetWmiClass(Win32_OperatingSystem, &operatingSystem) < 0) + if (hypervGetOperatingSystem(priv, &operatingSystem) < 0) return 0; if (!operatingSystem) { diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 7a2a1d59ee..58abad61a4 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -36,4 +36,5 @@ struct _hypervPrivate { WsManClient *client; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; + char *version; }; -- 2.30.0
On 1/21/21 1:50 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 62 +++++++++++++++++++++---------------- src/hyperv/hyperv_private.h | 1 + 2 files changed, 37 insertions(+), 26 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2399b5df7d..1ac379c14f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -182,6 +182,18 @@ hypervGetVirtualSystemByName(hypervPrivate *priv, const char *name, }
+static int +hypervGetOperatingSystem(hypervPrivate *priv, Win32_OperatingSystem **operatingSystem) +{ + g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 }; + + if (hypervGetWmiClass(Win32_OperatingSystem, operatingSystem) < 0)
I've always kinda disliked it when macros have implied use of variables (e.g. the way priv and query are used in the macro, but not included in the argument list). Sure, it makes the invocation more compact, but it also confuses the hell out of someone (like me) who's never looked at the code before. But that's not what we're here to discuss - it was already done in the past, so you adding one more use of it is not a problem. I just thought I'd passive-agressively point that out while I'm passing by... :-)
+ return -1; + + return 0; +} + + static int hypervRequestStateChange(virDomainPtr domain, int state) { @@ -1300,6 +1312,9 @@ hypervFreePrivate(hypervPrivate **priv) if ((*priv)->xmlopt) virObjectUnref((*priv)->xmlopt);
+ if ((*priv)->version) + g_free((*priv)->version); + hypervFreeParsedUri(&(*priv)->parsedUri); VIR_FREE(*priv); } @@ -1343,6 +1358,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, hypervPrivate *priv = NULL; g_autofree char *username = NULL; g_autofree char *password = NULL; + Win32_OperatingSystem *os = NULL;
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
@@ -1389,11 +1405,24 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, /* init xmlopt for domain XML */ priv->xmlopt = virDomainXMLOptionNew(&hypervDomainDefParserConfig, NULL, NULL, NULL, NULL);
+ if (hypervGetOperatingSystem(priv, &os) < 0) + goto cleanup; + + if (!os) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get version information for host %s"), + conn->uri->server); + goto cleanup; + } + + priv->version = g_strdup(os->data->Version); + conn->privateData = priv; priv = NULL; result = VIR_DRV_OPEN_SUCCESS;
cleanup: + hypervFreeObject(priv ? priv : conn->privateData, (hypervObject *)os); hypervFreePrivate(&priv);
return result; @@ -1423,27 +1452,14 @@ 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) - goto cleanup; - - if (!os) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not get version information for host %s"), - conn->uri->server); - goto cleanup; - } - - if (hypervParseVersionString(os->data->Version, &major, &minor, µ) < 0) { + if (hypervParseVersionString(priv->version, &major, &minor, µ) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse version from '%s'"), - os->data->Version); - goto cleanup; + priv->version); + return -1; }
/* @@ -1466,18 +1482,13 @@ hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) if (major > 99 || minor > 99 || micro > 999999) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not produce packed version number from '%s'"), - os->data->Version); - goto cleanup; + priv->version); + return -1; }
*version = major * 100000000 + minor * 1000000 + micro;
- result = 0; - - cleanup: - hypervFreeObject(priv, (hypervObject *)os); - - return result; + return 0; }
@@ -2865,9 +2876,8 @@ hypervNodeGetFreeMemory(virConnectPtr conn) unsigned long long freeMemoryBytes = 0; hypervPrivate *priv = conn->privateData; Win32_OperatingSystem *operatingSystem = NULL; - g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 };
- if (hypervGetWmiClass(Win32_OperatingSystem, &operatingSystem) < 0) + if (hypervGetOperatingSystem(priv, &operatingSystem) < 0) return 0;
if (!operatingSystem) { diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 7a2a1d59ee..58abad61a4 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -36,4 +36,5 @@ struct _hypervPrivate { WsManClient *client; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; + char *version; };
This enables casting subtypes to their parent. Signed-off-by: Matt Coleman <matt@datto.com> --- scripts/hyperv_wmi_generator.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/hyperv_wmi_generator.py b/scripts/hyperv_wmi_generator.py index d72e5d8ca1..e5e961d2db 100755 --- a/scripts/hyperv_wmi_generator.py +++ b/scripts/hyperv_wmi_generator.py @@ -221,10 +221,10 @@ def report_error(message): def parse_class(block, number): - # expected format: class <name> + # expected format: class <name> : <optional parent> header_items = block[0][1].split() - if len(header_items) != 2: + if len(header_items) not in [2, 4]: report_error("line %d: invalid block header" % (number)) assert header_items[0] == "class" @@ -234,7 +234,13 @@ def parse_class(block, number): if name in wmi_classes_by_name: report_error("class '%s' has already been defined" % name) - properties = [] + if len(header_items) == 4: + parent_class = header_items[3] + if parent_class not in wmi_classes_by_name: + report_error("nonexistent parent class specified: %s" % parent_class) + properties = wmi_classes_by_name[parent_class].properties.copy() + else: + properties = [] for line in block[1:]: # expected format: <type> <name> -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 131 +++++++++++++++++-------------------- src/hyperv/hyperv_wmi.c | 19 +++--- src/hyperv/hyperv_wmi.h | 3 +- 3 files changed, 74 insertions(+), 79 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 1ac379c14f..680d8b762b 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -198,7 +198,6 @@ static int hypervRequestStateChange(virDomainPtr domain, int state) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) @@ -212,7 +211,7 @@ hypervRequestStateChange(virDomainPtr domain, int state) result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, state); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -260,7 +259,7 @@ hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -642,7 +641,7 @@ hypervDomainAttachPhysicalDisk(virDomainPtr domain, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)diskdefault); + hypervFreeObject((hypervObject *)diskdefault); return result; } @@ -930,8 +929,8 @@ hypervDomainAttachStorage(virDomainPtr domain, virDomainDefPtr def, const char * result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)rasd); - hypervFreeObject(priv, (hypervObject *)vssd); + hypervFreeObject((hypervObject *)rasd); + hypervFreeObject((hypervObject *)vssd); return result; } @@ -1213,7 +1212,7 @@ hypervDomainDefParsePhysicalDisk(hypervPrivate *priv, cleanup: if (result != 0 && disk) virDomainDiskDefFree(disk); - hypervFreeObject(priv, (hypervObject *)diskdrive); + hypervFreeObject((hypervObject *)diskdrive); return result; } @@ -1422,7 +1421,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, result = VIR_DRV_OPEN_SUCCESS; cleanup: - hypervFreeObject(priv ? priv : conn->privateData, (hypervObject *)os); + hypervFreeObject((hypervObject *)os); hypervFreePrivate(&priv); return result; @@ -1505,7 +1504,7 @@ hypervConnectGetHostname(virConnectPtr conn) hostname = g_strdup(computerSystem->data->DNSHostName); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return hostname; } @@ -1546,7 +1545,7 @@ hypervConnectGetMaxVcpus(virConnectPtr conn, const char *type G_GNUC_UNUSED) result = processorSettingData->data->VirtualQuantity; cleanup: - hypervFreeObject(priv, (hypervObject *)processorSettingData); + hypervFreeObject((hypervObject *)processorSettingData); return result; } @@ -1618,8 +1617,8 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)processorList); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)processorList); return result; } @@ -1651,7 +1650,7 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) success = true; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystemList); + hypervFreeObject((hypervObject *)computerSystemList); return success ? count : -1; } @@ -1677,7 +1676,7 @@ hypervConnectNumOfDomains(virConnectPtr conn) success = true; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystemList); + hypervFreeObject((hypervObject *)computerSystemList); return success ? count : -1; } @@ -1696,7 +1695,7 @@ hypervDomainLookupByID(virConnectPtr conn, int id) hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return domain; } @@ -1718,7 +1717,7 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return domain; } @@ -1744,7 +1743,7 @@ hypervDomainLookupByName(virConnectPtr conn, const char *name) hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return domain; } @@ -1761,7 +1760,6 @@ static int hypervDomainResume(virDomainPtr domain) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) @@ -1777,7 +1775,7 @@ hypervDomainResume(virDomainPtr domain) MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -1842,8 +1840,8 @@ hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)shutdown); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)shutdown); return result; } @@ -1876,7 +1874,6 @@ static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; bool in_transition = false; @@ -1896,7 +1893,7 @@ hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -1939,8 +1936,8 @@ hypervDomainGetMaxMemory(virDomainPtr domain) maxMemoryBytes = mem_sd->data->Limit * 1024; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)mem_sd); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)mem_sd); return maxMemoryBytes; } @@ -1984,8 +1981,8 @@ hypervDomainSetMemoryProperty(virDomainPtr domain, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)memsd); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)memsd); return result; } @@ -2056,10 +2053,10 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); - hypervFreeObject(priv, (hypervObject *)processorSettingData); - hypervFreeObject(priv, (hypervObject *)memorySettingData); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)virtualSystemSettingData); + hypervFreeObject((hypervObject *)processorSettingData); + hypervFreeObject((hypervObject *)memorySettingData); return result; } @@ -2070,7 +2067,6 @@ hypervDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; virCheckFlags(0, -1); @@ -2086,7 +2082,7 @@ hypervDomainGetState(virDomainPtr domain, int *state, int *reason, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -2132,8 +2128,8 @@ hypervDomainSetVcpusFlags(virDomainPtr domain, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)proc_sd); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)proc_sd); return result; } @@ -2189,9 +2185,9 @@ hypervDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) result = proc_sd->data->VirtualQuantity; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)proc_sd); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)proc_sd); return result; } @@ -2220,7 +2216,7 @@ hypervDomainGetVcpus(virDomainPtr domain, g_autofree char *vcpu_name = g_strdup_printf("%s:Hv VP %d", domain->name, vcpu_number); /* try to free objects from previous iteration */ - hypervFreeObject(priv, (hypervObject *)vproc); + hypervFreeObject((hypervObject *)vproc); vproc = NULL; /* get the info */ @@ -2246,7 +2242,7 @@ hypervDomainGetVcpus(virDomainPtr domain, count++; } - hypervFreeObject(priv, (hypervObject *)vproc); + hypervFreeObject((hypervObject *)vproc); return count; } @@ -2373,12 +2369,12 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: virDomainDefFree(def); - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData); - hypervFreeObject(priv, (hypervObject *)processorSettingData); - hypervFreeObject(priv, (hypervObject *)memorySettingData); - hypervFreeObject(priv, (hypervObject *)rasd); - hypervFreeObject(priv, (hypervObject *)sasd); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)virtualSystemSettingData); + hypervFreeObject((hypervObject *)processorSettingData); + hypervFreeObject((hypervObject *)memorySettingData); + hypervFreeObject((hypervObject *)rasd); + hypervFreeObject((hypervObject *)sasd); return xml; } @@ -2420,7 +2416,7 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn count = -1; } - hypervFreeObject(priv, (hypervObject *)computerSystemList); + hypervFreeObject((hypervObject *)computerSystemList); return count; } @@ -2446,7 +2442,7 @@ hypervConnectNumOfDefinedDomains(virConnectPtr conn) success = true; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystemList); + hypervFreeObject((hypervObject *)computerSystemList); return success ? count : -1; } @@ -2456,7 +2452,6 @@ static int hypervDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; virCheckFlags(0, -1); @@ -2474,7 +2469,7 @@ hypervDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -2705,9 +2700,9 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)rasd); - hypervFreeObject(priv, (hypervObject *)host); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)rasd); + hypervFreeObject((hypervObject *)host); return result; } @@ -2737,7 +2732,7 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); + hypervFreeObject((hypervObject *)vssd); return result; } @@ -2784,7 +2779,7 @@ hypervDomainSetAutostart(virDomainPtr domain, int autostart) result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); + hypervFreeObject((hypervObject *)vssd); return result; } @@ -2852,9 +2847,9 @@ hypervDomainGetSchedulerParametersFlags(virDomainPtr domain, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); - hypervFreeObject(priv, (hypervObject *)vssd); - hypervFreeObject(priv, (hypervObject *)proc_sd); + hypervFreeObject((hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)vssd); + hypervFreeObject((hypervObject *)proc_sd); return result; } @@ -2889,7 +2884,7 @@ hypervNodeGetFreeMemory(virConnectPtr conn) freeMemoryBytes = operatingSystem->data->FreePhysicalMemory * 1024; - hypervFreeObject(priv, (hypervObject *)operatingSystem); + hypervFreeObject((hypervObject *)operatingSystem); return freeMemoryBytes; } @@ -2941,7 +2936,6 @@ static int hypervDomainIsActive(virDomainPtr domain) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) @@ -2950,7 +2944,7 @@ hypervDomainIsActive(virDomainPtr domain) result = hypervIsMsvmComputerSystemActive(computerSystem, NULL) ? 1 : 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -2985,7 +2979,6 @@ static int hypervDomainManagedSave(virDomainPtr domain, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; bool in_transition = false; @@ -3004,7 +2997,7 @@ hypervDomainManagedSave(virDomainPtr domain, unsigned int flags) result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_OFFLINE); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -3014,7 +3007,6 @@ static int hypervDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; virCheckFlags(0, -1); @@ -3025,7 +3017,7 @@ hypervDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) result = computerSystem->data->EnabledState == MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED ? 1 : 0; cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -3035,7 +3027,6 @@ static int hypervDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) { int result = -1; - hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; virCheckFlags(0, -1); @@ -3053,7 +3044,7 @@ hypervDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); cleanup: - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)computerSystem); return result; } @@ -3179,7 +3170,7 @@ hypervConnectListAllDomains(virConnectPtr conn, VIR_FREE(doms); } - hypervFreeObject(priv, (hypervObject *)computerSystemList); + hypervFreeObject((hypervObject *)computerSystemList); return ret; } @@ -3283,8 +3274,8 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, cleanup: VIR_FREE(translatedKeycodes); VIR_FREE(selector); - hypervFreeObject(priv, (hypervObject *)keyboard); - hypervFreeObject(priv, (hypervObject *)computerSystem); + hypervFreeObject((hypervObject *)keyboard); + hypervFreeObject((hypervObject *)computerSystem); return result; } diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index d89aeb1874..241993f2be 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -870,7 +870,7 @@ hypervInvokeMethod(hypervPrivate *priv, case MSVM_CONCRETEJOB_JOBSTATE_STARTING: case MSVM_CONCRETEJOB_JOBSTATE_RUNNING: case MSVM_CONCRETEJOB_JOBSTATE_SHUTTING_DOWN: - hypervFreeObject(priv, (hypervObject *)job); + hypervFreeObject((hypervObject *)job); job = NULL; g_usleep(100 * 1000); /* sleep 100 ms */ timeout -= 100; @@ -917,7 +917,7 @@ hypervInvokeMethod(hypervPrivate *priv, VIR_FREE(jobcode_instance_xpath); VIR_FREE(returnValue); VIR_FREE(instanceID); - hypervFreeObject(priv, (hypervObject *)job); + hypervFreeObject((hypervObject *)job); *paramsPtr = NULL; return result; } @@ -1030,6 +1030,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, object = g_new0(hypervObject, 1); object->info = wmiInfo; object->data = data; + object->priv = priv; if (head == NULL) { head = object; @@ -1061,14 +1062,14 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, VIR_FREE(query_string); ws_xml_destroy_doc(response); VIR_FREE(enumContext); - hypervFreeObject(priv, head); + hypervFreeObject(head); return result; } void -hypervFreeObject(hypervPrivate *priv G_GNUC_UNUSED, hypervObject *object) +hypervFreeObject(hypervObject *object) { hypervObject *next; WsSerializerContextH serializerContext; @@ -1076,11 +1077,13 @@ hypervFreeObject(hypervPrivate *priv G_GNUC_UNUSED, hypervObject *object) if (object == NULL) return; - serializerContext = wsmc_get_serialization_context(priv->client); + serializerContext = wsmc_get_serialization_context(object->priv->client); while (object != NULL) { next = object->next; + object->priv = NULL; + if (ws_serializer_free_mem(serializerContext, object->data, object->info->serializerInfo) < 0) { VIR_ERROR(_("Could not free deserialized data")); @@ -1267,7 +1270,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, case MSVM_CONCRETEJOB_JOBSTATE_STARTING: case MSVM_CONCRETEJOB_JOBSTATE_RUNNING: case MSVM_CONCRETEJOB_JOBSTATE_SHUTTING_DOWN: - hypervFreeObject(priv, (hypervObject *)concreteJob); + hypervFreeObject((hypervObject *)concreteJob); concreteJob = NULL; g_usleep(100 * 1000); @@ -1312,7 +1315,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, VIR_FREE(properties); VIR_FREE(returnValue); VIR_FREE(instanceID); - hypervFreeObject(priv, (hypervObject *)concreteJob); + hypervFreeObject((hypervObject *)concreteJob); return result; } @@ -1610,7 +1613,7 @@ hypervMsvmVSMSAddResourceSettings(virDomainPtr domain, result = 0; cleanup: - hypervFreeObject(priv, (hypervObject *)vssd); + hypervFreeObject((hypervObject *)vssd); *resourceSettingsPtr = NULL; return result; diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index ff96b8063d..11898f46e2 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -50,6 +50,7 @@ struct _hypervObject { XML_TYPE_PTR data; /* Unserialized data from wsman response */ hypervWmiClassInfoPtr info; /* The info used to make wsman request */ hypervObject *next; + hypervPrivate *priv; }; typedef struct _hypervWqlQuery hypervWqlQuery; @@ -62,7 +63,7 @@ struct _hypervWqlQuery { int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, hypervObject **list); -void hypervFreeObject(hypervPrivate *priv, hypervObject *object); +void hypervFreeObject(hypervObject *object); /* -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 3 +-- src/hyperv/hyperv_wmi.h | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 241993f2be..2a4377a12f 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -939,7 +939,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, filter_t *filter = NULL; WsXmlDocH response = NULL; char *enumContext = NULL; - hypervObject *head = NULL; + g_autoptr(hypervObject) head = NULL; hypervObject *tail = NULL; WsXmlNodeH node = NULL; hypervObject *object; @@ -1062,7 +1062,6 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, VIR_FREE(query_string); ws_xml_destroy_doc(response); VIR_FREE(enumContext); - hypervFreeObject(head); return result; } diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index 11898f46e2..c9b2728df3 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -64,6 +64,7 @@ int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, hypervObject **list); void hypervFreeObject(hypervObject *object); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(hypervObject, hypervFreeObject); /* -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- scripts/hyperv_wmi_generator.py | 4 ++++ src/hyperv/hyperv_wmi.c | 12 ++++++------ src/hyperv/hyperv_wmi.h | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/hyperv_wmi_generator.py b/scripts/hyperv_wmi_generator.py index e5e961d2db..f26558cb56 100755 --- a/scripts/hyperv_wmi_generator.py +++ b/scripts/hyperv_wmi_generator.py @@ -98,6 +98,8 @@ class WmiClass: typedef = "typedef struct _%s %s;\n" % (self.name, self.name) typedef += "typedef struct _%s_Data %s_Data;\n" % (self.name, self.name) + typedef += "G_DEFINE_AUTOPTR_CLEANUP_FUNC(%s, hypervFreeObject);\n" % self.name + typedef += "\n" return typedef @@ -308,6 +310,8 @@ def main(): classes_header.write(notice) classes_source.write(notice) + classes_typedef.write("void hypervFreeObject(void *object);\n\n\n") + names = sorted(wmi_classes_by_name.keys()) for name in names: diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 2a4377a12f..0a9d4bf4fd 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1068,7 +1068,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, void -hypervFreeObject(hypervObject *object) +hypervFreeObject(void *object) { hypervObject *next; WsSerializerContextH serializerContext; @@ -1076,15 +1076,15 @@ hypervFreeObject(hypervObject *object) if (object == NULL) return; - serializerContext = wsmc_get_serialization_context(object->priv->client); + serializerContext = wsmc_get_serialization_context(((hypervObject *)object)->priv->client); while (object != NULL) { - next = object->next; + next = ((hypervObject *)object)->next; - object->priv = NULL; + ((hypervObject *)object)->priv = NULL; - if (ws_serializer_free_mem(serializerContext, object->data, - object->info->serializerInfo) < 0) { + if (ws_serializer_free_mem(serializerContext, ((hypervObject *)object)->data, + ((hypervObject *)object)->info->serializerInfo) < 0) { VIR_ERROR(_("Could not free deserialized data")); } diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h index c9b2728df3..f09948895e 100644 --- a/src/hyperv/hyperv_wmi.h +++ b/src/hyperv/hyperv_wmi.h @@ -63,7 +63,7 @@ struct _hypervWqlQuery { int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, hypervObject **list); -void hypervFreeObject(hypervObject *object); +void hypervFreeObject(void *object); G_DEFINE_AUTOPTR_CLEANUP_FUNC(hypervObject, hypervFreeObject); -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_private.h | 3 +-- src/hyperv/hyperv_wsman.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 58abad61a4..a8855e8118 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -22,11 +22,10 @@ #pragma once -#include <wsman-api.h> - #include "internal.h" #include "virerror.h" #include "hyperv_util.h" +#include "hyperv_wsman.h" #include "capabilities.h" #include "domain_conf.h" diff --git a/src/hyperv/hyperv_wsman.h b/src/hyperv/hyperv_wsman.h new file mode 100644 index 0000000000..c69619d2c1 --- /dev/null +++ b/src/hyperv/hyperv_wsman.h @@ -0,0 +1,28 @@ +/* + * hyperv_wsman.h: OpenWSMAN include and GLib auto-cleanup + * + * Copyright (C) 2021 Datto Inc + * + * 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/>. + * + */ + +#pragma once + +#include <wsman-api.h> + +G_DEFINE_AUTO_CLEANUP_FREE_FUNC(WsXmlDocH, ws_xml_destroy_doc, NULL); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(client_opt_t, wsmc_options_destroy); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(filter_t, filter_destroy); -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 680d8b762b..830a3414c5 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1357,7 +1357,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, hypervPrivate *priv = NULL; g_autofree char *username = NULL; g_autofree char *password = NULL; - Win32_OperatingSystem *os = NULL; + g_autoptr(Win32_OperatingSystem) os = NULL; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -1421,7 +1421,6 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, result = VIR_DRV_OPEN_SUCCESS; cleanup: - hypervFreeObject((hypervObject *)os); hypervFreePrivate(&priv); return result; -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 830a3414c5..8b59dd05f7 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1493,19 +1493,12 @@ hypervConnectGetVersion(virConnectPtr conn, unsigned long *version) static char * hypervConnectGetHostname(virConnectPtr conn) { - char *hostname = NULL; - hypervPrivate *priv = conn->privateData; - Win32_ComputerSystem *computerSystem = NULL; - - if (hypervGetPhysicalSystemList(priv, &computerSystem) < 0) - goto cleanup; - - hostname = g_strdup(computerSystem->data->DNSHostName); + g_autoptr(Win32_ComputerSystem) computerSystem = NULL; - cleanup: - hypervFreeObject((hypervObject *)computerSystem); + if (hypervGetPhysicalSystemList((hypervPrivate *)conn->privateData, &computerSystem) < 0) + return NULL; - return hostname; + return g_strdup(computerSystem->data->DNSHostName); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 8b59dd05f7..6375f6b011 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1514,10 +1514,9 @@ 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; + g_autoptr(Msvm_ProcessorSettingData) processorSettingData = NULL; /* Get max processors definition */ virBufferAddLit(&query, @@ -1525,21 +1524,16 @@ hypervConnectGetMaxVcpus(virConnectPtr conn, const char *type G_GNUC_UNUSED) "WHERE InstanceID LIKE 'Microsoft:Definition%Maximum'"); if (hypervGetWmiClass(Msvm_ProcessorSettingData, &processorSettingData) < 0) - goto cleanup; + return -1; if (!processorSettingData) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get maximum definition of Msvm_ProcessorSettingData for host %s"), conn->uri->server); - goto cleanup; + return -1; } - result = processorSettingData->data->VirtualQuantity; - - cleanup: - hypervFreeObject((hypervObject *)processorSettingData); - - return result; + return processorSettingData->data->VirtualQuantity; } -- 2.30.0
On 1/21/21 1:50 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 8b59dd05f7..6375f6b011 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1514,10 +1514,9 @@ 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; + g_autoptr(Msvm_ProcessorSettingData) processorSettingData = NULL;
/* Get max processors definition */ virBufferAddLit(&query, @@ -1525,21 +1524,16 @@ hypervConnectGetMaxVcpus(virConnectPtr conn, const char *type G_GNUC_UNUSED) "WHERE InstanceID LIKE 'Microsoft:Definition%Maximum'");
if (hypervGetWmiClass(Msvm_ProcessorSettingData, &processorSettingData) < 0) - goto cleanup; + return -1;
if (!processorSettingData) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get maximum definition of Msvm_ProcessorSettingData for host %s"), conn->uri->server); - goto cleanup; + return -1; }
- result = processorSettingData->data->VirtualQuantity; - - cleanup: - hypervFreeObject((hypervObject *)processorSettingData); - - return result; + return processorSettingData->data->VirtualQuantity;
So. Something I'm unsure of - what you're doing here is returning a value that is stored in an object that is being auto-freed by the cleanup when the scope of the function is exited. I *guess* that the cleanup happens after copying the return value into the register that's used to return it to the caller? But I'm not sure, and it could just as easily be done in the opposite order. It's too late in the day (early in the next day, actually) for me to compile something and look at it with gdb to verify. Anybody else know the answer (or feel like experimenting)?
}
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 6375f6b011..e2773d0d2f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1540,20 +1540,19 @@ hypervConnectGetMaxVcpus(virConnectPtr conn, const char *type G_GNUC_UNUSED) static int hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { - int result = -1; hypervPrivate *priv = conn->privateData; - Win32_ComputerSystem *computerSystem = NULL; - Win32_Processor *processorList = NULL; + g_autoptr(Win32_ComputerSystem) computerSystem = NULL; + g_autoptr(Win32_Processor) processorList = NULL; Win32_Processor *processor = NULL; char *tmp; memset(info, 0, sizeof(*info)); if (hypervGetPhysicalSystemList(priv, &computerSystem) < 0) - goto cleanup; + return -1; if (hypervGetProcessorsByName(priv, computerSystem->data->Name, &processorList) < 0) { - goto cleanup; + return -1; } /* Strip the string to fit more relevant information in 32 chars */ @@ -1583,7 +1582,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU model %s too long for destination"), processorList->data->Name); - goto cleanup; + return -1; } info->memory = computerSystem->data->TotalPhysicalMemory / 1024; /* byte to kilobyte */ @@ -1600,13 +1599,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) info->threads = processorList->data->NumberOfLogicalProcessors / info->cores; info->cpus = info->sockets * info->cores; - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)processorList); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index e2773d0d2f..efafe9ece2 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1638,26 +1638,20 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) static int hypervConnectNumOfDomains(virConnectPtr conn) { - bool success = false; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystemList = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; if (hypervGetActiveVirtualSystemList(priv, &computerSystemList) < 0) - goto cleanup; + return -1; for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { ++count; } - success = true; - - cleanup: - hypervFreeObject((hypervObject *)computerSystemList); - - return success ? count : -1; + return count; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index efafe9ece2..45463d120d 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1606,9 +1606,8 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) static int hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) { - bool success = false; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystemList = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; @@ -1616,7 +1615,7 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) return 0; if (hypervGetActiveVirtualSystemList(priv, &computerSystemList) < 0) - goto cleanup; + return -1; for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { @@ -1626,12 +1625,7 @@ hypervConnectListDomains(virConnectPtr conn, int *ids, int maxids) break; } - success = true; - - cleanup: - hypervFreeObject((hypervObject *)computerSystemList); - - return success ? count : -1; + return count; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 45463d120d..e5a62c728b 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1654,16 +1654,13 @@ hypervDomainLookupByID(virConnectPtr conn, int id) { virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; if (hypervGetVirtualSystemByID(priv, id, &computerSystem) < 0) - goto cleanup; + return NULL; hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - return domain; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index e5a62c728b..fe1ab1c52c 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1671,18 +1671,15 @@ hypervDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; virUUIDFormat(uuid, uuid_string); if (hypervMsvmComputerSystemFromUUID(priv, uuid_string, &computerSystem) < 0) - goto cleanup; + return NULL; hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - return domain; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fe1ab1c52c..dd5c42c45b 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1689,23 +1689,20 @@ hypervDomainLookupByName(virConnectPtr conn, const char *name) { virDomainPtr domain = NULL; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; if (hypervGetVirtualSystemByName(priv, name, &computerSystem) < 0) - goto cleanup; + return NULL; if (computerSystem->next) { virReportError(VIR_ERR_MULTIPLE_DOMAINS, _("Multiple domains exist with the name '%s': repeat the request using a UUID"), name); - goto cleanup; + return NULL; } hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain); - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - return domain; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index dd5c42c45b..1eb3dbd48e 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1717,8 +1717,7 @@ hypervDomainSuspend(virDomainPtr domain) static int hypervDomainResume(virDomainPtr domain) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) return -1; @@ -1726,16 +1725,11 @@ hypervDomainResume(virDomainPtr domain) if (computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_QUIESCE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not paused")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, - MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, + MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 1eb3dbd48e..fcb764c0f7 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1736,10 +1736,9 @@ hypervDomainResume(virDomainPtr domain) static int hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_ShutdownComponent *shutdown = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_ShutdownComponent) shutdown = NULL; bool in_transition = false; char uuid[VIR_UUID_STRING_BUFLEN]; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; @@ -1751,13 +1750,13 @@ hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) virUUIDFormat(domain->uuid, uuid); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (!hypervIsMsvmComputerSystemActive(computerSystem, &in_transition) || in_transition) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active or in state transition")); - goto cleanup; + return -1; } virBufferEscapeSQL(&query, MSVM_SHUTDOWNCOMPONENT_WQL_SELECT "WHERE SystemName = '%s'", uuid); @@ -1767,7 +1766,7 @@ hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) virReportError(VIR_ERR_OPERATION_FAILED, _("Could not get Msvm_ShutdownComponent for domain with UUID '%s'"), uuid); - goto cleanup; + return -1; } selector = g_strdup_printf("CreationClassName=\"Msvm_ShutdownComponent\"&DeviceID=\"%s\"&" @@ -1777,7 +1776,7 @@ hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) params = hypervCreateInvokeParamsList("InitiateShutdown", selector, Msvm_ShutdownComponent_WmiInfo); if (!params) - goto cleanup; + return -1; hypervAddSimpleParam(params, "Force", "False"); @@ -1787,15 +1786,9 @@ hypervDomainShutdownFlags(virDomainPtr domain, unsigned int flags) hypervAddSimpleParam(params, "Reason", "Planned shutdown via libvirt"); if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)shutdown); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index fcb764c0f7..7da4c216b1 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1818,29 +1818,23 @@ hypervDomainReset(virDomainPtr domain, unsigned int flags) static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; bool in_transition = false; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (!hypervIsMsvmComputerSystemActive(computerSystem, &in_transition) || in_transition) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active or is in state transition")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, - MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, + MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 7da4c216b1..2ec0415f62 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1860,25 +1860,18 @@ hypervDomainGetMaxMemory(virDomainPtr domain) { char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_MemorySettingData *mem_sd = NULL; - int maxMemoryBytes = 0; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_MemorySettingData) mem_sd = NULL; virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return 0; if (hypervGetMemorySD(priv, vssd->data->InstanceID, &mem_sd) < 0) - goto cleanup; - - maxMemoryBytes = mem_sd->data->Limit * 1024; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)mem_sd); + return 0; - return maxMemoryBytes; + return mem_sd->data->Limit * 1024; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 7da4c216b1..2ec0415f62 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1860,25 +1860,18 @@ hypervDomainGetMaxMemory(virDomainPtr domain) { char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_MemorySettingData *mem_sd = NULL; - int maxMemoryBytes = 0; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_MemorySettingData) mem_sd = NULL;
virUUIDFormat(domain->uuid, uuid_string);
if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return 0;
if (hypervGetMemorySD(priv, vssd->data->InstanceID, &mem_sd) < 0) - goto cleanup; - - maxMemoryBytes = mem_sd->data->Limit * 1024; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)mem_sd); + return 0;
- return maxMemoryBytes; + return mem_sd->data->Limit * 1024;
Another case where the return value is being retrieved from an auto-freed object. Might be right, but I want to verify it with someone who knows (or examination of the generated assembly code in gdb)
}
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2ec0415f62..f3fe88926e 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1880,43 +1880,36 @@ hypervDomainSetMemoryProperty(virDomainPtr domain, unsigned long memory, const char* propertyName) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_MemorySettingData *memsd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_MemorySettingData) memsd = NULL; g_autoptr(GHashTable) memResource = NULL; g_autofree char *memory_str = g_strdup_printf("%lu", VIR_ROUND_UP(VIR_DIV_UP(memory, 1024), 2)); virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetMemorySD(priv, vssd->data->InstanceID, &memsd) < 0) - goto cleanup; + return -1; memResource = hypervCreateEmbeddedParam(Msvm_MemorySettingData_WmiInfo); if (!memResource) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(memResource, propertyName, memory_str) < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(memResource, "InstanceID", memsd->data->InstanceID) < 0) - goto cleanup; + return -1; if (hypervMsvmVSMSModifyResourceSettings(priv, &memResource, Msvm_MemorySettingData_WmiInfo) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)memsd); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index f3fe88926e..ef3ae54f7d 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -197,23 +197,17 @@ hypervGetOperatingSystem(hypervPrivate *priv, Win32_OperatingSystem **operatingS static int hypervRequestStateChange(virDomainPtr domain, int state) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, state); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, state); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index ef3ae54f7d..3c4ef5f33f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -237,25 +237,20 @@ hypervParseVersionString(const char *str, unsigned int *major, static int hypervLookupHostSystemBiosUuid(hypervPrivate *priv, unsigned char *uuid) { - Win32_ComputerSystemProduct *computerSystem = NULL; + g_autoptr(Win32_ComputerSystemProduct) computerSystem = NULL; g_auto(virBuffer) query = { g_string_new(WIN32_COMPUTERSYSTEMPRODUCT_WQL_SELECT), 0 }; - int result = -1; if (hypervGetWmiClass(Win32_ComputerSystemProduct, &computerSystem) < 0) - goto cleanup; + return -1; if (virUUIDParse(computerSystem->data->UUID, uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse UUID from string '%s'"), computerSystem->data->UUID); - goto cleanup; + return -1; } - result = 0; - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 3c4ef5f33f..07e8d376e0 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -533,14 +533,13 @@ hypervDomainAttachPhysicalDisk(virDomainPtr domain, Msvm_ResourceAllocationSettingData *controller, const char *hostname) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; g_autofree char *hostResource = NULL; g_autofree char *controller__PATH = NULL; g_auto(GStrv) matches = NULL; ssize_t found = 0; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ResourceAllocationSettingData *diskdefault = NULL; + g_autoptr(Msvm_ResourceAllocationSettingData) diskdefault = NULL; g_autofree char *controllerInstanceIdEscaped = NULL; g_autoptr(GHashTable) diskResource = NULL; g_autofree char *addressString = g_strdup_printf("%u", disk->info.addr.drive.unit); @@ -584,7 +583,7 @@ hypervDomainAttachPhysicalDisk(virDomainPtr domain, if (found < 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get Msvm_DiskDrive default InstanceID")); - goto cleanup; + return -1; } hostResource = g_strdup_printf("\\\\%s\\Root\\Virtualization\\V2:" @@ -604,35 +603,30 @@ hypervDomainAttachPhysicalDisk(virDomainPtr domain, "Msvm_ResourceAllocationSettingData.InstanceID=\"%s\"", hostname, controllerInstanceIdEscaped); if (!controller__PATH) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(diskResource, "Parent", controller__PATH) < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(diskResource, "AddressOnParent", addressString) < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(diskResource, "ResourceType", resourceType) < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(diskResource, "ResourceSubType", "Microsoft:Hyper-V:Physical Disk Drive") < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(diskResource, "HostResource", hostResource) < 0) - goto cleanup; + return -1; if (hypervMsvmVSMSAddResourceSettings(domain, &diskResource, Msvm_ResourceAllocationSettingData_WmiInfo, NULL) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)diskdefault); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 07e8d376e0..06eee379a1 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -839,14 +839,13 @@ hypervDomainAttachStorageVolume(virDomainPtr domain, static int hypervDomainAttachStorage(virDomainPtr domain, virDomainDefPtr def, const char *hostname) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; size_t i = 0; char uuid_string[VIR_UUID_STRING_BUFLEN]; int num_scsi_controllers = 0; int ctrlr_idx = -1; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_ResourceAllocationSettingData *rasd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_ResourceAllocationSettingData) rasd = NULL; Msvm_ResourceAllocationSettingData *entry = NULL; Msvm_ResourceAllocationSettingData *ideChannels[HYPERV_MAX_IDE_CHANNELS]; Msvm_ResourceAllocationSettingData *scsiControllers[HYPERV_MAX_SCSI_CONTROLLERS]; @@ -865,10 +864,10 @@ hypervDomainAttachStorage(virDomainPtr domain, virDomainDefPtr def, const char * /* filter through all the rasd entries and isolate our controllers */ if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetResourceAllocationSD(priv, vssd->data->InstanceID, &rasd) < 0) - goto cleanup; + return -1; entry = rasd; while (entry) { @@ -889,33 +888,27 @@ hypervDomainAttachStorage(virDomainPtr domain, virDomainDefPtr def, const char * ctrlr_idx = def->disks[i]->info.addr.drive.bus; if (hypervDomainAttachStorageVolume(domain, def->disks[i], ideChannels[ctrlr_idx], hostname) < 0) { - goto cleanup; + return -1; } break; case VIR_DOMAIN_DISK_BUS_SCSI: ctrlr_idx = def->disks[i]->info.addr.drive.controller; if (hypervDomainAttachStorageVolume(domain, def->disks[i], scsiControllers[ctrlr_idx], hostname) < 0) { - goto cleanup; + return -1; } break; case VIR_DOMAIN_DISK_BUS_FDC: if (hypervDomainAttachFloppy(domain, def->disks[i], floppySettings, hostname) < 0) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unsupported controller type")); - goto cleanup; + return -1; } } - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)rasd); - hypervFreeObject((hypervObject *)vssd); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 06eee379a1..40e4c97d35 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1101,7 +1101,7 @@ hypervDomainDefParsePhysicalDisk(hypervPrivate *priv, { int result = -1; Msvm_ResourceAllocationSettingData *controller = NULL; - Msvm_DiskDrive *diskdrive = NULL; + g_autoptr(Msvm_DiskDrive) diskdrive = NULL; virDomainDiskDefPtr disk = NULL; char **hostResource = entry->data->HostResource.data; g_autofree char *hostEscaped = NULL; @@ -1188,7 +1188,6 @@ hypervDomainDefParsePhysicalDisk(hypervPrivate *priv, cleanup: if (result != 0 && disk) virDomainDiskDefFree(disk); - hypervFreeObject((hypervObject *)diskdrive); return result; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 40e4c97d35..9cf074b6a7 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1913,13 +1913,12 @@ hypervDomainSetMemory(virDomainPtr domain, unsigned long memory) static int hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; - Msvm_ProcessorSettingData *processorSettingData = NULL; - Msvm_MemorySettingData *memorySettingData = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) virtualSystemSettingData = NULL; + g_autoptr(Msvm_ProcessorSettingData) processorSettingData = NULL; + g_autoptr(Msvm_MemorySettingData) memorySettingData = NULL; memset(info, 0, sizeof(*info)); @@ -1927,21 +1926,21 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) /* Get Msvm_ComputerSystem */ if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &virtualSystemSettingData) < 0) - goto cleanup; + return -1; if (hypervGetProcessorSD(priv, virtualSystemSettingData->data->InstanceID, &processorSettingData) < 0) - goto cleanup; + return -1; if (hypervGetMemorySD(priv, virtualSystemSettingData->data->InstanceID, &memorySettingData) < 0) - goto cleanup; + return -1; /* Fill struct */ info->state = hypervMsvmComputerSystemEnabledStateToDomainState(computerSystem); @@ -1950,15 +1949,7 @@ hypervDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) info->nrVirtCpu = processorSettingData->data->VirtualQuantity; info->cpuTime = 0; - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)virtualSystemSettingData); - hypervFreeObject((hypervObject *)processorSettingData); - hypervFreeObject((hypervObject *)memorySettingData); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 9cf074b6a7..44be9fbd2f 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1957,25 +1957,19 @@ static int hypervDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; *state = hypervMsvmComputerSystemEnabledStateToDomainState(computerSystem); if (reason != NULL) *reason = 0; - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 44be9fbd2f..f056761338 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1978,11 +1978,10 @@ hypervDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, unsigned int flags) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_ProcessorSettingData *proc_sd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_ProcessorSettingData) proc_sd = NULL; g_autoptr(GHashTable) vcpuResource = NULL; g_autofree char *nvcpus_str = g_strdup_printf("%u", nvcpus); @@ -1991,32 +1990,26 @@ hypervDomainSetVcpusFlags(virDomainPtr domain, virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetProcessorSD(priv, vssd->data->InstanceID, &proc_sd) < 0) - goto cleanup; + return -1; vcpuResource = hypervCreateEmbeddedParam(Msvm_ProcessorSettingData_WmiInfo); if (!vcpuResource) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(vcpuResource, "VirtualQuantity", nvcpus_str) < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(vcpuResource, "InstanceID", proc_sd->data->InstanceID) < 0) - goto cleanup; + return -1; if (hypervMsvmVSMSModifyResourceSettings(priv, &vcpuResource, Msvm_ProcessorSettingData_WmiInfo) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)proc_sd); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index f056761338..0642e42b35 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2023,12 +2023,11 @@ hypervDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) static int hypervDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_ProcessorSettingData *proc_sd = NULL; - Msvm_VirtualSystemSettingData *vssd = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_ProcessorSettingData) proc_sd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -2038,36 +2037,27 @@ hypervDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) /* Start by getting the Msvm_ComputerSystem */ if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; /* Check @flags to see if we are to query a running domain, and fail * if that domain is not running */ if (flags & VIR_DOMAIN_VCPU_LIVE && computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active")); - goto cleanup; + return -1; } /* Check @flags to see if we are to return the maximum vCPU limit */ - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - result = hypervConnectGetMaxVcpus(domain->conn, NULL); - goto cleanup; - } + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + return hypervConnectGetMaxVcpus(domain->conn, NULL); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetProcessorSD(priv, vssd->data->InstanceID, &proc_sd) < 0) - goto cleanup; - - result = proc_sd->data->VirtualQuantity; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)proc_sd); + return -1; - return result; + return proc_sd->data->VirtualQuantity; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index f056761338..0642e42b35 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2023,12 +2023,11 @@ hypervDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) static int hypervDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_ProcessorSettingData *proc_sd = NULL; - Msvm_VirtualSystemSettingData *vssd = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_ProcessorSettingData) proc_sd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -2038,36 +2037,27 @@ hypervDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
/* Start by getting the Msvm_ComputerSystem */ if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1;
/* Check @flags to see if we are to query a running domain, and fail * if that domain is not running */ if (flags & VIR_DOMAIN_VCPU_LIVE && computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_ENABLED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active")); - goto cleanup; + return -1; }
/* Check @flags to see if we are to return the maximum vCPU limit */ - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - result = hypervConnectGetMaxVcpus(domain->conn, NULL); - goto cleanup; - } + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + return hypervConnectGetMaxVcpus(domain->conn, NULL);
if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1;
if (hypervGetProcessorSD(priv, vssd->data->InstanceID, &proc_sd) < 0) - goto cleanup; - - result = proc_sd->data->VirtualQuantity; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)proc_sd); + return -1;
- return result; + return proc_sd->data->VirtualQuantity;
Again, returning a value that is coming out of an auto-freed object. (Each time I see it, I'm more convinced that it probably does the right thing...)
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 0642e42b35..f1e6efd343 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2253,7 +2253,7 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn { bool success = false; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystemList = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; size_t i; @@ -2284,8 +2284,6 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn count = -1; } - hypervFreeObject((hypervObject *)computerSystemList); - return count; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index f1e6efd343..a73b463260 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2291,26 +2291,20 @@ hypervConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxn static int hypervConnectNumOfDefinedDomains(virConnectPtr conn) { - bool success = false; hypervPrivate *priv = conn->privateData; - Msvm_ComputerSystem *computerSystemList = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; int count = 0; if (hypervGetInactiveVirtualSystemList(priv, &computerSystemList) < 0) - goto cleanup; + return -1; for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { ++count; } - success = true; - - cleanup: - hypervFreeObject((hypervObject *)computerSystemList); - - return success ? count : -1; + return count; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index a73b463260..535bd34733 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2311,27 +2311,21 @@ hypervConnectNumOfDefinedDomains(virConnectPtr conn) static int hypervDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (hypervIsMsvmComputerSystemActive(computerSystem, NULL)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is already active or is in state transition")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, - MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, + MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 535bd34733..c63599c09e 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2572,23 +2572,18 @@ hypervDomainAttachDevice(virDomainPtr domain, const char *xml) static int hypervDomainGetAutostart(virDomainPtr domain, int *autostart) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; *autostart = vssd->data->AutomaticStartupAction == 4; - result = 0; - cleanup: - hypervFreeObject((hypervObject *)vssd); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index c63599c09e..15770eeba8 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2590,47 +2590,41 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart) static int hypervDomainSetAutostart(virDomainPtr domain, int autostart) { - int result = -1; char uuid_string[VIR_UUID_STRING_BUFLEN]; hypervPrivate *priv = domain->conn->privateData; - Msvm_VirtualSystemSettingData *vssd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; g_autoptr(hypervInvokeParamsList) params = NULL; g_autoptr(GHashTable) autostartParam = NULL; virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; params = hypervCreateInvokeParamsList("ModifySystemSettings", MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, Msvm_VirtualSystemManagementService_WmiInfo); if (!params) - goto cleanup; + return -1; autostartParam = hypervCreateEmbeddedParam(Msvm_VirtualSystemSettingData_WmiInfo); if (hypervSetEmbeddedProperty(autostartParam, "AutomaticStartupAction", autostart ? "4" : "2") < 0) - goto cleanup; + return -1; if (hypervSetEmbeddedProperty(autostartParam, "InstanceID", vssd->data->InstanceID) < 0) - goto cleanup; + return -1; if (hypervAddEmbeddedParam(params, "SystemSettings", &autostartParam, Msvm_VirtualSystemSettingData_WmiInfo) < 0) - goto cleanup; + return -1; if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 15770eeba8..6fa32d175d 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2644,57 +2644,49 @@ hypervDomainGetSchedulerParametersFlags(virDomainPtr domain, int *nparams, unsigned int flags) { hypervPrivate *priv = domain->conn->privateData; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_VirtualSystemSettingData *vssd = NULL; - Msvm_ProcessorSettingData *proc_sd = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(Msvm_ProcessorSettingData) proc_sd = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN]; int saved_nparams = 0; - int result = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; /* get info from host */ virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetProcessorSD(priv, vssd->data->InstanceID, &proc_sd) < 0) - goto cleanup; + return -1; /* parse it all out */ if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_LIMIT, VIR_TYPED_PARAM_LLONG, proc_sd->data->Limit) < 0) - goto cleanup; + return -1; saved_nparams++; if (*nparams > saved_nparams) { if (virTypedParameterAssign(¶ms[1], VIR_DOMAIN_SCHEDULER_RESERVATION, VIR_TYPED_PARAM_LLONG, proc_sd->data->Reservation) < 0) - goto cleanup; + return -1; saved_nparams++; } if (*nparams > saved_nparams) { if (virTypedParameterAssign(¶ms[2], VIR_DOMAIN_SCHEDULER_WEIGHT, VIR_TYPED_PARAM_UINT, proc_sd->data->Weight) < 0) - goto cleanup; + return -1; saved_nparams++; } *nparams = saved_nparams; - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)proc_sd); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 6fa32d175d..e501a88889 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2770,18 +2770,12 @@ hypervConnectIsAlive(virConnectPtr conn) static int hypervDomainIsActive(virDomainPtr domain) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; - - result = hypervIsMsvmComputerSystemActive(computerSystem, NULL) ? 1 : 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); + return -1; - return result; + return hypervIsMsvmComputerSystemActive(computerSystem, NULL) ? 1 : 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index e501a88889..476c24533d 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2807,28 +2807,22 @@ hypervDomainIsUpdated(virDomainPtr domain G_GNUC_UNUSED) static int hypervDomainManagedSave(virDomainPtr domain, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; bool in_transition = false; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (!hypervIsMsvmComputerSystemActive(computerSystem, &in_transition) || in_transition) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not active or is in state transition")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_OFFLINE); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_OFFLINE); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 476c24533d..d4852d55af 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2829,20 +2829,14 @@ hypervDomainManagedSave(virDomainPtr domain, unsigned int flags) static int hypervDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; - - result = computerSystem->data->EnabledState == MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED ? 1 : 0; - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); + return -1; - return result; + return computerSystem->data->EnabledState == MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED ? 1 : 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index d4852d55af..f134b7f137 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2843,27 +2843,21 @@ hypervDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) static int hypervDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) { - int result = -1; - Msvm_ComputerSystem *computerSystem = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; virCheckFlags(0, -1); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; if (computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_SUSPENDED) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain has no managed save image")); - goto cleanup; + return -1; } - result = hypervInvokeMsvmComputerSystemRequestStateChange(domain, - MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); - - cleanup: - hypervFreeObject((hypervObject *)computerSystem); - - return result; + return hypervInvokeMsvmComputerSystemRequestStateChange(domain, + MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_DISABLED); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index f134b7f137..2dfccb4802 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2869,7 +2869,7 @@ hypervConnectListAllDomains(virConnectPtr conn, { hypervPrivate *priv = conn->privateData; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ComputerSystem *computerSystemList = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; size_t ndoms; virDomainPtr domain; @@ -2981,8 +2981,6 @@ hypervConnectListAllDomains(virConnectPtr conn, VIR_FREE(doms); } - hypervFreeObject((hypervObject *)computerSystemList); - return ret; } #undef MATCH -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2dfccb4802..4bf91cfa1a 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2991,15 +2991,14 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, unsigned int holdtime, unsigned int *keycodes, int nkeycodes, unsigned int flags) { - int result = -1; size_t i = 0; int keycode = 0; - int *translatedKeycodes = NULL; + g_autofree int *translatedKeycodes = NULL; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - char *selector = NULL; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_Keyboard *keyboard = NULL; + g_autofree char *selector = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_Keyboard) keyboard = NULL; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; g_autoptr(hypervInvokeParamsList) params = NULL; char keycodeStr[VIR_INT64_STR_BUFLEN]; @@ -3009,7 +3008,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, virUUIDFormat(domain->uuid, uuid_string); if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return -1; virBufferEscapeSQL(&query, "ASSOCIATORS OF {Msvm_ComputerSystem.CreationClassName='Msvm_ComputerSystem',Name='%s'} " @@ -3017,7 +3016,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, uuid_string); if (hypervGetWmiClass(Msvm_Keyboard, &keyboard) < 0) - goto cleanup; + return -1; translatedKeycodes = g_new0(int, nkeycodes); @@ -3031,7 +3030,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, if (keycode < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not translate keycode")); - goto cleanup; + return -1; } translatedKeycodes[i] = keycode; } @@ -3049,13 +3048,13 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, Msvm_Keyboard_WmiInfo); if (!params) - goto cleanup; + return -1; if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0) - goto cleanup; + return -1; if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; + return -1; } /* simulate holdtime by sleeping */ @@ -3069,23 +3068,16 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, Msvm_Keyboard_WmiInfo); if (!params) - goto cleanup; + return -1; if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0) - goto cleanup; + return -1; if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; + return -1; } - result = 0; - - cleanup: - VIR_FREE(translatedKeycodes); - VIR_FREE(selector); - hypervFreeObject((hypervObject *)keyboard); - hypervFreeObject((hypervObject *)computerSystem); - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 68 ++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 0a9d4bf4fd..459d207ee7 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -744,35 +744,36 @@ hypervInvokeMethod(hypervPrivate *priv, WsXmlDocH *res) { g_autoptr(hypervInvokeParamsList) params = *paramsPtr; - int result = -1; size_t i = 0; int returnCode; - WsXmlDocH paramsDocRoot = NULL; - client_opt_t *options = NULL; - WsXmlDocH response = NULL; + g_auto(WsXmlDocH) paramsDocRoot = NULL; + g_autoptr(client_opt_t) options = NULL; + g_auto(WsXmlDocH) response = NULL; WsXmlNodeH methodNode = NULL; - char *returnValue_xpath = NULL; - char *jobcode_instance_xpath = NULL; - char *returnValue = NULL; - char *instanceID = NULL; + g_autofree char *returnValue_xpath = NULL; + g_autofree char *jobcode_instance_xpath = NULL; + g_autofree char *returnValue = NULL; + g_autofree char *instanceID = NULL; bool completed = false; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ConcreteJob *job = NULL; + g_autoptr(Msvm_ConcreteJob) job = NULL; int jobState = -1; hypervParamPtr p = NULL; int timeout = HYPERV_JOB_TIMEOUT_MS; + *paramsPtr = NULL; + if (hypervCreateInvokeXmlDoc(params, ¶msDocRoot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create XML document")); - goto cleanup; + return -1; } methodNode = xml_parser_get_root(paramsDocRoot); if (!methodNode) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get root of XML document")); - goto cleanup; + return -1; } /* Serialize parameters */ @@ -783,22 +784,22 @@ hypervInvokeMethod(hypervPrivate *priv, case HYPERV_SIMPLE_PARAM: if (hypervSerializeSimpleParam(p, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; case HYPERV_EPR_PARAM: if (hypervSerializeEprParam(p, priv, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; case HYPERV_EMBEDDED_PARAM: if (hypervSerializeEmbeddedParam(p, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unknown parameter type")); - goto cleanup; + return -1; } } @@ -807,7 +808,7 @@ hypervInvokeMethod(hypervPrivate *priv, options = wsmc_options_init(); if (!options) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); - goto cleanup; + return -1; } wsmc_add_selectors_from_str(options, params->selector); @@ -824,11 +825,11 @@ hypervInvokeMethod(hypervPrivate *priv, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get return value for %s invocation"), params->method); - goto cleanup; + return -1; } if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) - goto cleanup; + return -1; if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) { jobcode_instance_xpath = g_strdup_printf("/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:ReferenceParameters/" @@ -840,7 +841,7 @@ hypervInvokeMethod(hypervPrivate *priv, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get instance ID for %s invocation"), params->method); - goto cleanup; + return -1; } /* @@ -862,7 +863,7 @@ hypervInvokeMethod(hypervPrivate *priv, "WHERE InstanceID = '%s'", instanceID); if (hypervGetWmiClass(Msvm_ConcreteJob, &job) < 0 || !job) - goto cleanup; + return -1; jobState = job->data->JobState; switch (jobState) { @@ -882,44 +883,29 @@ hypervInvokeMethod(hypervPrivate *priv, case MSVM_CONCRETEJOB_JOBSTATE_KILLED: case MSVM_CONCRETEJOB_JOBSTATE_EXCEPTION: case MSVM_CONCRETEJOB_JOBSTATE_SERVICE: - goto cleanup; + return -1; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unknown invocation state")); - goto cleanup; + return -1; } } if (!completed && timeout < 0) { virReportError(VIR_ERR_OPERATION_TIMEOUT, _("Timeout waiting for %s invocation"), params->method); - goto cleanup; + return -1; } } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invocation of %s returned an error: %s (%d)"), params->method, hypervReturnCodeToString(returnCode), returnCode); - goto cleanup; + return -1; } if (res) - *res = response; + *res = g_steal_pointer(&response); - result = 0; - - cleanup: - if (options) - wsmc_options_destroy(options); - if (response && (!res)) - ws_xml_destroy_doc(response); - if (paramsDocRoot) - ws_xml_destroy_doc(paramsDocRoot); - VIR_FREE(returnValue_xpath); - VIR_FREE(jobcode_instance_xpath); - VIR_FREE(returnValue); - VIR_FREE(instanceID); - hypervFreeObject((hypervObject *)job); - *paramsPtr = NULL; - return result; + return 0; } /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 68 ++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 41 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 0a9d4bf4fd..459d207ee7 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -744,35 +744,36 @@ hypervInvokeMethod(hypervPrivate *priv, WsXmlDocH *res) { g_autoptr(hypervInvokeParamsList) params = *paramsPtr; - int result = -1; size_t i = 0; int returnCode; - WsXmlDocH paramsDocRoot = NULL; - client_opt_t *options = NULL; - WsXmlDocH response = NULL; + g_auto(WsXmlDocH) paramsDocRoot = NULL; + g_autoptr(client_opt_t) options = NULL; + g_auto(WsXmlDocH) response = NULL; WsXmlNodeH methodNode = NULL; - char *returnValue_xpath = NULL; - char *jobcode_instance_xpath = NULL; - char *returnValue = NULL; - char *instanceID = NULL; + g_autofree char *returnValue_xpath = NULL; + g_autofree char *jobcode_instance_xpath = NULL; + g_autofree char *returnValue = NULL; + g_autofree char *instanceID = NULL; bool completed = false; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ConcreteJob *job = NULL; + g_autoptr(Msvm_ConcreteJob) job = NULL; int jobState = -1; hypervParamPtr p = NULL; int timeout = HYPERV_JOB_TIMEOUT_MS;
+ *paramsPtr = NULL; + if (hypervCreateInvokeXmlDoc(params, ¶msDocRoot) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create XML document")); - goto cleanup; + return -1; }
methodNode = xml_parser_get_root(paramsDocRoot); if (!methodNode) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get root of XML document")); - goto cleanup; + return -1; }
/* Serialize parameters */ @@ -783,22 +784,22 @@ hypervInvokeMethod(hypervPrivate *priv, case HYPERV_SIMPLE_PARAM: if (hypervSerializeSimpleParam(p, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; case HYPERV_EPR_PARAM: if (hypervSerializeEprParam(p, priv, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; case HYPERV_EMBEDDED_PARAM: if (hypervSerializeEmbeddedParam(p, params->resourceUri, &methodNode) < 0) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unknown parameter type")); - goto cleanup; + return -1; } }
@@ -807,7 +808,7 @@ hypervInvokeMethod(hypervPrivate *priv, options = wsmc_options_init(); if (!options) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); - goto cleanup; + return -1; } wsmc_add_selectors_from_str(options, params->selector);
@@ -824,11 +825,11 @@ hypervInvokeMethod(hypervPrivate *priv, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get return value for %s invocation"), params->method); - goto cleanup; + return -1; }
if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) - goto cleanup; + return -1;
if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) { jobcode_instance_xpath = g_strdup_printf("/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:ReferenceParameters/" @@ -840,7 +841,7 @@ hypervInvokeMethod(hypervPrivate *priv, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get instance ID for %s invocation"), params->method); - goto cleanup; + return -1; }
/* @@ -862,7 +863,7 @@ hypervInvokeMethod(hypervPrivate *priv, "WHERE InstanceID = '%s'", instanceID);
if (hypervGetWmiClass(Msvm_ConcreteJob, &job) < 0 || !job) - goto cleanup; + return -1;
jobState = job->data->JobState; switch (jobState) { @@ -882,44 +883,29 @@ hypervInvokeMethod(hypervPrivate *priv, case MSVM_CONCRETEJOB_JOBSTATE_KILLED: case MSVM_CONCRETEJOB_JOBSTATE_EXCEPTION: case MSVM_CONCRETEJOB_JOBSTATE_SERVICE: - goto cleanup; + return -1; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unknown invocation state")); - goto cleanup; + return -1; } } if (!completed && timeout < 0) { virReportError(VIR_ERR_OPERATION_TIMEOUT, _("Timeout waiting for %s invocation"), params->method); - goto cleanup; + return -1; } } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invocation of %s returned an error: %s (%d)"), params->method, hypervReturnCodeToString(returnCode), returnCode); - goto cleanup; + return -1; }
if (res) - *res = response; + *res = g_steal_pointer(&response);
- result = 0; - - cleanup: - if (options) - wsmc_options_destroy(options); - if (response && (!res)) - ws_xml_destroy_doc(response); - if (paramsDocRoot) - ws_xml_destroy_doc(paramsDocRoot);
I notice that the above three cleanup functions are only called if the pointer is != NULL. Do you know for certain that they are safe to call with a NULL pointer? If not, then you'll need to supply a wrapper to use as the cleanup function.
- VIR_FREE(returnValue_xpath); - VIR_FREE(jobcode_instance_xpath); - VIR_FREE(returnValue); - VIR_FREE(instanceID); - hypervFreeObject((hypervObject *)job); - *paramsPtr = NULL; - return result; + return 0; }
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 50 +++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 459d207ee7..bd62174739 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1169,18 +1169,17 @@ int hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, int requestedState) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - WsXmlDocH response = NULL; - client_opt_t *options = NULL; - char *selector = NULL; - char *properties = NULL; - char *returnValue = NULL; + g_auto(WsXmlDocH) response = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autofree char *selector = NULL; + g_autofree char *properties = NULL; + g_autofree char *returnValue = NULL; int returnCode; - char *instanceID = NULL; + g_autofree char *instanceID = NULL; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ConcreteJob *concreteJob = NULL; + g_autoptr(Msvm_ConcreteJob) concreteJob = NULL; bool completed = false; virUUIDFormat(domain->uuid, uuid_string); @@ -1193,7 +1192,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, if (options == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not initialize options")); - goto cleanup; + return -1; } wsmc_add_selectors_from_str(options, selector); @@ -1204,7 +1203,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, options, "RequestStateChange", NULL); if (hypervVerifyResponse(priv->client, response, "invocation") < 0) - goto cleanup; + return -1; /* Check return value */ returnValue = ws_xml_get_xpath_value(response, (char *)"/s:Envelope/s:Body/p:RequestStateChange_OUTPUT/p:ReturnValue"); @@ -1213,13 +1212,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "ReturnValue", "RequestStateChange"); - goto cleanup; + return -1; } if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse return code from '%s'"), returnValue); - goto cleanup; + return -1; } if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) { @@ -1230,7 +1229,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "InstanceID", "RequestStateChange"); - goto cleanup; + return -1; } /* FIXME: Poll every 100ms until the job completes or fails. There @@ -1241,13 +1240,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, "WHERE InstanceID = '%s'", instanceID); if (hypervGetWmiClass(Msvm_ConcreteJob, &concreteJob) < 0) - goto cleanup; + return -1; if (concreteJob == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "Msvm_ConcreteJob", "RequestStateChange"); - goto cleanup; + return -1; } switch (concreteJob->data->JobState) { @@ -1272,13 +1271,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Concrete job for %s invocation is in error state"), "RequestStateChange"); - goto cleanup; + return -1; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Concrete job for %s invocation is in unknown state"), "RequestStateChange"); - goto cleanup; + return -1; } } } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) { @@ -1286,23 +1285,10 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, _("Invocation of %s returned an error: %s (%d)"), "RequestStateChange", hypervReturnCodeToString(returnCode), returnCode); - goto cleanup; + return -1; } - result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options); - - ws_xml_destroy_doc(response); - VIR_FREE(selector); - VIR_FREE(properties); - VIR_FREE(returnValue); - VIR_FREE(instanceID); - hypervFreeObject((hypervObject *)concreteJob); - - return result; + return 0; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 50 +++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 32 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 459d207ee7..bd62174739 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1169,18 +1169,17 @@ int hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, int requestedState) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - WsXmlDocH response = NULL; - client_opt_t *options = NULL; - char *selector = NULL; - char *properties = NULL; - char *returnValue = NULL; + g_auto(WsXmlDocH) response = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autofree char *selector = NULL; + g_autofree char *properties = NULL; + g_autofree char *returnValue = NULL; int returnCode; - char *instanceID = NULL; + g_autofree char *instanceID = NULL; g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER; - Msvm_ConcreteJob *concreteJob = NULL; + g_autoptr(Msvm_ConcreteJob) concreteJob = NULL; bool completed = false;
virUUIDFormat(domain->uuid, uuid_string); @@ -1193,7 +1192,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, if (options == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not initialize options")); - goto cleanup; + return -1; }
wsmc_add_selectors_from_str(options, selector); @@ -1204,7 +1203,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, options, "RequestStateChange", NULL);
if (hypervVerifyResponse(priv->client, response, "invocation") < 0) - goto cleanup; + return -1;
/* Check return value */ returnValue = ws_xml_get_xpath_value(response, (char *)"/s:Envelope/s:Body/p:RequestStateChange_OUTPUT/p:ReturnValue"); @@ -1213,13 +1212,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "ReturnValue", "RequestStateChange"); - goto cleanup; + return -1; }
if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse return code from '%s'"), returnValue); - goto cleanup; + return -1; }
if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) { @@ -1230,7 +1229,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "InstanceID", "RequestStateChange"); - goto cleanup; + return -1; }
/* FIXME: Poll every 100ms until the job completes or fails. There @@ -1241,13 +1240,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, "WHERE InstanceID = '%s'", instanceID);
if (hypervGetWmiClass(Msvm_ConcreteJob, &concreteJob) < 0) - goto cleanup; + return -1;
if (concreteJob == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not lookup %s for %s invocation"), "Msvm_ConcreteJob", "RequestStateChange"); - goto cleanup; + return -1; }
switch (concreteJob->data->JobState) { @@ -1272,13 +1271,13 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, virReportError(VIR_ERR_INTERNAL_ERROR, _("Concrete job for %s invocation is in error state"), "RequestStateChange"); - goto cleanup; + return -1;
default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Concrete job for %s invocation is in unknown state"), "RequestStateChange"); - goto cleanup; + return -1; } } } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) { @@ -1286,23 +1285,10 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, _("Invocation of %s returned an error: %s (%d)"), "RequestStateChange", hypervReturnCodeToString(returnCode), returnCode); - goto cleanup; + return -1; }
- result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options);
Same question here as in the previous patch.
- - ws_xml_destroy_doc(response); - VIR_FREE(selector); - VIR_FREE(properties); - VIR_FREE(returnValue); - VIR_FREE(instanceID); - hypervFreeObject((hypervObject *)concreteJob); - - return result; + return 0; }
Fixes a memory leak when hypervCreateInvokeParamsList() fails. Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 43 ++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index bd62174739..ed0091ba06 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1545,18 +1545,19 @@ hypervMsvmVSMSAddResourceSettings(virDomainPtr domain, hypervWmiClassInfoPtr wmiInfo, WsXmlDocH *response) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - Msvm_VirtualSystemSettingData *vssd = NULL; - GHashTable *resourceSettings = *resourceSettingsPtr; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(GHashTable) resourceSettings = *resourceSettingsPtr; g_autoptr(hypervInvokeParamsList) params = NULL; g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER; + *resourceSettingsPtr = NULL; + virUUIDFormat(domain->uuid, uuid_string); if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; virBufferEscapeSQL(&eprQuery, MSVM_VIRTUALSYSTEMSETTINGDATA_WQL_SELECT "WHERE InstanceID='%s'", @@ -1567,27 +1568,21 @@ hypervMsvmVSMSAddResourceSettings(virDomainPtr domain, Msvm_VirtualSystemManagementService_WmiInfo); if (!params) - goto cleanup; + return -1; if (hypervAddEprParam(params, "AffectedConfiguration", &eprQuery, Msvm_VirtualSystemSettingData_WmiInfo) < 0) - goto cleanup; + return -1; if (hypervAddEmbeddedParam(params, "ResourceSettings", &resourceSettings, wmiInfo) < 0) { hypervFreeEmbeddedParam(resourceSettings); - goto cleanup; + return -1; } if (hypervInvokeMethod(priv, ¶ms, response) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - *resourceSettingsPtr = NULL; + return -1; - return result; + return 0; } @@ -1596,29 +1591,25 @@ hypervMsvmVSMSModifyResourceSettings(hypervPrivate *priv, GHashTable **resourceSettingsPtr, hypervWmiClassInfoPtr wmiInfo) { - int result = -1; - GHashTable *resourceSettings = *resourceSettingsPtr; + g_autoptr(GHashTable) resourceSettings = *resourceSettingsPtr; g_autoptr(hypervInvokeParamsList) params = NULL; + *resourceSettingsPtr = NULL; + params = hypervCreateInvokeParamsList("ModifyResourceSettings", MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, Msvm_VirtualSystemManagementService_WmiInfo); if (!params) - goto cleanup; + return -1; if (hypervAddEmbeddedParam(params, "ResourceSettings", &resourceSettings, wmiInfo) < 0) { hypervFreeEmbeddedParam(resourceSettings); - goto cleanup; + return -1; } if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; - - result = 0; - - cleanup: - *resourceSettingsPtr = NULL; + return -1; - return result; + return 0; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Fixes a memory leak when hypervCreateInvokeParamsList() fails.
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 43 ++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index bd62174739..ed0091ba06 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -1545,18 +1545,19 @@ hypervMsvmVSMSAddResourceSettings(virDomainPtr domain, hypervWmiClassInfoPtr wmiInfo, WsXmlDocH *response) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; char uuid_string[VIR_UUID_STRING_BUFLEN]; - Msvm_VirtualSystemSettingData *vssd = NULL; - GHashTable *resourceSettings = *resourceSettingsPtr; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; + g_autoptr(GHashTable) resourceSettings = *resourceSettingsPtr; g_autoptr(hypervInvokeParamsList) params = NULL; g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
+ *resourceSettingsPtr = NULL;
If you wanted to get rid of this line, you could change the initialization of resourceSettings to this: g_autoptr(GHashTable) resourceSettings = g_steal_pointer(resourceSettingsPtr); It's okay as it is too, though.
+ virUUIDFormat(domain->uuid, uuid_string);
if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1;
virBufferEscapeSQL(&eprQuery, MSVM_VIRTUALSYSTEMSETTINGDATA_WQL_SELECT "WHERE InstanceID='%s'", @@ -1567,27 +1568,21 @@ hypervMsvmVSMSAddResourceSettings(virDomainPtr domain, Msvm_VirtualSystemManagementService_WmiInfo);
if (!params) - goto cleanup; + return -1;
if (hypervAddEprParam(params, "AffectedConfiguration", &eprQuery, Msvm_VirtualSystemSettingData_WmiInfo) < 0) - goto cleanup; + return -1;
if (hypervAddEmbeddedParam(params, "ResourceSettings", &resourceSettings, wmiInfo) < 0) { hypervFreeEmbeddedParam(resourceSettings); - goto cleanup; + return -1; }
if (hypervInvokeMethod(priv, ¶ms, response) < 0) - goto cleanup; - - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - *resourceSettingsPtr = NULL; + return -1;
- return result; + return 0; }
@@ -1596,29 +1591,25 @@ hypervMsvmVSMSModifyResourceSettings(hypervPrivate *priv, GHashTable **resourceSettingsPtr, hypervWmiClassInfoPtr wmiInfo) { - int result = -1; - GHashTable *resourceSettings = *resourceSettingsPtr; + g_autoptr(GHashTable) resourceSettings = *resourceSettingsPtr; g_autoptr(hypervInvokeParamsList) params = NULL;
+ *resourceSettingsPtr = NULL; + params = hypervCreateInvokeParamsList("ModifyResourceSettings", MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, Msvm_VirtualSystemManagementService_WmiInfo);
if (!params) - goto cleanup; + return -1;
if (hypervAddEmbeddedParam(params, "ResourceSettings", &resourceSettings, wmiInfo) < 0) { hypervFreeEmbeddedParam(resourceSettings); - goto cleanup; + return -1; }
if (hypervInvokeMethod(priv, ¶ms, NULL) < 0) - goto cleanup; - - result = 0; - - cleanup: - *resourceSettingsPtr = NULL; + return -1;
- return result; + return 0; }
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 4bf91cfa1a..84ffb24f1a 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2071,7 +2071,7 @@ hypervDomainGetVcpus(virDomainPtr domain, int count = 0; int vcpu_number; hypervPrivate *priv = domain->conn->privateData; - Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor *vproc = NULL; + g_autoptr(Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor) vproc = NULL; /* Hyper-V does not allow setting CPU affinity: all cores will be used */ if (cpumaps && maplen > 0) @@ -2110,8 +2110,6 @@ hypervDomainGetVcpus(virDomainPtr domain, count++; } - hypervFreeObject((hypervObject *)vproc); - return count; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 84ffb24f1a..bd3c5a7c34 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2701,9 +2701,8 @@ hypervDomainGetSchedulerParameters(virDomainPtr domain, static unsigned long long hypervNodeGetFreeMemory(virConnectPtr conn) { - unsigned long long freeMemoryBytes = 0; hypervPrivate *priv = conn->privateData; - Win32_OperatingSystem *operatingSystem = NULL; + g_autoptr(Win32_OperatingSystem) operatingSystem = NULL; if (hypervGetOperatingSystem(priv, &operatingSystem) < 0) return 0; @@ -2715,11 +2714,7 @@ hypervNodeGetFreeMemory(virConnectPtr conn) return 0; } - freeMemoryBytes = operatingSystem->data->FreePhysicalMemory * 1024; - - hypervFreeObject((hypervObject *)operatingSystem); - - return freeMemoryBytes; + return operatingSystem->data->FreePhysicalMemory * 1024; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 49 ++++++++++++++------------------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index bd3c5a7c34..e8296ead21 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2117,52 +2117,51 @@ hypervDomainGetVcpus(virDomainPtr domain, static char * hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) { - char *xml = NULL; hypervPrivate *priv = domain->conn->privateData; - virDomainDefPtr def = NULL; + g_autoptr(virDomainDef) def = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN]; - Msvm_ComputerSystem *computerSystem = NULL; - Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL; - Msvm_ProcessorSettingData *processorSettingData = NULL; - Msvm_MemorySettingData *memorySettingData = NULL; - Msvm_ResourceAllocationSettingData *rasd = NULL; - Msvm_StorageAllocationSettingData *sasd = NULL; + g_autoptr(Msvm_ComputerSystem) computerSystem = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) virtualSystemSettingData = NULL; + g_autoptr(Msvm_ProcessorSettingData) processorSettingData = NULL; + g_autoptr(Msvm_MemorySettingData) memorySettingData = NULL; + g_autoptr(Msvm_ResourceAllocationSettingData) rasd = NULL; + g_autoptr(Msvm_StorageAllocationSettingData) sasd = NULL; virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL); if (!(def = virDomainDefNew())) - goto cleanup; + return NULL; virUUIDFormat(domain->uuid, uuid_string); /* Get Msvm_ComputerSystem */ if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) - goto cleanup; + return NULL; if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &virtualSystemSettingData) < 0) - goto cleanup; + return NULL; if (hypervGetProcessorSD(priv, virtualSystemSettingData->data->InstanceID, &processorSettingData) < 0) - goto cleanup; + return NULL; if (hypervGetMemorySD(priv, virtualSystemSettingData->data->InstanceID, &memorySettingData) < 0) - goto cleanup; + return NULL; if (hypervGetResourceAllocationSD(priv, virtualSystemSettingData->data->InstanceID, &rasd) < 0) { - goto cleanup; + return NULL; } if (hypervGetStorageAllocationSD(priv, virtualSystemSettingData->data->InstanceID, &sasd) < 0) { - goto cleanup; + return NULL; } /* Fill struct */ @@ -2207,10 +2206,10 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) virDomainDefSetMemoryTotal(def, memorySettingData->data->VirtualQuantity * 1024); if (virDomainDefSetVcpusMax(def, processorSettingData->data->VirtualQuantity, NULL) < 0) - goto cleanup; + return NULL; if (virDomainDefSetVcpus(def, processorSettingData->data->VirtualQuantity) < 0) - goto cleanup; + return NULL; def->os.type = VIR_DOMAIN_OSTYPE_HVM; @@ -2227,22 +2226,10 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) def->ncontrollers = 0; if (hypervDomainDefParseStorage(priv, def, rasd, sasd) < 0) - goto cleanup; + return NULL; /* XXX xmlopts must be non-NULL */ - xml = virDomainDefFormat(def, NULL, - virDomainDefFormatConvertXMLFlags(flags)); - - cleanup: - virDomainDefFree(def); - hypervFreeObject((hypervObject *)computerSystem); - hypervFreeObject((hypervObject *)virtualSystemSettingData); - hypervFreeObject((hypervObject *)processorSettingData); - hypervFreeObject((hypervObject *)memorySettingData); - hypervFreeObject((hypervObject *)rasd); - hypervFreeObject((hypervObject *)sasd); - - return xml; + return virDomainDefFormat(def, NULL, virDomainDefFormatConvertXMLFlags(flags)); } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index e8296ead21..2c0e9e0614 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2443,17 +2443,16 @@ hypervDomainDefineXML(virConnectPtr conn, const char *xml) static int hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - int result = -1; hypervPrivate *priv = domain->conn->privateData; g_autoptr(virDomainDef) def = NULL; g_autoptr(virDomainDeviceDef) dev = NULL; - Win32_ComputerSystem *host = NULL; + g_autoptr(Win32_ComputerSystem) host = NULL; char *hostname = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN]; Msvm_ResourceAllocationSettingData *controller = NULL; - Msvm_ResourceAllocationSettingData *rasd = NULL; + g_autoptr(Msvm_ResourceAllocationSettingData) rasd = NULL; Msvm_ResourceAllocationSettingData *entry = NULL; - Msvm_VirtualSystemSettingData *vssd = NULL; + g_autoptr(Msvm_VirtualSystemSettingData) vssd = NULL; int num_scsi = 0; virCheckFlags(0, -1); @@ -2462,16 +2461,16 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int /* get domain definition */ if (!(def = virDomainDefNew())) - return result; + return -1; /* get domain device definition */ dev = virDomainDeviceDefParse(xml, def, priv->xmlopt, NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE); if (!dev) - goto cleanup; + return -1; /* get the host computer system */ if (hypervGetPhysicalSystemList(priv, &host) < 0) - goto cleanup; + return -1; hostname = host->data->Name; @@ -2479,10 +2478,10 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int case VIR_DOMAIN_DEVICE_DISK: /* get our controller */ if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) - goto cleanup; + return -1; if (hypervGetResourceAllocationSD(priv, vssd->data->InstanceID, &rasd) < 0) - goto cleanup; + return -1; entry = rasd; switch (dev->data.disk->bus) { @@ -2496,7 +2495,7 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int entry = entry->next; } if (!entry) - goto cleanup; + return -1; break; case VIR_DOMAIN_DISK_BUS_SCSI: while (entry) { @@ -2508,7 +2507,7 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int entry = entry->next; } if (!entry) - goto cleanup; + return -1; break; case VIR_DOMAIN_DISK_BUS_FDC: while (entry) { @@ -2519,31 +2518,24 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int entry = entry->next; } if (!entry) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid disk bus in definition")); - goto cleanup; + return -1; } if (hypervDomainAttachStorageVolume(domain, dev->data.disk, controller, hostname) < 0) - goto cleanup; + return -1; break; default: /* unsupported device type */ virReportError(VIR_ERR_INTERNAL_ERROR, _("Attaching devices of type %d is not implemented"), dev->type); - goto cleanup; + return -1; } - result = 0; - - cleanup: - hypervFreeObject((hypervObject *)vssd); - hypervFreeObject((hypervObject *)rasd); - hypervFreeObject((hypervObject *)host); - - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 56 ++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index ed0091ba06..c1325b2ccc 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -455,23 +455,21 @@ static int hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, const char *resourceUri, WsXmlNodeH *methodNode) { - int result = -1; WsXmlNodeH xmlNodeParam = NULL, xmlNodeTemp = NULL, xmlNodeAddr = NULL, xmlNodeRef = NULL; - WsXmlDocH xmlDocResponse = NULL; - WsXmlNsH ns = NULL; - client_opt_t *options = NULL; - filter_t *filter = NULL; - char *enumContext = NULL; - char *query_string = NULL; + g_auto(WsXmlDocH) xmlDocResponse = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autoptr(filter_t) filter = NULL; + g_autofree char *enumContext = NULL; + g_autofree char *query_string = NULL; /* init and set up options */ options = wsmc_options_init(); if (!options) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); - goto cleanup; + return -1; } wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR); @@ -480,14 +478,14 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string); if (!filter) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create WQL filter")); - goto cleanup; + return -1; } /* enumerate based on the filter from this query */ xmlDocResponse = wsmc_action_enumerate(priv->client, p->epr.info->rootUri, options, filter); if (hypervVerifyResponse(priv->client, xmlDocResponse, "enumeration") < 0) - goto cleanup; + return -1; /* Get context */ enumContext = wsmc_get_enum_context(xmlDocResponse); @@ -498,41 +496,41 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, filter, enumContext); if (hypervVerifyResponse(priv->client, xmlDocResponse, "pull") < 0) - goto cleanup; + return -1; /* drill down and extract EPR node children */ if (!(xmlNodeTemp = ws_xml_get_soap_body(xmlDocResponse))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get SOAP body")); - goto cleanup; + return -1; } if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, WSENUM_PULL_RESP))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response")); - goto cleanup; + return -1; } if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, WSENUM_ITEMS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response items")); - goto cleanup; + return -1; } if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_EPR))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR items")); - goto cleanup; + return -1; } if (!(xmlNodeAddr = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_ADDRESS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR address")); - goto cleanup; + return -1; } if (!(xmlNodeRef = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_REFERENCE_PARAMETERS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup EPR item reference parameters")); - goto cleanup; + return -1; } /* now build a new xml doc with the EPR node children */ @@ -540,38 +538,26 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, p->epr.name, NULL))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add child node to methodNode")); - goto cleanup; + return -1; } - if (!(ns = ws_xml_ns_add(xmlNodeParam, - "http://schemas.xmlsoap.org/ws/2004/08/addressing", "a"))) { + if (!ws_xml_ns_add(xmlNodeParam, "http://schemas.xmlsoap.org/ws/2004/08/addressing", "a")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set namespace address for xmlNodeParam")); - goto cleanup; + return -1; } - if (!(ns = ws_xml_ns_add(xmlNodeParam, - "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd", "w"))) { + if (!ws_xml_ns_add(xmlNodeParam, "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd", "w")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set wsman namespace address for xmlNodeParam")); - goto cleanup; + return -1; } ws_xml_duplicate_tree(xmlNodeParam, xmlNodeAddr); ws_xml_duplicate_tree(xmlNodeParam, xmlNodeRef); /* we did it! */ - result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options); - if (filter != NULL) - filter_destroy(filter); - ws_xml_destroy_doc(xmlDocResponse); - VIR_FREE(enumContext); - VIR_FREE(query_string); - return result; + return 0; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 56 ++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 35 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index ed0091ba06..c1325b2ccc 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -455,23 +455,21 @@ static int hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, const char *resourceUri, WsXmlNodeH *methodNode) { - int result = -1; WsXmlNodeH xmlNodeParam = NULL, xmlNodeTemp = NULL, xmlNodeAddr = NULL, xmlNodeRef = NULL; - WsXmlDocH xmlDocResponse = NULL; - WsXmlNsH ns = NULL; - client_opt_t *options = NULL; - filter_t *filter = NULL; - char *enumContext = NULL; - char *query_string = NULL; + g_auto(WsXmlDocH) xmlDocResponse = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autoptr(filter_t) filter = NULL; + g_autofree char *enumContext = NULL; + g_autofree char *query_string = NULL;
/* init and set up options */ options = wsmc_options_init(); if (!options) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options")); - goto cleanup; + return -1; } wsmc_set_action_option(options, FLAG_ENUMERATION_ENUM_EPR);
@@ -480,14 +478,14 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string); if (!filter) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create WQL filter")); - goto cleanup; + return -1; }
/* enumerate based on the filter from this query */ xmlDocResponse = wsmc_action_enumerate(priv->client, p->epr.info->rootUri, options, filter); if (hypervVerifyResponse(priv->client, xmlDocResponse, "enumeration") < 0) - goto cleanup; + return -1;
/* Get context */ enumContext = wsmc_get_enum_context(xmlDocResponse); @@ -498,41 +496,41 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, filter, enumContext);
if (hypervVerifyResponse(priv->client, xmlDocResponse, "pull") < 0) - goto cleanup; + return -1;
/* drill down and extract EPR node children */ if (!(xmlNodeTemp = ws_xml_get_soap_body(xmlDocResponse))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get SOAP body")); - goto cleanup; + return -1; }
if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, WSENUM_PULL_RESP))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response")); - goto cleanup; + return -1; }
if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ENUMERATION, WSENUM_ITEMS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get response items")); - goto cleanup; + return -1; }
if (!(xmlNodeTemp = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_EPR))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR items")); - goto cleanup; + return -1; }
if (!(xmlNodeAddr = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_ADDRESS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get EPR address")); - goto cleanup; + return -1; }
if (!(xmlNodeRef = ws_xml_get_child(xmlNodeTemp, 0, XML_NS_ADDRESSING, WSA_REFERENCE_PARAMETERS))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup EPR item reference parameters")); - goto cleanup; + return -1; }
/* now build a new xml doc with the EPR node children */ @@ -540,38 +538,26 @@ hypervSerializeEprParam(hypervParamPtr p, hypervPrivate *priv, p->epr.name, NULL))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add child node to methodNode")); - goto cleanup; + return -1; }
- if (!(ns = ws_xml_ns_add(xmlNodeParam, - "http://schemas.xmlsoap.org/ws/2004/08/addressing", "a"))) { + if (!ws_xml_ns_add(xmlNodeParam, "http://schemas.xmlsoap.org/ws/2004/08/addressing", "a")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set namespace address for xmlNodeParam")); - goto cleanup; + return -1; }
- if (!(ns = ws_xml_ns_add(xmlNodeParam, - "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd", "w"))) { + if (!ws_xml_ns_add(xmlNodeParam, "http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd", "w")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set wsman namespace address for xmlNodeParam")); - goto cleanup; + return -1; }
ws_xml_duplicate_tree(xmlNodeParam, xmlNodeAddr); ws_xml_duplicate_tree(xmlNodeParam, xmlNodeRef);
/* we did it! */ - result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options); - if (filter != NULL) - filter_destroy(filter);
Same question as before about whether these cleanup functions can safely be called with a NULL pointer.
- ws_xml_destroy_doc(xmlDocResponse); - VIR_FREE(enumContext); - VIR_FREE(query_string); - return result; + return 0; }
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 42 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index c1325b2ccc..b68b555a3b 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -903,14 +903,13 @@ int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, hypervObject **list) { - int result = -1; WsSerializerContextH serializerContext; - client_opt_t *options = NULL; - char *query_string = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autofree char *query_string = NULL; hypervWmiClassInfoPtr wmiInfo = wqlQuery->info; - filter_t *filter = NULL; - WsXmlDocH response = NULL; - char *enumContext = NULL; + g_autoptr(filter_t) filter = NULL; + g_auto(WsXmlDocH) response = NULL; + g_autofree char *enumContext = NULL; g_autoptr(hypervObject) head = NULL; hypervObject *tail = NULL; WsXmlNodeH node = NULL; @@ -931,7 +930,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (options == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not initialize options")); - goto cleanup; + return -1; } filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string); @@ -939,14 +938,14 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (filter == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create filter")); - goto cleanup; + return -1; } response = wsmc_action_enumerate(priv->client, wmiInfo->rootUri, options, filter); if (hypervVerifyResponse(priv->client, response, "enumeration") < 0) - goto cleanup; + return -1; enumContext = wsmc_get_enum_context(response); @@ -960,14 +959,14 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, filter, enumContext); if (hypervVerifyResponse(priv->client, response, "pull") < 0) - goto cleanup; + return -1; node = ws_xml_get_soap_body(response); if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup SOAP body")); - goto cleanup; + return -1; } node = ws_xml_get_child(node, 0, XML_NS_ENUMERATION, WSENUM_PULL_RESP); @@ -975,7 +974,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup pull response")); - goto cleanup; + return -1; } node = ws_xml_get_child(node, 0, XML_NS_ENUMERATION, WSENUM_ITEMS); @@ -983,7 +982,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup pull response items")); - goto cleanup; + return -1; } if (ws_xml_get_child(node, 0, wmiInfo->resourceUri, @@ -996,7 +995,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (data == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not deserialize pull response item")); - goto cleanup; + return -1; } object = g_new0(hypervObject, 1); @@ -1022,20 +1021,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, *list = head; head = NULL; - result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options); - - if (filter != NULL) - filter_destroy(filter); - - VIR_FREE(query_string); - ws_xml_destroy_doc(response); - VIR_FREE(enumContext); - - return result; + return 0; } -- 2.30.0
On 1/21/21 1:51 PM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 42 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index c1325b2ccc..b68b555a3b 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -903,14 +903,13 @@ int hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, hypervObject **list) { - int result = -1; WsSerializerContextH serializerContext; - client_opt_t *options = NULL; - char *query_string = NULL; + g_autoptr(client_opt_t) options = NULL; + g_autofree char *query_string = NULL; hypervWmiClassInfoPtr wmiInfo = wqlQuery->info; - filter_t *filter = NULL; - WsXmlDocH response = NULL; - char *enumContext = NULL; + g_autoptr(filter_t) filter = NULL; + g_auto(WsXmlDocH) response = NULL; + g_autofree char *enumContext = NULL; g_autoptr(hypervObject) head = NULL; hypervObject *tail = NULL; WsXmlNodeH node = NULL; @@ -931,7 +930,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (options == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not initialize options")); - goto cleanup; + return -1; }
filter = filter_create_simple(WSM_WQL_FILTER_DIALECT, query_string); @@ -939,14 +938,14 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (filter == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create filter")); - goto cleanup; + return -1; }
response = wsmc_action_enumerate(priv->client, wmiInfo->rootUri, options, filter);
if (hypervVerifyResponse(priv->client, response, "enumeration") < 0) - goto cleanup; + return -1;
enumContext = wsmc_get_enum_context(response);
@@ -960,14 +959,14 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, filter, enumContext);
if (hypervVerifyResponse(priv->client, response, "pull") < 0) - goto cleanup; + return -1;
node = ws_xml_get_soap_body(response);
if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup SOAP body")); - goto cleanup; + return -1; }
node = ws_xml_get_child(node, 0, XML_NS_ENUMERATION, WSENUM_PULL_RESP); @@ -975,7 +974,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup pull response")); - goto cleanup; + return -1; }
node = ws_xml_get_child(node, 0, XML_NS_ENUMERATION, WSENUM_ITEMS); @@ -983,7 +982,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (node == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not lookup pull response items")); - goto cleanup; + return -1; }
if (ws_xml_get_child(node, 0, wmiInfo->resourceUri, @@ -996,7 +995,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, if (data == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not deserialize pull response item")); - goto cleanup; + return -1; }
object = g_new0(hypervObject, 1); @@ -1022,20 +1021,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery, *list = head; head = NULL;
- result = 0; - - cleanup: - if (options != NULL) - wsmc_options_destroy(options); - - if (filter != NULL) - filter_destroy(filter);
And again.
- - VIR_FREE(query_string); - ws_xml_destroy_doc(response); - VIR_FREE(enumContext); - - return result; + return 0; }
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 44 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index b68b555a3b..96ae9a40c8 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -565,17 +565,16 @@ static int hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, WsXmlNodeH *methodNode) { - int result = -1; WsXmlNodeH xmlNodeInstance = NULL, xmlNodeProperty = NULL, xmlNodeParam = NULL, xmlNodeArray = NULL; - WsXmlDocH xmlDocTemp = NULL, - xmlDocCdata = NULL; - char *cdataContent = NULL; + g_auto(WsXmlDocH) xmlDocTemp = NULL; + g_auto(WsXmlDocH) xmlDocCdata = NULL; + g_autofree char *cdataContent = NULL; xmlNodePtr xmlNodeCdata = NULL; hypervWmiClassInfoPtr classInfo = p->embedded.info; - virHashKeyValuePairPtr items = NULL; + g_autofree virHashKeyValuePairPtr items = NULL; hypervCimTypePtr property = NULL; ssize_t numKeys = -1; int len = 0, i = 0; @@ -584,7 +583,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, NULL))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not add child node %s"), p->embedded.name); - goto cleanup; + return -1; } /* create the temp xml doc */ @@ -593,13 +592,13 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, if (!(xmlDocTemp = ws_xml_create_doc(NULL, "INSTANCE"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create temporary xml doc")); - goto cleanup; + return -1; } if (!(xmlNodeInstance = xml_parser_get_root(xmlDocTemp))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get temp xml doc root")); - goto cleanup; + return -1; } /* add CLASSNAME node to INSTANCE node */ @@ -607,7 +606,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, classInfo->name))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add attribute to node")); - goto cleanup; + return -1; } /* retrieve parameters out of hash table */ @@ -616,7 +615,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, if (!items) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not read embedded param hash table")); - goto cleanup; + return -1; } /* Add the parameters */ @@ -629,7 +628,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, &property) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not read type information")); - goto cleanup; + return -1; } if (!(xmlNodeProperty = ws_xml_add_child(xmlNodeInstance, NULL, @@ -637,19 +636,19 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, NULL))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add child to XML node")); - goto cleanup; + return -1; } if (!(ws_xml_add_node_attr(xmlNodeProperty, NULL, "NAME", name))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add attribute to XML node")); - goto cleanup; + return -1; } if (!(ws_xml_add_node_attr(xmlNodeProperty, NULL, "TYPE", property->type))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add attribute to XML node")); - goto cleanup; + return -1; } /* If this attribute is an array, add VALUE.ARRAY node */ @@ -658,7 +657,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, "VALUE.ARRAY", NULL))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add child to XML node")); - goto cleanup; + return -1; } } @@ -667,7 +666,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, NULL, "VALUE", value))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add child to XML node")); - goto cleanup; + return -1; } xmlNodeArray = NULL; @@ -682,7 +681,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, (xmlChar *)cdataContent, len))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create CDATA element")); - goto cleanup; + return -1; } /* @@ -694,18 +693,11 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char *resourceUri, if (!(xmlAddChild((xmlNodePtr)(void *)xmlNodeParam, xmlNodeCdata))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add CDATA to doc root")); - goto cleanup; + return -1; } /* we did it! */ - result = 0; - - cleanup: - VIR_FREE(items); - ws_xml_destroy_doc(xmlDocCdata); - ws_xml_destroy_doc(xmlDocTemp); - ws_xml_free_memory(cdataContent); - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_wmi.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 96ae9a40c8..a28bb0e815 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -398,38 +398,27 @@ hypervGetCimTypeInfo(hypervCimTypePtr typemap, const char *name, static int hypervCreateInvokeXmlDoc(hypervInvokeParamsListPtr params, WsXmlDocH *docRoot) { - int result = -1; - char *method = NULL; + g_autofree char *method = g_strdup_printf("%s_INPUT", params->method); + g_auto(WsXmlDocH) invokeXmlDocRoot = ws_xml_create_doc(NULL, method); WsXmlNodeH xmlNodeMethod = NULL; - method = g_strdup_printf("%s_INPUT", params->method); - - *docRoot = ws_xml_create_doc(NULL, method); - if (*docRoot == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not instantiate XML document")); - goto cleanup; + if (!invokeXmlDocRoot) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not instantiate XML document")); + return -1; } - xmlNodeMethod = xml_parser_get_root(*docRoot); - if (xmlNodeMethod == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not get root node of XML document")); - goto cleanup; + xmlNodeMethod = xml_parser_get_root(invokeXmlDocRoot); + if (!xmlNodeMethod) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not get root node of XML document")); + return -1; } /* add resource URI as namespace */ ws_xml_set_ns(xmlNodeMethod, params->resourceUri, "p"); - result = 0; + *docRoot = g_steal_pointer(&invokeXmlDocRoot); - cleanup: - if (result < 0 && *docRoot != NULL) { - ws_xml_destroy_doc(*docRoot); - *docRoot = NULL; - } - VIR_FREE(method); - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 2c0e9e0614..b81cedf426 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -496,9 +496,8 @@ hypervDomainAttachVirtualDisk(virDomainPtr domain, Msvm_ResourceAllocationSettingData *controller, const char *hostname) { - int result = -1; g_autofree char *parentInstanceID = NULL; - WsXmlDocH response = NULL; + g_auto(WsXmlDocH) response = NULL; VIR_DEBUG("Now attaching disk image '%s' with address %d to bus %d of type %d", disk->src->path, disk->info.addr.drive.unit, disk->info.addr.drive.controller, disk->bus); @@ -513,17 +512,12 @@ hypervDomainAttachVirtualDisk(virDomainPtr domain, parentInstanceID = hypervGetInstanceIDFromXMLResponse(response); if (!parentInstanceID) - goto cleanup; + return -1; if (hypervDomainAddVirtualHardDisk(domain, disk, hostname, parentInstanceID) < 0) - goto cleanup; - - result = 0; - - cleanup: - ws_xml_destroy_doc(response); + return -1; - return result; + return 0; } -- 2.30.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/hyperv/hyperv_driver.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index b81cedf426..bdc084790a 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -722,8 +722,7 @@ hypervDomainAttachCDROM(virDomainPtr domain, Msvm_ResourceAllocationSettingData *controller, const char *hostname) { - int result = -1; - WsXmlDocH response = NULL; + g_auto(WsXmlDocH) response = NULL; g_autofree char *driveInstanceID = NULL; VIR_DEBUG("Now attaching CD/DVD '%s' with address %d to bus %d of type %d", @@ -731,22 +730,16 @@ hypervDomainAttachCDROM(virDomainPtr domain, disk->info.addr.drive.controller, disk->bus); if (hypervDomainAddOpticalDrive(domain, disk, controller, hostname, &response) < 0) - goto cleanup; + return -1; driveInstanceID = hypervGetInstanceIDFromXMLResponse(response); if (!driveInstanceID) - goto cleanup; + return -1; if (hypervDomainAddOpticalDisk(domain, disk, hostname, driveInstanceID) < 0) - goto cleanup; - - result = 0; - - cleanup: - if (response) - ws_xml_destroy_doc(response); + return -1; - return result; + return 0; } -- 2.30.0
On Thu, Jan 21, 2021 at 1:53 PM Matt Coleman <mcoleman@datto.com> wrote:
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit.
Simplifications:
* add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types
Changes:
* store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016
Matt Coleman (55): hyperv: add a macro for retrieving setting data hyperv: store the Hyper-V version when connecting hyperv: add inheritance to the WMI generator hyperv: store hypervPrivate in hypervObject hyperv: enable use of g_autoptr for hypervObject hyperv: enable use of g_autoptr for the rest of the CIM/WMI classes hyperv: enable automatic cleanup for OpenWSMAN types hyperv: use g_autoptr for Win32_OperatingSystem in hypervConnectOpen hyperv: use g_autoptr for Win32_ComputerSystem in hypervConnectGetHostname hyperv: use g_autoptr for Msvm_ProcessorSettingData in hypervConnectGetMaxVcpus hyperv: use g_autoptr for WMI classes in hypervNodeGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByUUID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByName hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainResume hyperv: use g_autoptr for WMI classes in hypervDomainShutdownFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainDestroyFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetMaxMemory hyperv: use g_autoptr for WMI classes in hypervDomainSetMemoryProperty hyperv: use g_autoptr for Msvm_ComputerSystem in hypervRequestStateChange hyperv: use g_autoptr for Win32_ComputerSystemProduct in hypervLookupHostSystemBiosUuid hyperv: use g_autoptr for Msvm_ResourceAllocationSettingData in hypervDomainAttachPhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainAttachStorage hyperv: use g_autoptr for Msvm_DiskDrive in hypervDomainDefParsePhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainGetState hyperv: use g_autoptr for WMI classes in hypervDomainSetVcpusFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetVcpusFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainCreateWithFlags hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainGetAutostart hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainSetAutostart hyperv: use g_autoptr for WMI classes in hypervDomainGetSchedulerParametersFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainIsActive hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSave hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainHasManagedSaveImage hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSaveRemove hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListAllDomains hyperv: use GLib auto-cleanup in hypervDomainSendKey hyperv: use GLib auto-cleanup in hypervInvokeMethod hyperv: use GLib auto-cleanup in hypervInvokeMsvmComputerSystemRequestStateChange hyperv: use GLib auto-cleanup in hypervMsvmVSMSAddResourceSettings and hypervMsvmVSMSModifyResourceSettings hyperv: use g_autoptr for Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor in hypervDomainGetVcpus hyperv: use g_autoptr for Win32_OperatingSystem in hypervNodeGetFreeMemory hyperv: use GLib auto-cleanup in hypervDomainGetXMLDesc hyperv: use g_autoptr for WMI classes in hypervDomainAttachDeviceFlags hyperv: use GLib auto-cleanup in hypervSerializeEprParam hyperv: use GLib auto-cleanup in hypervEnumAndPull hyperv: use GLib auto-cleanup in hypervSerializeEmbeddedParam hyperv: use GLib auto-cleanup in hypervCreateInvokeXmlDoc hyperv: use g_auto for WsXmlDocH in hypervDomainAttachVirtualDisk hyperv: use g_auto for WsXmlDocH in hypervDomainAttachCDROM
scripts/hyperv_wmi_generator.py | 16 +- src/hyperv/hyperv_driver.c | 755 +++++++++++--------------------- src/hyperv/hyperv_private.h | 4 +- src/hyperv/hyperv_wmi.c | 408 +++++++---------- src/hyperv/hyperv_wmi.h | 4 +- src/hyperv/hyperv_wsman.h | 28 ++ 6 files changed, 457 insertions(+), 758 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h
-- 2.30.0
Quite the doozy, but looks mostly repetitious and simple... Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!
(Thought I sent this 7 hours ago before I went to sleep, but when I sat down this morning I saw it was still sitting there as a draft :-/) On 1/21/21 1:50 PM, Matt Coleman wrote:
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit.
Simplifications:
* add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types
Changes:
* store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016
I've gone through all of these, and just have two questions that affect multiple patches each (I've replied to the associated patches): 1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup? 2) There are several places where you're returning a value that is retrieved from an object that is being auto-freed, and I don't know enough about the details of the teardown code that's generated by the compiler to be certain that the return value would be retrieved from the object *before* it's freed rather than *after*. If someone knows the answer to this, then that's great, otherwise someone should compile a test program and list out the assembly code using gdb to see what the order is. Once those two questions are resolved (possibly requiring no change to your patches), then Reviewed-by: Laine Stump <laine@redhat.com>
Matt Coleman (55): hyperv: add a macro for retrieving setting data hyperv: store the Hyper-V version when connecting hyperv: add inheritance to the WMI generator hyperv: store hypervPrivate in hypervObject hyperv: enable use of g_autoptr for hypervObject hyperv: enable use of g_autoptr for the rest of the CIM/WMI classes hyperv: enable automatic cleanup for OpenWSMAN types hyperv: use g_autoptr for Win32_OperatingSystem in hypervConnectOpen hyperv: use g_autoptr for Win32_ComputerSystem in hypervConnectGetHostname hyperv: use g_autoptr for Msvm_ProcessorSettingData in hypervConnectGetMaxVcpus hyperv: use g_autoptr for WMI classes in hypervNodeGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByUUID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByName hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainResume hyperv: use g_autoptr for WMI classes in hypervDomainShutdownFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainDestroyFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetMaxMemory hyperv: use g_autoptr for WMI classes in hypervDomainSetMemoryProperty hyperv: use g_autoptr for Msvm_ComputerSystem in hypervRequestStateChange hyperv: use g_autoptr for Win32_ComputerSystemProduct in hypervLookupHostSystemBiosUuid hyperv: use g_autoptr for Msvm_ResourceAllocationSettingData in hypervDomainAttachPhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainAttachStorage hyperv: use g_autoptr for Msvm_DiskDrive in hypervDomainDefParsePhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainGetState hyperv: use g_autoptr for WMI classes in hypervDomainSetVcpusFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetVcpusFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainCreateWithFlags hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainGetAutostart hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainSetAutostart hyperv: use g_autoptr for WMI classes in hypervDomainGetSchedulerParametersFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainIsActive hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSave hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainHasManagedSaveImage hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSaveRemove hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListAllDomains hyperv: use GLib auto-cleanup in hypervDomainSendKey hyperv: use GLib auto-cleanup in hypervInvokeMethod hyperv: use GLib auto-cleanup in hypervInvokeMsvmComputerSystemRequestStateChange hyperv: use GLib auto-cleanup in hypervMsvmVSMSAddResourceSettings and hypervMsvmVSMSModifyResourceSettings hyperv: use g_autoptr for Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor in hypervDomainGetVcpus hyperv: use g_autoptr for Win32_OperatingSystem in hypervNodeGetFreeMemory hyperv: use GLib auto-cleanup in hypervDomainGetXMLDesc hyperv: use g_autoptr for WMI classes in hypervDomainAttachDeviceFlags hyperv: use GLib auto-cleanup in hypervSerializeEprParam hyperv: use GLib auto-cleanup in hypervEnumAndPull hyperv: use GLib auto-cleanup in hypervSerializeEmbeddedParam hyperv: use GLib auto-cleanup in hypervCreateInvokeXmlDoc hyperv: use g_auto for WsXmlDocH in hypervDomainAttachVirtualDisk hyperv: use g_auto for WsXmlDocH in hypervDomainAttachCDROM
scripts/hyperv_wmi_generator.py | 16 +- src/hyperv/hyperv_driver.c | 755 +++++++++++--------------------- src/hyperv/hyperv_private.h | 4 +- src/hyperv/hyperv_wmi.c | 408 +++++++---------- src/hyperv/hyperv_wmi.h | 4 +- src/hyperv/hyperv_wsman.h | 28 ++ 6 files changed, 457 insertions(+), 758 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h
On Fri, Jan 22, 2021 at 11:05:22AM -0500, Laine Stump wrote:
(Thought I sent this 7 hours ago before I went to sleep, but when I sat down this morning I saw it was still sitting there as a draft :-/)
On 1/21/21 1:50 PM, Matt Coleman wrote:
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit.
Simplifications:
* add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types
Changes:
* store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016
I've gone through all of these, and just have two questions that affect multiple patches each (I've replied to the associated patches):
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
The G_DEFINE_AUTOPTR_* macros alrady define wrappers that include a NULL check I believe. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 1/22/21 11:05 AM, Laine Stump wrote:
(Thought I sent this 7 hours ago before I went to sleep, but when I sat down this morning I saw it was still sitting there as a draft :-/)
On 1/21/21 1:50 PM, Matt Coleman wrote:
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit.
Simplifications:
* add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types
Changes:
* store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016
I've gone through all of these, and just have two questions that affect multiple patches each (I've replied to the associated patches):
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
2) There are several places where you're returning a value that is retrieved from an object that is being auto-freed, and I don't know enough about the details of the teardown code that's generated by the compiler to be certain that the return value would be retrieved from the object *before* it's freed rather than *after*. If someone knows the answer to this, then that's great, otherwise someone should compile a test program and list out the assembly code using gdb to see what the order is.
I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
Once those two questions are resolved (possibly requiring no change to your patches), then
Reviewed-by: Laine Stump <laine@redhat.com>
Matt Coleman (55): hyperv: add a macro for retrieving setting data hyperv: store the Hyper-V version when connecting hyperv: add inheritance to the WMI generator hyperv: store hypervPrivate in hypervObject hyperv: enable use of g_autoptr for hypervObject hyperv: enable use of g_autoptr for the rest of the CIM/WMI classes hyperv: enable automatic cleanup for OpenWSMAN types hyperv: use g_autoptr for Win32_OperatingSystem in hypervConnectOpen hyperv: use g_autoptr for Win32_ComputerSystem in hypervConnectGetHostname hyperv: use g_autoptr for Msvm_ProcessorSettingData in hypervConnectGetMaxVcpus hyperv: use g_autoptr for WMI classes in hypervNodeGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByUUID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByName hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainResume hyperv: use g_autoptr for WMI classes in hypervDomainShutdownFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainDestroyFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetMaxMemory hyperv: use g_autoptr for WMI classes in hypervDomainSetMemoryProperty hyperv: use g_autoptr for Msvm_ComputerSystem in hypervRequestStateChange hyperv: use g_autoptr for Win32_ComputerSystemProduct in hypervLookupHostSystemBiosUuid hyperv: use g_autoptr for Msvm_ResourceAllocationSettingData in hypervDomainAttachPhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainAttachStorage hyperv: use g_autoptr for Msvm_DiskDrive in hypervDomainDefParsePhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainGetState hyperv: use g_autoptr for WMI classes in hypervDomainSetVcpusFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetVcpusFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainCreateWithFlags hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainGetAutostart hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainSetAutostart hyperv: use g_autoptr for WMI classes in hypervDomainGetSchedulerParametersFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainIsActive hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSave hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainHasManagedSaveImage hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSaveRemove hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListAllDomains hyperv: use GLib auto-cleanup in hypervDomainSendKey hyperv: use GLib auto-cleanup in hypervInvokeMethod hyperv: use GLib auto-cleanup in hypervInvokeMsvmComputerSystemRequestStateChange hyperv: use GLib auto-cleanup in hypervMsvmVSMSAddResourceSettings and hypervMsvmVSMSModifyResourceSettings hyperv: use g_autoptr for Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor in hypervDomainGetVcpus hyperv: use g_autoptr for Win32_OperatingSystem in hypervNodeGetFreeMemory hyperv: use GLib auto-cleanup in hypervDomainGetXMLDesc hyperv: use g_autoptr for WMI classes in hypervDomainAttachDeviceFlags hyperv: use GLib auto-cleanup in hypervSerializeEprParam hyperv: use GLib auto-cleanup in hypervEnumAndPull hyperv: use GLib auto-cleanup in hypervSerializeEmbeddedParam hyperv: use GLib auto-cleanup in hypervCreateInvokeXmlDoc hyperv: use g_auto for WsXmlDocH in hypervDomainAttachVirtualDisk hyperv: use g_auto for WsXmlDocH in hypervDomainAttachCDROM
scripts/hyperv_wmi_generator.py | 16 +- src/hyperv/hyperv_driver.c | 755 +++++++++++--------------------- src/hyperv/hyperv_private.h | 4 +- src/hyperv/hyperv_wmi.c | 408 +++++++---------- src/hyperv/hyperv_wmi.h | 4 +- src/hyperv/hyperv_wsman.h | 28 ++ 6 files changed, 457 insertions(+), 758 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h
On Jan 22, 2021, at 11:30 AM, Laine Stump <laine@redhat.com> wrote:
On 1/22/21 11:05 AM, Laine Stump wrote:
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
For WsXmlDocH, GLib's documentation says that G_DEFINE_AUTOPTR_CLEANUP_FUNC() is NULL-safe:
The function will not be called if the variable to be cleaned up contains NULL.
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEF... For client_opt_t and filter_t, the third parameter to G_DEFINE_AUTO_CLEANUP_FREE_FUNC() was used to prevent the types' free functions from being called if they're NULL. -- Matt
On Jan 22, 2021, at 12:07 PM, Matt Coleman <mcoleman@datto.com> wrote:
On Jan 22, 2021, at 11:30 AM, Laine Stump <laine@redhat.com> wrote:
On 1/22/21 11:05 AM, Laine Stump wrote:
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
For WsXmlDocH, GLib's documentation says that G_DEFINE_AUTOPTR_CLEANUP_FUNC() is NULL-safe:
The function will not be called if the variable to be cleaned up contains NULL.
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEF...
For client_opt_t and filter_t, the third parameter to G_DEFINE_AUTO_CLEANUP_FREE_FUNC() was used to prevent the types' free functions from being called if they're NULL.
Oh, woops. Caffeine hasn't kicked in. I got that all backwards. There is an issue with client_opt_t, which doesn't have a NULL check: https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f... filter_t does have a NULL check: https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f... -- Matt
On Jan 22, 2021, at 12:12 PM, Matt Coleman <mcoleman@datto.com> wrote:
On Jan 22, 2021, at 12:07 PM, Matt Coleman <mcoleman@datto.com> wrote:
On Jan 22, 2021, at 11:30 AM, Laine Stump <laine@redhat.com> wrote:
On 1/22/21 11:05 AM, Laine Stump wrote:
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
For WsXmlDocH, GLib's documentation says that G_DEFINE_AUTOPTR_CLEANUP_FUNC() is NULL-safe:
The function will not be called if the variable to be cleaned up contains NULL.
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEF...
For client_opt_t and filter_t, the third parameter to G_DEFINE_AUTO_CLEANUP_FREE_FUNC() was used to prevent the types' free functions from being called if they're NULL.
Oh, woops. Caffeine hasn't kicked in. I got that all backwards.
There is an issue with client_opt_t, which doesn't have a NULL check: https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f...
filter_t does have a NULL check: https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f...
-- Matt
Evidently, I'm not the sharpest knife in the drawer today: this is NULL-safe as it is. For WsXmlDocH, G_DEFINE_AUTO_CLEANUP_FREE_FUNC's third parameter ensures that the cleanup function won't be called if the variable is NULL: https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEF... On top of that, its cleanup function also includes a NULL check: https://github.com/Openwsman/openwsman/blob/bcf670dba4d81ec1669cfd1dd7e2b57f... For client_opt_t and filter_t, G_DEFINE_AUTOPTR_CLEANUP_FUNC expands to code that contains a NULL check: https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEF... -- Matt
participants (5)
-
Daniel P. Berrangé -
Laine Stump -
Laine Stump -
Matt Coleman -
Neal Gompa