[libvirt] [PATCH v2 1/3] docs, conf, schema: add support for shmem device

[Michal Privoznik]

On 25.09.2014 11:45, Martin Kletzander wrote:
> This patch adds parsing/formatting code as well as documentation for
> shared memory devices.  This will currently be only accessible in QEMU
> using it's ivshmem device, but is designed as generic as possible to
> allow future expansion for other hypervisors.
>
> In the devices section in the domain XML users may specify:
>
> - For shmem device using a server:
>
>   <shmem name='shmem0'>
>     <server path='/tmp/socket-ivshmem0'/>
>     <size unit='M'>32</size>
>     <msi vectors='32' ioeventfd='on'/>
>   </shmem>
>
> - For ivshmem device not using an ivshmem server:
>
>   <shmem name='shmem1'>
>     <size unit='M'>32</size>
>   </shmem>
>
> Most of the configuration is made optional so it also allows
> specifications like:
>
>   <shmem name='shmem1/>
>   <shmem name='shmem2'>
>     <server/>
>   </shmem>
>
> Signed-off-by: Maxime Leroy <maxime.leroy at 6wind.com>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>   docs/formatdomain.html.in                          |  52 +++++
>   docs/schemas/domaincommon.rng                      |  39 ++++
>   src/conf/domain_conf.c                             | 210 ++++++++++++++++++++-
>   src/conf/domain_conf.h                             |  24 +++
>   src/qemu/qemu_hotplug.c                            |   1 +
>   .../qemuxml2argv-shmem-invalid-size.xml            |  24 +++
>   .../qemuxml2argv-shmem-invalid.xml                 |  24 +++
>   .../qemuxml2argv-shmem-msi-only.xml                |  24 +++
>   .../qemuxml2argv-shmem-small-size.xml              |  24 +++
>   tests/qemuxml2argvdata/qemuxml2argv-shmem.args     |  16 ++
>   tests/qemuxml2argvdata/qemuxml2argv-shmem.xml      |  51 +++++
>   tests/qemuxml2argvtest.c                           |   5 +
>   tests/qemuxml2xmltest.c                            |   1 +
>   13 files changed, 494 insertions(+), 1 deletion(-)
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.xml
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid.xml
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-small-size.xml
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem.args
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml
>

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0a7d0b8..ab615fb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

> +static virDomainShmemDefPtr
> +virDomainShmemDefParseXML(xmlNodePtr node,
> +                          xmlXPathContextPtr ctxt,
> +                          unsigned int flags)
> +{
> +    char *tmp = NULL;
> +    virDomainShmemDefPtr def = NULL;
> +    virDomainShmemDefPtr ret = NULL;
> +    xmlNodePtr msi = NULL;
> +    xmlNodePtr save = ctxt->node;
> +    xmlNodePtr server = NULL;
> +
> +
> +    if (VIR_ALLOC(def) < 0)
> +        return NULL;
> +
> +    ctxt->node = node;
> +
> +    if (!(def->name = virXMLPropString(node, "name"))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("shmem element must contain 'name' attribute"));
> +        goto cleanup;
> +    }
> +
> +    if (virDomainParseScaledValue("./size[1]", ctxt, &def->size,
> +                                    1, ULLONG_MAX, false) < 0)
> +        goto cleanup;
> +
> +    if ((server = virXPathNode("./server", ctxt))) {
> +        def->server.enabled = true;
> +
> +        if ((tmp = virXMLPropString(server, "path")))
> +            def->server.path = virFileSanitizePath(tmp);
> +        VIR_FREE(tmp);
> +
> +        /*
> +         * We can always safely remove this check, but until the
> +         * server starting part is in it is better to directly disable
> +         * it rather then just ignoring it.
> +         */
> +        if ((tmp = virXMLPropString(server, "start"))) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("Unsupported start attribute for "
> +                             "shmem server element"));
> +            goto cleanup;
> +        }
> +    }
> +
> +    if ((msi = virXPathNode("./msi", ctxt))) {
> +        def->msi.enabled = true;
> +
> +        if ((tmp = virXMLPropString(msi, "vectors")) &&
> +            virStrToLong_uip(tmp, NULL, 0, &def->msi.vectors) < 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("invalid number of vectors for shmem: '%s'"),
> +                           tmp);
> +            goto cleanup;
> +        }
> +        VIR_FREE(tmp);
> +
> +        if ((tmp = virXMLPropString(msi, "ioeventfd")) &&
> +            (def->msi.ioeventfd = virTristateSwitchTypeFromString(tmp)) <= 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("invalid msi ioeventfd setting for shmem: '%s'"),
> +                           tmp);
> +            goto cleanup;
> +        }
> +        VIR_FREE(tmp);
> +    }
> +
> +    /* msi option is only relevant with a server */
> +    if (def->msi.enabled && !def->server.enabled) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("msi option is only supported with a server"));
> +        goto cleanup;
> +    }
> +
> +    /* size should be a power of two */
> +    if (def->size && def->size & (def->size - 1)) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("shmem size must be a power of two"));
> +        goto cleanup;
> +    }
> +    /* and greater than 1MiB (for now); the formatting code depends on
> +     * it and must be adjusted in case this condition changes */
> +    if (def->size && def->size < 1024 * 1024) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("shmem size must be at least 1 MiB"));
> +        goto cleanup;
> +    }

Where do these restrictions come from? If they're result of qemu 
implementation, than they should be checked in 3/3. If other HV learned 
shmem these limitations may not apply to it. Or is it a kernel thing 
that only areas with 1MB granularity can be mapped? Moreover, if such 
granularity is required, does it makes sense to store the size in bytes?

> +
> +    if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> +        goto cleanup;
> +
> +
> +    ret = def;
> +    def = NULL;
> + cleanup:
> +    ctxt->node = save;
> +    VIR_FREE(tmp);
> +    virDomainShmemDefFree(def);
> +    return ret;
> +}
> +

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
> new file mode 100644
> index 0000000..a3d3cc8
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args

This file doesn't belong here yet. Any .args should be introduced with 
the qemu_driver implementation.

> @@ -0,0 +1,16 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \
> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
> +-device ivshmem,shm=shmem0 \
> +-device ivshmem,size=128m,shm=shmem1 \
> +-device ivshmem,size=256m,shm=shmem2 \
> +-device ivshmem,size=512m,chardev=charshmem3 \
> +-chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \
> +-device ivshmem,size=1024m,chardev=charshmem4 \
> +-chardev socket,id=charshmem4,path=/tmp/shmem4-sock \
> +-device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off \
> +-chardev socket,id=charshmem5,path=/tmp/shmem5-sock \
> +-device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16 \
> +-chardev socket,id=charshmem6,path=/tmp/shmem6-sock \
> +-device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on \
> +-chardev socket,id=charshmem7,path=/tmp/shmem7-sock

Michal