[libvirt] [PATCH v2 5/5] hyperv: Add support for virDomainSetMemory
Matthias Bolte
matthias.bolte at googlemail.com
Sat Apr 22 15:00:06 UTC 2017
2017-04-19 20:21 GMT+02:00 Sri Ramanujam <sramanujam at datto.com>:
> 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
More information about the libvir-list
mailing list