[libvirt] [PATCH v2 0/3] Add support for shared memory devices
This is based on Maxime's patches, but leaving out the last one with server starting, the last version was here: https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html There were some points raised, some discussion went on. Basically whatever we agreed is composed in, but feel free to raise questions and further discussions. However it would be really great if this could get into 1.2.9 release. Martin Kletzander (2): docs, conf, schema: add support for shmem device qemu: Build command line for ivshmem device Maxime Leroy (1): qemu: add capability probing for ivshmem device 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_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 118 +++++++++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 15 +- .../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 | 8 + tests/qemuxml2xmltest.c | 1 + 25 files changed, 635 insertions(+), 7 deletions(-) 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 -- 2.1.1
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/docs/formatdomain.html.in b/docs/formatdomain.html.in index bb72452..7bdb1bd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5560,6 +5560,58 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsShmem">Shared memory device</a></h4> + + <p> + A shared memory device allows to share a memory region between + different virtual machines and the host. + <span class="since">Since 1.2.9, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <shmem name='my_shmem0'> + <size unit='M'>4</size> + </shmem> + <shmem name='shmem_server'> + <size unit='M'>2</size> + <server path='/tmp/socket-shmem'/> + <msi vectors='32' ioeventfd='on'/> + </shmem> + </devices> + ... +</pre> + + <dl> + <dt><code>shmem</code></dt> + <dd> + The <code>shmem</code> element has one mandatory attribute, + <code>name</code> to identify the shared memory. + </dd> + <dt><code>size</code></dt> + <dd> + The optional <code>size</code> element specifies the size of the shared + memory. This must be power of 2 and greater than or equal to 1 MiB. + </dd> + <dt><code>server</code></dt> + <dd> + The optional <code>server</code> element can be used to configure a server + socket the device is supposed to connect to. The optional + <code>path</code> attribute specifies the path to the unix socket and + defaults to <code>/var/lib/libvirt/shmem/$shmem_name-sock</code>. + </dd> + <dt><code>msi</code></dt> + <dd> + The optional <code>msi</code> element enables/disables (values "on"/"off", + respectively) MSI interrupts. This option can currently be used only + together with the <code>server</code> element. The <code>vectors</code> + attribute can be used to specify the number of interrupt + vectors. The <code>ioeventd</code> attribute enables/disables (values + "on"/"off", respectively) ioeventfd. + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b6b309d..6864d54 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3214,6 +3214,44 @@ </optional> </element> </define> + + <define name="shmem"> + <element name="shmem"> + <attribute name="name"/> + <interleave> + <optional> + <element name="size"> + <ref name="scaledInteger"/> + </element> + </optional> + <optional> + <element name="server"> + <optional> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + </optional> + </element> + </optional> + <optional> + <element name="msi"> + <optional> + <ref name="ioeventfd"/> + </optional> + <optional> + <attribute name="vectors"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </element> + </optional> + <optional> + <ref name="address"/> + </optional> + </interleave> + </element> + </define> + <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3800,6 +3838,7 @@ <ref name="redirfilter"/> <ref name="rng"/> <ref name="tpm"/> + <ref name="shmem"/> </choice> </zeroOrMore> <optional> 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 @@ -255,7 +255,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "chr", "memballoon", "nvram", - "rng") + "rng", + "shmem") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", @@ -1720,6 +1721,17 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) VIR_FREE(def); } +void virDomainShmemDefFree(virDomainShmemDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->server.path); + VIR_FREE(def->name); + VIR_FREE(def); +} + void virDomainVideoDefFree(virDomainVideoDefPtr def) { if (!def) @@ -1921,6 +1933,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_NVRAM: virDomainNVRAMDefFree(def->data.nvram); break; + case VIR_DOMAIN_DEVICE_SHMEM: + virDomainShmemDefFree(def->data.shmem); + break; case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: break; @@ -2181,6 +2196,10 @@ void virDomainDefFree(virDomainDefPtr def) virDomainRedirFilterDefFree(def->redirfilter); + for (i = 0; i < def->nshmems; i++) + virDomainShmemDefFree(def->shmems[i]); + VIR_FREE(def->shmems); + if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); @@ -2613,6 +2632,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device) return &device->data.memballoon->info; case VIR_DOMAIN_DEVICE_NVRAM: return &device->data.nvram->info; + case VIR_DOMAIN_DEVICE_SHMEM: + return &device->data.shmem->info; case VIR_DOMAIN_DEVICE_RNG: return &device->data.rng->info; @@ -2828,6 +2849,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->hubs[i]->info, opaque) < 0) return -1; } + device.type = VIR_DOMAIN_DEVICE_SHMEM; + for (i = 0; i < def->nshmems; i++) { + device.data.shmem = def->shmems[i]; + if (cb(def, &device, &def->shmems[i]->info, opaque) < 0) + return -1; + } /* Coverity is not very happy with this - all dead_error_condition */ #if !STATIC_ANALYSIS @@ -2856,6 +2883,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: break; @@ -9591,6 +9619,110 @@ virDomainNVRAMDefParseXML(xmlNodePtr node, return NULL; } +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; + } + + 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; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -10447,6 +10579,10 @@ virDomainDeviceDefParse(const char *xmlStr, if (!(dev->data.nvram = virDomainNVRAMDefParseXML(node, flags))) goto error; break; + case VIR_DOMAIN_DEVICE_SHMEM: + if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags))) + goto error; + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; @@ -13575,6 +13711,25 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); } + /* analysis of the shmem devices */ + if ((n = virXPathNodeSet("./devices/shmem", ctxt, &nodes)) < 0) { + goto error; + } + if (n && VIR_ALLOC_N(def->shmems, n) < 0) + goto error; + + node = ctxt->node; + for (i = 0; i < n; i++) { + virDomainShmemDefPtr shmem; + ctxt->node = nodes[i]; + shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags); + if (!shmem) + goto error; + + def->shmems[def->nshmems++] = shmem; + } + ctxt->node = node; + VIR_FREE(nodes); /* analysis of the user namespace mapping */ if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) @@ -17264,6 +17419,54 @@ static int virDomainPanicDefFormat(virBufferPtr buf, } static int +virDomainShmemDefFormat(virBufferPtr buf, + virDomainShmemDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, "<shmem name='%s'", def->name); + + if (!def->size && + !def->server.enabled && + !def->msi.enabled && + !virDomainDeviceInfoIsSet(&def->info, flags)) { + virBufferAddLit(buf, "/>\n"); + return 0; + } else { + virBufferAddLit(buf, ">\n"); + } + + virBufferAdjustIndent(buf, 2); + + if (def->size) + virBufferAsprintf(buf, "<size unit='M'>%llu</size>\n", + VIR_DIV_UP(def->size, 1024 * 1024)); + + if (def->server.enabled) { + virBufferAddLit(buf, "<server"); + virBufferEscapeString(buf, " path='%s'", def->server.path); + virBufferAddLit(buf, "/>\n"); + } + + if (def->msi.enabled) { + virBufferAddLit(buf, "<msi"); + if (def->msi.vectors) + virBufferAsprintf(buf, " vectors='%u'", def->msi.vectors); + if (def->msi.ioeventfd) + virBufferAsprintf(buf, " ioeventfd='%s'", + virTristateSwitchTypeToString(def->msi.ioeventfd)); + virBufferAddLit(buf, "/>\n"); + } + + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</shmem>\n"); + + return 0; +} + +static int virDomainRNGDefFormat(virBufferPtr buf, virDomainRNGDefPtr def, unsigned int flags) @@ -18883,6 +19086,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainPanicDefFormat(buf, def->panic) < 0) goto error; + for (n = 0; n < def->nshmems; n++) + if (virDomainShmemDefFormat(buf, def->shmems[n], flags) < 0) + goto error; + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n"); @@ -20248,6 +20455,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d97b1f8..acb3adf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -136,6 +136,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr; typedef struct _virDomainChrSourceDef virDomainChrSourceDef; typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; +typedef struct _virDomainShmemDef virDomainShmemDef; +typedef virDomainShmemDef *virDomainShmemDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -157,6 +160,7 @@ typedef enum { VIR_DOMAIN_DEVICE_MEMBALLOON, VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, + VIR_DOMAIN_DEVICE_SHMEM, VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; @@ -184,6 +188,7 @@ struct _virDomainDeviceDef { virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; + virDomainShmemDefPtr shmem; } data; }; @@ -1491,6 +1496,21 @@ struct _virDomainNVRAMDef { virDomainDeviceInfo info; }; +struct _virDomainShmemDef { + char *name; + unsigned long long size; + struct { + bool enabled; + char *path; + } server; + struct { + bool enabled; + unsigned vectors; + virTristateSwitch ioeventfd; + } msi; + virDomainDeviceInfo info; +}; + typedef enum { VIR_DOMAIN_SMBIOS_NONE = 0, VIR_DOMAIN_SMBIOS_EMULATE, @@ -2067,6 +2087,9 @@ struct _virDomainDef { size_t nrngs; virDomainRNGDefPtr *rngs; + size_t nshmems; + virDomainShmemDefPtr *shmems; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2264,6 +2287,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); +void virDomainShmemDefFree(virDomainShmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d631887..3a654e4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2859,6 +2859,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("don't know how to remove a %s device"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.xml new file mode 100644 index 0000000..992f8fd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <shmem name='shmem0'> + <size unit='K'>12345</size> + </shmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid.xml new file mode 100644 index 0000000..a5bfb97 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <shmem name='shmem0'> + <server start="yes"/> + </shmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml new file mode 100644 index 0000000..d70279c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <shmem name='shmem0'> + <msi/> + </shmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-small-size.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem-small-size.xml new file mode 100644 index 0000000..8f99b14 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-small-size.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <shmem name='shmem0'> + <size unit='K'>16</size> + </shmem> + </devices> +</domain> 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 @@ -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 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml new file mode 100644 index 0000000..fd79c89 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml @@ -0,0 +1,51 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + <shmem name='shmem0'/> + <shmem name='shmem1'> + <size unit='M'>128</size> + </shmem> + <shmem name='shmem2'> + <size unit='M'>256</size> + </shmem> + <shmem name='shmem3'> + <size unit='M'>512</size> + <server/> + </shmem> + <shmem name='shmem4'> + <size unit='M'>1024</size> + <server path='/tmp/shmem4-sock'/> + </shmem> + <shmem name='shmem5'> + <size unit='M'>2048</size> + <server path='/tmp/shmem5-sock'/> + <msi ioeventfd='off'/> + </shmem> + <shmem name='shmem6'> + <size unit='M'>4096</size> + <server path='/tmp/shmem6-sock'/> + <msi vectors='16'/> + </shmem> + <shmem name='shmem7'> + <size unit='M'>8192</size> + <server path='/tmp/shmem7-sock'/> + <msi vectors='32' ioeventfd='on'/> + </shmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ab923d0..60c4405 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1462,6 +1462,11 @@ mymain(void) DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS); + DO_TEST_PARSE_ERROR("shmem-invalid", NONE); + DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); + DO_TEST_PARSE_ERROR("shmem-invalid-size", NONE); + DO_TEST_PARSE_ERROR("shmem-small-size", NONE); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 56a371e..c63f14f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -403,6 +403,7 @@ mymain(void) DO_TEST("bios-nvram"); DO_TEST("tap-vhost"); + DO_TEST("shmem"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.1.1
Hi Martin, I am going to test it on my platform before to review/ack it. Maxime
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
Hi, On Thu, Sep 25, 2014 at 03:37:46PM +0200, Michal Privoznik wrote:
On 25.09.2014 11:45, Martin Kletzander wrote: [...]
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?
It's ivshmem... The problem is that ivshmem predominantly thinks that any size argument without a suffix like 'g/G' or 'm/M' is in megabytes. I believe this is nice, but it would be nicer to have a 'k/K' suffix so that we could tell ivshmem that we want the size in kilobytes... Maybe even a 'b' suffix for bytes. Do you guys see any value in this? If yes, I will cook a patch ASAP, so we might as well remove this check from the QEMU parts... But yes, this check is definitely not needed here. Thanks, Levente.
On Thu, Sep 25, 2014 at 04:21:14PM +0200, Levente Kurusa wrote:
Hi,
On Thu, Sep 25, 2014 at 03:37:46PM +0200, Michal Privoznik wrote:
On 25.09.2014 11:45, Martin Kletzander wrote: [...]
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?
I wanted to make it more future-proof. That is that this way we can start storing the size in MB any time we want and we can lax the parsing whenever wanted. If you want, I can just store the value in MiB for now or remove the check (but not both, of course). I'd be in favour of the first thing.
It's ivshmem... The problem is that ivshmem predominantly thinks that any size argument without a suffix like 'g/G' or 'm/M' is in megabytes. I believe this is nice, but it would be nicer to have a 'k/K' suffix so that we could tell ivshmem that we want the size in kilobytes... Maybe even a 'b' suffix for bytes.
That should be working after re-factoring done by David Marchand if I'm correct, but it's not yet available.
Do you guys see any value in this? If yes, I will cook a patch ASAP, so we might as well remove this check from the QEMU parts...
But yes, this check is definitely not needed here.
It would be nice if you could help move the QEMU parts a little bit. I haven't heard about any status since we discussed some ivshmem server details with David. Martin
On 25.09.2014 16:35, Martin Kletzander wrote:
On Thu, Sep 25, 2014 at 04:21:14PM +0200, Levente Kurusa wrote:
Hi,
On Thu, Sep 25, 2014 at 03:37:46PM +0200, Michal Privoznik wrote:
On 25.09.2014 11:45, Martin Kletzander wrote: [...]
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?
I wanted to make it more future-proof. That is that this way we can start storing the size in MB any time we want and we can lax the parsing whenever wanted.
If you want, I can just store the value in MiB for now or remove the check (but not both, of course). I'd be in favour of the first thing.
Yeah, so just drop the check here and leave it in src/qemu/. And store the value in bytes. That's okay - I was just curios. Michal
On Thu, Sep 25, 2014 at 04:49:04PM +0200, Michal Privoznik wrote:
On 25.09.2014 16:35, Martin Kletzander wrote:
On Thu, Sep 25, 2014 at 04:21:14PM +0200, Levente Kurusa wrote:
Hi,
On Thu, Sep 25, 2014 at 03:37:46PM +0200, Michal Privoznik wrote:
On 25.09.2014 11:45, Martin Kletzander wrote: [...]
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?
I wanted to make it more future-proof. That is that this way we can start storing the size in MB any time we want and we can lax the parsing whenever wanted.
If you want, I can just store the value in MiB for now or remove the check (but not both, of course). I'd be in favour of the first thing.
Yeah, so just drop the check here and leave it in src/qemu/. And store the value in bytes. That's okay - I was just curios.
Although I was in favour of the former, I'll take your approach. I'll wait if Levente has anything to add before pushing this. Martin
On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan@redhat.com> 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>
I would prefer: <shmem name='shmem0'> <server path='/tmp/socket-ivshmem0'> <msi vectors='32' ioeventfd='on'/> </server> <size unit='M'>32</size> </shmem> [..]
+ if ((server = virXPathNode("./server", ctxt))) { + def->server.enabled = true; + + if ((tmp = virXMLPropString(server, "path"))) + def->server.path = virFileSanitizePath(tmp); + VIR_FREE(tmp); +
The server path is mandatory if the ivshmem server is not started by libvirt. If the user needs to start manually the ivshmem server, having a default path doesn't make sense anymore.
+ /* + * 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"));
If an autostart ivshmem server is implemented, as we agree to implement for the V2 (see http://www.redhat.com/archives/libvir-list/2014-September/msg00124.html), there is no need for a "start" attribute in the server node. Why this 'dead' code ?
+ 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;
If the msi node is inside the server node, i.e: <server path='/tmp/socket-ivshmem0'> <msi vectors='32' ioeventfd='on'/> </server> Then, this test can be removed. Maxime
On Fri, Sep 26, 2014 at 10:32:08AM +0200, Maxime Leroy wrote:
On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan@redhat.com> 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>
I would prefer:
<shmem name='shmem0'> <server path='/tmp/socket-ivshmem0'> <msi vectors='32' ioeventfd='on'/> </server> <size unit='M'>32</size> </shmem>
Take that discussed generality into account. What if there's another implementation that we'll want to cover with the smem device (be it qemu or another hypervisor) that allows MSI without any server configuration. It won't make sense then.
[..]
+ if ((server = virXPathNode("./server", ctxt))) { + def->server.enabled = true; + + if ((tmp = virXMLPropString(server, "path"))) + def->server.path = virFileSanitizePath(tmp); + VIR_FREE(tmp); +
The server path is mandatory if the ivshmem server is not started by libvirt.
If the user needs to start manually the ivshmem server, having a default path doesn't make sense anymore.
It is not. Users can start the server themselves with the listening socket residing in /var/lib/libvirt/shmem-<name>-sock and it will just work. I'm trying to keep that possibility available for the users.
+ /* + * 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"));
If an autostart ivshmem server is implemented, as we agree to implement for the V2 (see http://www.redhat.com/archives/libvir-list/2014-September/msg00124.html), there is no need for a "start" attribute in the server node.
Why this 'dead' code ?
Simply, because I forgot about that, thanks for catching that, I'll remove it ;) Martin
From: Maxime Leroy <maxime.leroy@6wind.com> Ivshmem is supported by QEMU since 0.13 release. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 15 ++++++++++----- 10 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 49f5f75..6a905df 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -270,6 +270,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "splash-timeout", /* 175 */ "iothread", "migrate-rdma", + "ivshmem", ); @@ -1500,6 +1501,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "memory-backend-file", QEMU_CAPS_OBJECT_MEMORY_FILE }, { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO }, { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, + { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a0bb5d3..0214f30 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -217,6 +217,7 @@ typedef enum { QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */ QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */ QEMU_CAPS_MIGRATE_RDMA = 177, /* have rdma migration */ + QEMU_CAPS_DEVICE_IVSHMEM = 178, /* -device ivshmem */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 3b1b699..fc8dfc1 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -115,4 +115,5 @@ <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> <flag name='usb-audio'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 4b7651e..f4f0397 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -129,4 +129,5 @@ <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> <flag name='usb-audio'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index d146bf9..e6659e4 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -130,4 +130,5 @@ <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> <flag name='usb-audio'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index c220b46..9716cf5 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -139,4 +139,5 @@ <flag name='host-pci-multidomain'/> <flag name='usb-audio'/> <flag name='splash-timeout'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 21d4320..a820cd4 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -145,4 +145,5 @@ <flag name='msg-timestamp'/> <flag name='usb-audio'/> <flag name='splash-timeout'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index 44f7b0c..f65b3f4 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -145,4 +145,5 @@ <flag name='numa'/> <flag name='usb-audio'/> <flag name='splash-timeout'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 71d2459..7fc654d 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -159,4 +159,5 @@ <flag name='splash-timeout'/> <flag name='iothread'/> <flag name='migrate-rdma'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 975edf3..7e62a50 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -518,7 +518,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_DEVICE_USB_STORAGE, - QEMU_CAPS_HOST_PCI_MULTIDOMAIN); + QEMU_CAPS_HOST_PCI_MULTIDOMAIN, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -746,7 +747,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_DEVICE_USB_STORAGE, - QEMU_CAPS_SPLASH_TIMEOUT); + QEMU_CAPS_SPLASH_TIMEOUT, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -845,7 +847,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_OBJECT_USB_AUDIO, - QEMU_CAPS_SPLASH_TIMEOUT); + QEMU_CAPS_SPLASH_TIMEOUT, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-1.2.0", 1002000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -956,7 +959,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_USB_STORAGE_REMOVABLE, QEMU_CAPS_OBJECT_USB_AUDIO, - QEMU_CAPS_SPLASH_TIMEOUT); + QEMU_CAPS_SPLASH_TIMEOUT, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1072,7 +1076,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_USB_STORAGE_REMOVABLE, QEMU_CAPS_OBJECT_USB_AUDIO, - QEMU_CAPS_SPLASH_TIMEOUT); + QEMU_CAPS_SPLASH_TIMEOUT, + QEMU_CAPS_DEVICE_IVSHMEM); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.1.1
This patch implements support for the ivshmem device in QEMU. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 1 + tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 65864d2..2f26bcd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1029,6 +1029,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) return -1; } + for (i = 0; i < def->nshmems; i++) { + if (virAsprintf(&def->shmems[i]->info.alias, "shmem%zu", i) < 0) + return -1; + } for (i = 0; i < def->nsmartcards; i++) { if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0) return -1; @@ -7452,6 +7456,114 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } static int +qemuBuildShmemDevCmd(virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem device is not supported " + "with this QEMU binary")); + goto error; + } + + virBufferAddLit(&buf, "ivshmem"); + if (shmem->size) { + /* + * Thanks to our parsing code, we have a guarantee that the + * size is power of two and is at least a mebibyte in size. + * But because it may change inthe future, the checks are + * doubled in here. + */ + if (shmem->size & (shmem->size - 1)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem size must be a power of two")); + goto error; + } + if (shmem->size < 1024 * 1024) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem size must be at least 1 MiB")); + goto error; + } + virBufferAsprintf(&buf, ",size=%llum", + VIR_DIV_UP(shmem->size, 1024 * 1024)); + } + + if (!shmem->server.enabled) { + virBufferAsprintf(&buf, ",shm=%s", shmem->name); + } else { + virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); + if (shmem->msi.enabled) { + virBufferAddLit(&buf, ",msi=on"); + if (shmem->msi.vectors) + virBufferAsprintf(&buf, ",vectors=%u", shmem->msi.vectors); + if (shmem->msi.ioeventfd) + virBufferAsprintf(&buf, ",ioeventfd=%s", + virTristateSwitchTypeToString(shmem->msi.ioeventfd)); + } + } + + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) + goto error; + + if (virBufferCheckError(&buf) < 0) + goto error; + + virCommandAddArg(cmd, "-device"); + virCommandAddArgBuffer(cmd, &buf); + + return 0; + + error: + virBufferFreeAndReset(&buf); + return -1; +} + +static int +qemuBuildShmemCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + return -1; + + if (shmem->server.enabled) { + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->info.alias) < 0) + return -1; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + if (!devstr) + return -1; + + virCommandAddArg(cmd, "-chardev"); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + + return 0; +} + +static int qemuBuildChrDeviceCommandLine(virCommandPtr cmd, virDomainDefPtr def, virDomainChrDefPtr chr, @@ -9646,6 +9758,11 @@ qemuBuildCommandLine(virConnectPtr conn, } } + for (i = 0; i < def->nshmems; i++) { + if (qemuBuildShmemCommandLine(cmd, def, def->shmems[i], qemuCaps)) + goto error; + } + if (mlock) { unsigned long long memKB; @@ -9853,7 +9970,6 @@ qemuBuildChrDeviceStr(char **deviceStr, return ret; } - /* * This method takes a string representing a QEMU command line ARGV set * optionally prefixed by a list of environment variables. It then tries diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index aa40c9e..5083223 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -131,6 +131,7 @@ char *qemuBuildDriveDevStr(virDomainDefPtr def, char *qemuBuildFSDevStr(virDomainDefPtr domainDef, virDomainFSDefPtr fs, virQEMUCapsPtr qemuCaps); + /* Current, best practice */ char *qemuBuildControllerDevStr(virDomainDefPtr domainDef, virDomainControllerDefPtr def, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 60c4405..e58717b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1462,6 +1462,9 @@ mymain(void) DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS); + DO_TEST("shmem", QEMU_CAPS_PCIDEVICE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); + DO_TEST_FAILURE("shmem", NONE); DO_TEST_PARSE_ERROR("shmem-invalid", NONE); DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); DO_TEST_PARSE_ERROR("shmem-invalid-size", NONE); -- 2.1.1
On 25.09.2014 11:45, Martin Kletzander wrote:
This patch implements support for the ivshmem device in QEMU.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 1 + tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 121 insertions(+), 1 deletion(-)
This is the correct place to introduce .args. It's okay to introduce .xml in the patch that adds XML parsing/formatting as the .xml can be tested straight away. Michal
On Thu, Sep 25, 2014 at 03:37:42PM +0200, Michal Privoznik wrote:
On 25.09.2014 11:45, Martin Kletzander wrote:
This patch implements support for the ivshmem device in QEMU.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 1 + tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 121 insertions(+), 1 deletion(-)
This is the correct place to introduce .args. It's okay to introduce .xml in the patch that adds XML parsing/formatting as the .xml can be tested straight away.
I know, I probably used '-a' for 'git commit' when creating a fixup for 1/3 and that pulled the .args changes in. I'll push that file as a part of this commit of course. Thanks for he review, Martin
On 25.09.2014 11:45, Martin Kletzander wrote:
This is based on Maxime's patches, but leaving out the last one with server starting, the last version was here:
https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html
There were some points raised, some discussion went on. Basically whatever we agreed is composed in, but feel free to raise questions and further discussions. However it would be really great if this could get into 1.2.9 release.
Martin Kletzander (2): docs, conf, schema: add support for shmem device qemu: Build command line for ivshmem device
Maxime Leroy (1): qemu: add capability probing for ivshmem device
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_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 118 +++++++++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 15 +- .../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 | 8 + tests/qemuxml2xmltest.c | 1 + 25 files changed, 635 insertions(+), 7 deletions(-) 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
ACK series, but see my inline comments. Michal
Hi Martin, On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
This is based on Maxime's patches, but leaving out the last one with server starting, the last version was here:
https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html
As my original patches have been modified, I want to test it on my plateform to check there is no regression. Nack until that. So, please, wait for this serie. Maxime
On Fri, Sep 26, 2014 at 10:35:09AM +0200, Maxime Leroy wrote:
Hi Martin,
On Thu, Sep 25, 2014 at 11:45 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
This is based on Maxime's patches, but leaving out the last one with server starting, the last version was here:
https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html
As my original patches have been modified, I want to test it on my plateform to check there is no regression.
Nack until that.
So, please, wait for this serie.
Please test it with the v3 I just posted here: https://www.redhat.com/archives/libvir-list/2014-September/msg01617.html Martin
participants (4)
-
Levente Kurusa -
Martin Kletzander -
Maxime Leroy -
Michal Privoznik