Re: [PATCH v2 1/7] hyperv: implement domainSetAutostart

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/13/20 7:13 AM, Matt Coleman wrote:
Co-authored-by: Sri Ramanujam <sramanujam@xxxxxxxxx>
Signed-off-by: Matt Coleman <matt@xxxxxxxxx>
---
  src/hyperv/hyperv_driver.c | 91 ++++++++++++++++++++++++++++++++++++++
  1 file changed, 91 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 2ac30fa4c6..79b48a9dff 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1310,6 +1310,96 @@ 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;
+    hypervInvokeParamsListPtr params = NULL;
+    g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
+    virHashTablePtr autostartParam = NULL;
+    hypervWmiClassInfoListPtr embeddedParamClass = NULL;
+    const char *methodName = NULL, *embeddedParamName = NULL;
+    char enabledValue[] = "2", disabledValue[] = "0";
+
+    if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
+        methodName = "ModifyVirtualSystem";
+        embeddedParamName = "SystemSettingData";
+        embeddedParamClass = Msvm_VirtualSystemGlobalSettingData_WmiInfo;
+    } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
+        methodName = "ModifySystemSettings";
+        embeddedParamName = "SystemSettings";
+        embeddedParamClass = Msvm_VirtualSystemSettingData_WmiInfo;
+        enabledValue[0] = '4';
+        disabledValue[0] = '2';
+    }

As pino pointed out, this can be just one if.

+
+    virUUIDFormat(domain->uuid, uuid_string);
+
+    if (hypervGetVSSDFromUUID(priv, uuid_string, &vssd) < 0)
+        goto cleanup;
+
+    params = hypervCreateInvokeParamsList(priv, methodName,
+                                MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
+                                Msvm_VirtualSystemManagementService_WmiInfo);
+
+    if (!params) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not create params"));
+        goto cleanup;
+    }
+
+    if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
+        virBufferEscapeSQL(&eprQuery,
+                           MSVM_COMPUTERSYSTEM_WQL_SELECT "WHERE Name = '%s'",
+                           uuid_string);
+
+        if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery,
+                              Msvm_ComputerSystem_WmiInfo) < 0)
+            goto params_cleanup;
+    }
+
+    autostartParam = hypervCreateEmbeddedParam(priv, embeddedParamClass);
+
+    if (hypervSetEmbeddedProperty(autostartParam, "AutomaticStartupAction",
+                              autostart ? enabledValue : disabledValue) < 0) {
+        hypervFreeEmbeddedParam(autostartParam);
+        goto params_cleanup;
+    }
+
+    if (hypervSetEmbeddedProperty(autostartParam, "InstanceID",
+                                  vssd->data.common->InstanceID) < 0) {
+        hypervFreeEmbeddedParam(autostartParam);
+        goto params_cleanup;
+    }
+
+    if (hypervAddEmbeddedParam(params, priv, embeddedParamName, autostartParam,
+                               embeddedParamClass) < 0) {
+        hypervFreeEmbeddedParam(autostartParam);
+        goto params_cleanup;
+    }
+
+    if (hypervInvokeMethod(priv, params, NULL) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Could not set autostart"));
+        goto cleanup;
+    }
+
+    result = 0;
+    goto cleanup;
+
+ params_cleanup:
+    hypervFreeInvokeParams(params);

So the only reason for the separate lable is that hypervInvokeMethod() consumes @params and thus we must avoid jumping here once it was called. Fortunately, hypervFreeInvokeParams() accepts NULL so what we can do is to set params = NULL in both success and fail paths.

I'll post a patch that makes hypervInvokeMethod() behave more sanely.

Anyway, this is what I suggest is squashed in (I can squash it before pushing if you agree):


diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 79b48a9dff..1eae7c38e2 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1309,7 +1309,6 @@ hypervDomainGetAutostart(virDomainPtr domain, int *autostart)
 }


-
 static int
 hypervDomainSetAutostart(virDomainPtr domain, int autostart)
 {
@@ -1320,15 +1319,12 @@ hypervDomainSetAutostart(virDomainPtr domain, int autostart)
     hypervInvokeParamsListPtr params = NULL;
     g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
     virHashTablePtr autostartParam = NULL;
-    hypervWmiClassInfoListPtr embeddedParamClass = NULL;
-    const char *methodName = NULL, *embeddedParamName = NULL;
+ hypervWmiClassInfoListPtr embeddedParamClass = Msvm_VirtualSystemGlobalSettingData_WmiInfo;
+    const char *methodName = "ModifyVirtualSystem";
+    const char *embeddedParamName = "SystemSettingData";
     char enabledValue[] = "2", disabledValue[] = "0";

-    if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
-        methodName = "ModifyVirtualSystem";
-        embeddedParamName = "SystemSettingData";
-        embeddedParamClass = Msvm_VirtualSystemGlobalSettingData_WmiInfo;
-    } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
+    if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
         methodName = "ModifySystemSettings";
         embeddedParamName = "SystemSettings";
         embeddedParamClass = Msvm_VirtualSystemSettingData_WmiInfo;
@@ -1342,8 +1338,8 @@ hypervDomainSetAutostart(virDomainPtr domain, int autostart)
         goto cleanup;

     params = hypervCreateInvokeParamsList(priv, methodName,
- MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, - Msvm_VirtualSystemManagementService_WmiInfo); + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR, + Msvm_VirtualSystemManagementService_WmiInfo);

     if (!params) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1358,48 +1354,47 @@ hypervDomainSetAutostart(virDomainPtr domain, int autostart)

         if (hypervAddEprParam(params, "ComputerSystem", priv, &eprQuery,
                               Msvm_ComputerSystem_WmiInfo) < 0)
-            goto params_cleanup;
+            goto cleanup;
     }

     autostartParam = hypervCreateEmbeddedParam(priv, embeddedParamClass);

if (hypervSetEmbeddedProperty(autostartParam, "AutomaticStartupAction", - autostart ? enabledValue : disabledValue) < 0) { + autostart ? enabledValue : disabledValue) < 0) {
         hypervFreeEmbeddedParam(autostartParam);
-        goto params_cleanup;
+        goto cleanup;
     }

     if (hypervSetEmbeddedProperty(autostartParam, "InstanceID",
                                   vssd->data.common->InstanceID) < 0) {
         hypervFreeEmbeddedParam(autostartParam);
-        goto params_cleanup;
+        goto cleanup;
     }

if (hypervAddEmbeddedParam(params, priv, embeddedParamName, autostartParam,
                                embeddedParamClass) < 0) {
         hypervFreeEmbeddedParam(autostartParam);
-        goto params_cleanup;
+        goto cleanup;
     }

     if (hypervInvokeMethod(priv, params, NULL) < 0) {
+        params = NULL;
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not set autostart"));
         goto cleanup;
     }

+    params = NULL;
     result = 0;
-    goto cleanup;

- params_cleanup:
-    hypervFreeInvokeParams(params);
  cleanup:
+    hypervFreeInvokeParams(params);
     hypervFreeObject(priv, (hypervObject *) vssd);

     return result;
 }


-
 static int
 hypervConnectIsEncrypted(virConnectPtr conn)
 {




Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux