2017-05-19 22:58 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 | 94 +++++++++++++++++++++++++++++++++++
> src/hyperv/hyperv_wmi.c | 51 +++++++++++++++++++
> src/hyperv/hyperv_wmi.h | 11 ++++
> src/hyperv/hyperv_wmi_generator.input | 30 +++++++++++
> 4 files changed, 186 insertions(+)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index a01515a..455e1cd 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1481,6 +1481,98 @@ 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;
Use VIR_DIV_UP(memory, 1024) here, to round up to full megabytes.
> + 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 = hypervCreateInvokeParamsList(priv, "ModifyVirtualSystemResources",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
> + Msvm_VirtualSystemManagementService_WmiInfo);
Shouldn't you check for params == NULL here?
> +
> + 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 = hypervCreateInvokeParamsList(priv, "ModifyResourceSettings",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
> + Msvm_VirtualSystemManagementService_WmiInfo);
Shouldn't you check for params == NULL here?
> + }
> +
> + memResource = hypervCreateEmbeddedParam(priv, Msvm_MemorySettingData_WmiInfo);
> + if (memResource == NULL)
> + goto cleanup;
> +
> + if (hypervSetEmbeddedProperty(memResource, "VirtualQuantity", memory_str) < 0)
memResource leaks here.
> + goto cleanup;
> +
> + if (hypervSetEmbeddedProperty(memResource, "InstanceID",
> + memsd->data.common->InstanceID) < 0)
memResource leaks here.
> + goto cleanup;
> +
> + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
> + if (hypervAddEmbeddedParam(params, priv, "ResourceSettingData",
> + memResource, Msvm_MemorySettingData_WmiInfo) < 0)
memResource leaks here.
> + goto cleanup;
> +
> + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
> + if (hypervAddEmbeddedParam(params, priv, "ResourceSettings",
> + memResource, Msvm_MemorySettingData_WmiInfo) < 0)
memResource leaks here.
> + goto cleanup;
> + }
Only now after an successful hypervAddEmbeddedParam call memResource
will not leak anymore because the hypervInvokeMethod will free it as
part of the hypervInvokeParamsListPtr cleanup.
I suggest adding a hypervFreeEmbeddedParam function that does the
hashtable cleanup and call it from hypervFreeInvokeParams instead of
doing the hashtable cleanup directly in hypervFreeInvokeParams.
Then set memResource to NULL here and ...
> +
> + 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);
... call the new hypervFreeEmbeddedParam(memResource) here to catch
all the cases that goto here before memResource was added successfully
to the hypervInvokeParamsListPtr. The potential double free is avoided
by setting memResource to NULL after is was successfully added to the
hypervInvokeParamsListPtr that will then take care of its cleanup.
> + 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 */
> @@ -1515,6 +1607,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 2165838..f50a58c 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -1619,3 +1619,54 @@ hypervMsvmComputerSystemFromDomain(virDomainPtr domain,
>
> return 0;
> }
> +
> +
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * Msvm_VirtualSystemSettingData
> + */
> +
> +int
> +hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv,
> + const char *uuid_string, Msvm_VirtualSystemSettingData **list)
> +{
> + 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)
> + return -1;
> +
> + return 0;
> +}
> +
> +
> +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> + * Msvm_MemorySettingData
> + */
> +
> +int
> +hypervGetMsvmMemorySettingDataFromVSSD(hypervPrivate *priv,
> + const char *vssd_instanceid, Msvm_MemorySettingData **list)
> +{
> + 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)
> + return -1;
> +
> + return 0;
> +}
> diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
> index eb6f43d..c979b99 100644
> --- a/src/hyperv/hyperv_wmi.h
> +++ b/src/hyperv/hyperv_wmi.h
> @@ -35,6 +35,9 @@
>
> # define HYPERV_DEFAULT_PARAM_COUNT 5
>
> +# define MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR \
> + "CreationClassName=Msvm_VirtualSystemManagementService"
> +
> int hypervVerifyResponse(WsManClient *client, WsXmlDocH response,
> const char *detail);
>
> @@ -210,6 +213,10 @@ int hypervGetMsvmVirtualSystemSettingDataList(hypervPrivate *priv,
> virBufferPtr query,
> Msvm_VirtualSystemSettingData **list);
>
> +int hypervGetMsvmVirtualSystemSettingDataFromUUID(hypervPrivate *priv,
> + const char *uuid_string,
> + Msvm_VirtualSystemSettingData **list);
> +
> int hypervGetMsvmProcessorSettingDataList(hypervPrivate *priv,
> virBufferPtr query,
> Msvm_ProcessorSettingData **list);
> @@ -217,6 +224,10 @@ int hypervGetMsvmProcessorSettingDataList(hypervPrivate *priv,
> int hypervGetMsvmMemorySettingDataList(hypervPrivate *priv, virBufferPtr query,
> Msvm_MemorySettingData **list);
>
> +int hypervGetMsvmMemorySettingDataFromVSSD(hypervPrivate *priv,
> + const char *vssd_instanceid,
> + Msvm_MemorySettingData **list);
> +
> int hypervGetMsvmKeyboardList(hypervPrivate *priv, virBufferPtr query,
> Msvm_Keyboard **list);
>
> diff --git a/src/hyperv/hyperv_wmi_generator.input b/src/hyperv/hyperv_wmi_generator.input
> index 4ccda04..da19765 100644
> --- a/src/hyperv/hyperv_wmi_generator.input
> +++ b/src/hyperv/hyperv_wmi_generator.input
> @@ -787,6 +787,36 @@ class Msvm_VirtualSystemManagementService
> boolean Started
> end
>
> +class v2/Msvm_VirtualSystemManagementService
> + string InstanceID
> + string Caption
> + string Description
> + string ElementName
> + datetime InstallDate
> + string Name
> + uint16 OperationalStatus[]
> + string StatusDescriptions[]
> + string Status
> + uint16 HealthState
> + uint16 CommunicationStatus
> + uint16 DetailedStatus
> + uint16 OperatingStatus
> + uint16 PrimaryStatus
> + uint16 EnabledState
> + string OtherEnabledState
> + uint16 RequestedState
> + uint16 EnabledDefault
> + datetime TimeOfLastStateChange
> + uint16 AvailableRequestedStates[]
> + uint16 TransitioningToState
> + string SystemCreationClassName
> + string SystemName
> + string CreationClassName
> + string PrimaryOwnerName
> + string PrimaryOwnerContact
> + string StartMode
> + boolean Started
> +end
>
> class Msvm_VirtualSystemGlobalSettingData
> string Caption
> --
> 2.9.4
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
