[libvirt] [PATCH 05/20] conf: Add support for shmem role

[Peter Krempa]

On Thu, Sep 15, 2016 at 18:14:30 +0200, Martin Kletzander wrote:
> Role controls how the domain behaves on migration.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  docs/formatdomain.html.in                         | 10 +++++-
>  docs/schemas/domaincommon.rng                     |  8 +++++
>  src/conf/domain_conf.c                            | 39 ++++++++++++++++++++++-
>  src/conf/domain_conf.h                            | 11 +++++++
>  tests/qemuxml2argvdata/qemuxml2argv-shmem.xml     |  4 +--
>  tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml |  4 +--
>  6 files changed, 70 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index f48a4d8b813f..f4d08959c787 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in

[...]

> @@ -6764,6 +6764,14 @@ qemu-kvm -net nic,model=? /dev/null
>      <dd>
>        The <code>shmem</code> element has one mandatory attribute,
>        <code>name</code> to identify the shared memory.
> +      Optional attribute <code>role</code> can be set to values
> +      "master" or "peer".  The former will mean that upon migration,
> +      the data in the shared memory is migrated with the domain.
> +      There should be only one "master" per shared memory object.
> +      Migration with "peer" role is disabled.  If migration of such
> +      domain is required, the shmem device needs to be unplugged
> +      before migration and plugged in at the destination upon
> +      successful migration.

I'm not in favor of adding the workaround suggestion. I'd change it for
a more thoroguh explanation what this configuration does besides
controlling migratability.

>      </dd>
>      <dt><code>size</code></dt>
>      <dd>

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 97cb3de95529..2ccc10515f30 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -12201,6 +12206,23 @@ virDomainShmemDefParseXML(xmlNodePtr node,
> 
>      ctxt->node = node;
> 
> +    if (!(def->name = virXMLPropString(node, "name"))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("shmem element must contain 'name' attribute"));
> +        goto cleanup;
> +    }

This code is already in the function a few lines after this hunk. You
probably forgot to delete it.

> +
> +    tmp = virXMLPropString(node, "role");
> +    if (tmp) {
> +        if ((def->role = virDomainShmemRoleTypeFromString(tmp)) <= 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("Unknown shmem role type '%s'"), tmp);
> +            goto cleanup;
> +        }
> +
> +        VIR_FREE(tmp);
> +    }
> +
>      tmp = virXPathString("string(./model/@type)", ctxt);
>      if (tmp) {
>          /* If there's none, we will automatically have the first one
> @@ -18704,6 +18726,15 @@ virDomainShmemDefCheckABIStability(virDomainShmemDefPtr src,
>          return false;
>      }
> 
> +    if (src->role != dst->role) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target shared memory role '%s' does not match "
> +                         "source role '%s'"),
> +                       virDomainShmemRoleTypeToString(dst->role),
> +                       virDomainShmemRoleTypeToString(src->role));
> +        return false;
> +    }

Is this really guest ABI? Since it's not used in this patch I'll see
later.

> +
>      if (src->model != dst->model) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("Target shared memory model '%s' does not match "
> @@ -21790,8 +21821,14 @@ virDomainShmemDefFormat(virBufferPtr buf,
>                          virDomainShmemDefPtr def,
>                          unsigned int flags)
>  {
> -    virBufferEscapeString(buf, "<shmem name='%s'>\n", def->name);
> +    virBufferEscapeString(buf, "<shmem name='%s'", def->name);
> 
> +    if (def->role) {
> +        virBufferEscapeString(buf, " role='%s'",
> +                              virDomainShmemRoleTypeToString(def->role));
> +    }
> +
> +    virBufferAddLit(buf, ">\n");
>      virBufferAdjustIndent(buf, 2);
> 
>      virBufferAsprintf(buf, "<model type='%s'/>\n",

Peter