[PATCH 03/11] hyperv: implement domainCreateXML and domainDefineXML

[Daniel P. Berrangé]

On Tue, Nov 24, 2020 at 02:48:32PM -0500, 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 | 114 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 8e16ff529f..559b60d3df 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -703,6 +703,9 @@ hypervFreePrivate(hypervPrivate **priv)
>      if ((*priv)->caps)
>          virObjectUnref((*priv)->caps);
>  
> +    if ((*priv)->xmlopt)
> +        virObjectUnref((*priv)->xmlopt);
> +
>      hypervFreeParsedUri(&(*priv)->parsedUri);
>      VIR_FREE(*priv);
>  }
> @@ -735,6 +738,8 @@ hypervInitConnection(virConnectPtr conn, hypervPrivate *priv,
>  }
>  
>  
> +virDomainDefParserConfig hypervDomainDefParserConfig;
> +
>  static virDrvOpenStatus
>  hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>                    virConfPtr conf G_GNUC_UNUSED,
> @@ -787,6 +792,9 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>      if (!priv->caps)
>          goto cleanup;
>  
> +    /* init xmlopt for domain XML */
> +    priv->xmlopt = virDomainXMLOptionNew(&hypervDomainDefParserConfig, NULL, NULL, NULL, NULL);
> +
>      conn->privateData = priv;
>      priv = NULL;
>      result = VIR_DRV_OPEN_SUCCESS;
> @@ -1932,6 +1940,105 @@ hypervDomainUndefine(virDomainPtr domain)
>  }
>  
>  
> +static virDomainPtr
> +hypervDomainDefineXML(virConnectPtr conn, const char *xml)
> +{
> +    hypervPrivate *priv = conn->privateData;
> +    g_autoptr(virDomainDef) def = NULL;
> +    virDomainPtr domain = NULL;
> +    g_autoptr(hypervInvokeParamsList) params = NULL;
> +    g_autoptr(GHashTable) defineSystemParam = NULL;
> +
> +    /* parse xml */
> +    def = virDomainDefParseString(xml, priv->xmlopt, NULL,
> +                                  1 << VIR_DOMAIN_VIRT_HYPERV | VIR_DOMAIN_XML_INACTIVE);
> +
> +    if (!def)
> +        goto error;
> +
> +    /* abort if a domain with this UUID already exists */
> +    if ((domain = hypervDomainLookupByUUID(conn, def->uuid))) {
> +        char uuid_string[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(domain->uuid, uuid_string);
> +        virReportError(VIR_ERR_DOM_EXIST, _("Domain already exists with UUID '%s'"), uuid_string);
> +
> +        // Don't use the 'exit' label, since we don't want to delete the existing domain.
> +        virObjectUnref(domain);
> +        return NULL;
> +    }
> +
> +    /* prepare params: only set the VM's name for now */
> +    params = hypervCreateInvokeParamsList("DefineSystem",
> +                                          MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR,
> +                                          Msvm_VirtualSystemManagementService_WmiInfo);
> +
> +    if (!params)
> +        goto error;
> +
> +    defineSystemParam = hypervCreateEmbeddedParam(Msvm_VirtualSystemSettingData_WmiInfo);
> +
> +    if (hypervSetEmbeddedProperty(defineSystemParam, "ElementName", def->name) < 0)
> +        goto error;
> +
> +    if (hypervAddEmbeddedParam(params, "SystemSettings",
> +                               &defineSystemParam, Msvm_VirtualSystemSettingData_WmiInfo) < 0)
> +        goto error;
> +
> +    /* create the VM */
> +    if (hypervInvokeMethod(priv, &params, NULL) < 0)
> +        goto error;
> +
> +    /* populate a domain ptr so that we can edit it */
> +    domain = hypervDomainLookupByName(conn, def->name);
> +
> +    /* set domain vcpus */
> +    if (def->vcpus && hypervDomainSetVcpus(domain, def->maxvcpus) < 0)
> +        goto error;
> +
> +    /* set VM maximum memory */
> +    if (def->mem.max_memory > 0 && hypervDomainSetMaxMemory(domain, def->mem.max_memory) < 0)
> +        goto error;
> +
> +    /* set VM memory */
> +    if (def->mem.cur_balloon > 0 && hypervDomainSetMemory(domain, def->mem.cur_balloon) < 0)
> +        goto error;
> +
> +    return domain;
> +
> + error:
> +    VIR_DEBUG("Domain creation failed, rolling back");
> +    if (domain)
> +        hypervDomainUndefine(domain);
> +
> +    return NULL;
> +}
> +
> +
> +static virDomainPtr
> +hypervDomainCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags)
> +{
> +    virDomainPtr domain;
> +
> +    virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
> +
> +    /* create the new domain */
> +    domain = hypervDomainDefineXML(conn, xmlDesc);
> +    if (!domain)
> +        return NULL;
> +
> +    /* start the domain */
> +    if (hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED) < 0)
> +        return domain;

The virDomainCreateXML call is for creating so called trasient guests,
which disappear when shutoff. ie there is no persistent config file
present when not running.

If this isn't supported, then don't implement this method. Only implement
the virDomainCreate call, which pairs with virDomainDefineXML.

> +
> +    /* If VIR_DOMAIN_START_PAUSED is set, the guest domain will be started,
> +     * but its vCPUs will be paused. */
> +    if (flags & VIR_DOMAIN_START_PAUSED)
> +        hypervDomainSuspend(domain);

Hmm, racy.

The VIR_DOMAIN_START_PAUSED flag is intended to provide a race free
mechanism. If the caller can tolerate faces, then they can just use
virDomainSuspend directly after starting it.

IOW, if we can't provide race-free behaviour  we should reject use
of VIR_DOMAIN_START_PAUSED.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|