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

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



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 6wind com>
Signed-off-by: Martin Kletzander <mkletzan 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


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