[PATCH v3 3/9] hyperv: implement domainSetAutostart

[Michal Privoznik]

On 10/21/20 10:46 AM, Matt Coleman wrote:
> Co-authored-by: Sri Ramanujam <sramanujam at datto.com>
> Signed-off-by: Matt Coleman <matt at datto.com>
> ---
>   src/hyperv/hyperv_driver.c | 85 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 85 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 091b77ebc4..1521cf4244 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1316,6 +1316,90 @@ 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(hypervInvokeParamsList) params = NULL;
> +    g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
> +    g_autoptr(virHashTable) autostartParam = NULL;
> +    const char *methodName = NULL;
> +    hypervWmiClassInfoListPtr embeddedParamClass = NULL;
> +    const char *enabledValue = NULL, *disabledValue = NULL;
> +    const char *embeddedParamName = NULL;
> +
> +    switch (priv->wmiVersion) {
> +    case HYPERV_WMI_VERSION_V1:
> +        methodName = "ModifyVirtualSystem";
> +        embeddedParamClass = Msvm_VirtualSystemGlobalSettingData_WmiInfo;
> +        enabledValue = "2";
> +        disabledValue = "0";
> +        embeddedParamName = "SystemSettingData";
> +        break;
> +    case HYPERV_WMI_VERSION_V2:
> +        methodName = "ModifySystemSettings";
> +        embeddedParamClass = Msvm_VirtualSystemSettingData_WmiInfo;
> +        enabledValue = "4";
> +        disabledValue = "2";
> +        embeddedParamName = "SystemSettings";
> +        break;
> +    }
> +
> +    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 cleanup;
> +    }
> +
> +    autostartParam = hypervCreateEmbeddedParam(priv, embeddedParamClass);
> +
> +    if (hypervSetEmbeddedProperty(autostartParam, "AutomaticStartupAction",
> +                                  autostart ? enabledValue : disabledValue) < 0)
> +        goto cleanup;
> +
> +    if (hypervSetEmbeddedProperty(autostartParam, "InstanceID", vssd->data.common->InstanceID) < 0)
> +        goto cleanup;
> +
> +    if (hypervAddEmbeddedParam(params, priv, embeddedParamName, &autostartParam,
> +                               embeddedParamClass) < 0)

Nit pick - when you have to break an if() condition like this then the 
second line should be longer than the first one. Like this:

     if (hypervAddEmbeddedParam(params, priv, embeddedParamName,
                                &autostartParam, embeddedParamClass) < 0)


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

No need to report error here because hypervInvokeMethod() already 
reported one for us. And unfortunately we don't have stacked error 
messages, so this would just overwrite the error reported earlier.

Michal