2017-04-19 20:21 GMT+02:00 Sri Ramanujam <sramanujam@xxxxxxxxx>: > Introduces support for virDomainSetMemory. This also serves an an > example for how to use the new method invocation API with a more > complicated method, this time including an EPR and embedded param. > --- > src/hyperv/hyperv_driver.c | 97 +++++++++++++++++++++++++++++++++++ > src/hyperv/hyperv_wmi.c | 60 ++++++++++++++++++++++ > src/hyperv/hyperv_wmi.h | 11 ++++ > src/hyperv/hyperv_wmi_generator.input | 30 +++++++++++ > 4 files changed, 198 insertions(+) > > diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c > index 9562d5a..e01f63b 100644 > --- a/src/hyperv/hyperv_driver.c > +++ b/src/hyperv/hyperv_driver.c > @@ -1459,6 +1459,101 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, > } > > > +static int > +hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, > + unsigned int flags) > +{ > + int result = -1; > + char uuid_string[VIR_UUID_STRING_BUFLEN]; > + hypervPrivate *priv = domain->conn->privateData; > + char *memory_str = NULL; > + hypervInvokeParamsListPtr params = NULL; > + unsigned long memory_mb = memory / 1024; > + Msvm_VirtualSystemSettingData *vssd = NULL; > + Msvm_MemorySettingData *memsd = NULL; > + virBuffer eprQuery = VIR_BUFFER_INITIALIZER; > + virHashTablePtr memResource = NULL; > + > + virCheckFlags(0, -1); > + > + /* memory has to be a multiple of 2; round up if necessary */ > + if (memory_mb % 2) memory_mb++; > + > + if (virAsprintf(&memory_str, "%lu", memory_mb) < 0) > + goto cleanup; > + > + virUUIDFormat(domain->uuid, uuid_string); > + > + if (hypervGetMsvmVirtualSystemSettingDataFromUUID(priv, uuid_string, &vssd) < 0) > + goto cleanup; > + > + if (hypervGetMsvmMemorySettingDataFromVSSD(priv, vssd->data.common->InstanceID, > + &memsd) < 0) > + goto cleanup; > + > + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { > + params = hypervInitInvokeParamsList(priv, "ModifyVirtualSystemResources", > + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, > + Msvm_VirtualSystemManagementService_WmiInfo); > + > + virBufferAddLit(&eprQuery, MSVM_COMPUTERSYSTEM_WQL_SELECT); > + virBufferAsprintf(&eprQuery, "where Name = \"%s\"", uuid_string); > + > + if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery, > + Msvm_ComputerSystem_WmiInfo) < 0) > + goto cleanup; > + > + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) { > + params = hypervInitInvokeParamsList(priv, "ModifyResourceSettings", > + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, > + Msvm_VirtualSystemManagementService_WmiInfo); > + } > + > + memResource = hypervCreateEmbeddedParam(priv, Msvm_MemorySettingData_WmiInfo); This allocates a virHashTable but I cannot find the code that actually frees it again. I think this leaks the hash table in the error and the success path. > + if (memResource == NULL) > + goto cleanup; > + > + if (hypervSetEmbeddedProperty(memResource, "VirtualQuantity", memory_str) < 0) > + goto cleanup; You created the hash table with the virHashValueFree as the dataFree function. That means that the hash table takes ownership of the values you pass into it and will free them using virHashValueFree when the hash table itself is freed or the value gets replaced. Because hypervSetEmbeddedProperty basically just calls virHashUpdateEntry you passed the ownership to memory_str to the hash table, but you also free memory_str here in the cleanup section in all cases. This would result in a double free. Currently if doesn't because you leak the hash table completely. > + if (hypervSetEmbeddedProperty(memResource, "InstanceID", > + memsd->data.common->InstanceID) < 0) > + goto cleanup; > + > + > + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { > + if (hypervAddEmbeddedParam(params, priv, "ResourceSettingData", > + memResource, Msvm_MemorySettingData_WmiInfo) < 0) > + goto cleanup; > + > + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) { > + if (hypervAddEmbeddedParam(params, priv, "ResourceSettings", > + memResource, Msvm_MemorySettingData_WmiInfo) < 0) > + goto cleanup; > + } > + > + if (hypervInvokeMethod(priv, params, NULL) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set memory")); > + goto cleanup; > + } > + > + result = 0; > + cleanup: > + VIR_FREE(memory_str); > + hypervFreeObject(priv, (hypervObject *) vssd); > + hypervFreeObject(priv, (hypervObject *) memsd); > + virBufferFreeAndReset(&eprQuery); > + return result; > +} > + > + > +static int > +hypervDomainSetMemory(virDomainPtr domain, unsigned long memory) > +{ > + return hypervDomainSetMemoryFlags(domain, memory, 0); > +} > + > + > static virHypervisorDriver hypervHypervisorDriver = { > .name = "Hyper-V", > .connectOpen = hypervConnectOpen, /* 0.9.5 */ > @@ -1493,6 +1588,8 @@ static virHypervisorDriver hypervHypervisorDriver = { > .domainHasManagedSaveImage = hypervDomainHasManagedSaveImage, /* 0.9.5 */ > .domainManagedSaveRemove = hypervDomainManagedSaveRemove, /* 0.9.5 */ > .domainSendKey = hypervDomainSendKey, /* TODO: version */ > + .domainSetMemory = hypervDomainSetMemory, /* TODO: version */ > + .domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* TODO: version */ > .connectIsAlive = hypervConnectIsAlive, /* 0.9.8 */ > }; > > diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c > index 8025371..0c46566 100644 > --- a/src/hyperv/hyperv_wmi.c > +++ b/src/hyperv/hyperv_wmi.c > @@ -1595,3 +1595,63 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain, > > return 0; > } > + > + > +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > + * Msvm_VirtualSystemSettingData > + */ > + > +int > +hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv, > + const char *uuid_string, Msvm_VirtualSystemSettingData **list) > +{ > + int result = -1; > + virBuffer query = VIR_BUFFER_INITIALIZER; > + > + virBufferAsprintf(&query, > + "associators of " > + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\"," > + "Name=\"%s\"} " > + "where AssocClass = Msvm_SettingsDefineState " > + "ResultClass = Msvm_VirtualSystemSettingData", > + uuid_string); > + > + if (hypervGetWmiClassList(priv, Msvm_VirtualSystemSettingData_WmiInfo, &query, > + (hypervObject **) list) < 0 || *list == NULL) > + goto cleanup; > + > + result = 0; > + > + cleanup: > + virBufferFreeAndReset(&query); > + return result; > +} hypervGetWmiClassList guarantees that the query will be freed in all cases. There is no need to free it here again. > + > +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > + * Msvm_MemorySettingData > + */ > + > +int > +hypervGetMsvmMemorySettingDataFromVSSD(hypervPrivate *priv, > + const char *vssd_instanceid, Msvm_MemorySettingData **list) > +{ > + int result = -1; > + virBuffer query = VIR_BUFFER_INITIALIZER; > + > + virBufferAsprintf(&query, > + "associators of " > + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} " > + "where AssocClass = Msvm_VirtualSystemSettingDataComponent " > + "ResultClass = Msvm_MemorySettingData", > + vssd_instanceid); > + > + if (hypervGetWmiClassList(priv, Msvm_MemorySettingData_WmiInfo, &query, > + (hypervObject **) list) < 0 || *list == NULL) > + goto cleanup; > + > + result = 0; > + cleanup: > + virBufferFreeAndReset(&query); > + return result; > +} hypervGetWmiClassList guarantees that the query will be freed in all cases. There is no need to free it here again. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list