[libvirt] [PATCH v3 0/3] Add support for shared memory devices
v3: - removed the dead code for server start attribute - fixed default path in documentation - removed unnecessary check for shmem size v2 is here: https://www.redhat.com/archives/libvir-list/2014-September/msg01519.html v1 (Maxime's) is here: https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html 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 | 198 ++++++++++++++++++++- 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-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 | 7 + tests/qemuxml2xmltest.c | 1 + 24 files changed, 598 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.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 | 184 ++++++++++++++++++++- src/conf/domain_conf.h | 24 +++ src/qemu/qemu_hotplug.c | 1 + .../qemuxml2argv-shmem-msi-only.xml | 24 +++ tests/qemuxml2argvdata/qemuxml2argv-shmem.xml | 51 ++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 9 files changed, 377 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e7b585c..8e50b8c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5582,6 +5582,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 1a266e5..6b69fd1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3278,6 +3278,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"> @@ -3864,6 +3902,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 b114737..51bdd31 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; @@ -9702,6 +9730,84 @@ 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); + } + + 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 (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, @@ -10558,6 +10664,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; @@ -13686,6 +13796,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) @@ -17470,6 +17599,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) @@ -19089,6 +19266,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"); @@ -20454,6 +20635,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 ea201b3..d73d654 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; }; @@ -1506,6 +1511,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, @@ -2082,6 +2102,9 @@ struct _virDomainDef { size_t nrngs; virDomainRNGDefPtr *rngs; + size_t nshmems; + virDomainShmemDefPtr *shmems; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2279,6 +2302,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-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.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 4c85bfe..e55dc50 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1464,6 +1464,8 @@ mymain(void) DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS); + DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 9a56880..978e5f1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -404,6 +404,7 @@ mymain(void) DO_TEST("bios-nvram"); DO_TEST("tap-vhost"); + DO_TEST("shmem"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.1.1
On 26.09.2014 12:43, 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 | 184 ++++++++++++++++++++- src/conf/domain_conf.h | 24 +++ src/qemu/qemu_hotplug.c | 1 + .../qemuxml2argv-shmem-msi-only.xml | 24 +++ tests/qemuxml2argvdata/qemuxml2argv-shmem.xml | 51 ++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 9 files changed, 377 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b114737..51bdd31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
@@ -9702,6 +9730,84 @@ 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))) {
s,./server,./server[1] to make sure we parse only the first <server/> in case user supplies two or more in the XML.
+ def->server.enabled = true; + + if ((tmp = virXMLPropString(server, "path"))) + def->server.path = virFileSanitizePath(tmp); + VIR_FREE(tmp); + } + + if ((msi = virXPathNode("./msi", ctxt))) {
Same here.
+ 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 (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto cleanup; + + + ret = def; + def = NULL; + cleanup: + ctxt->node = save; + VIR_FREE(tmp); + virDomainShmemDefFree(def); + return ret; +} +
Michal
On Thu, Oct 02, 2014 at 09:42:36AM +0200, Michal Privoznik wrote:
On 26.09.2014 12:43, Martin Kletzander wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b114737..51bdd31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
@@ -9702,6 +9730,84 @@ 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))) {
s,./server,./server[1] to make sure we parse only the first <server/> in case user supplies two or more in the XML.
+ def->server.enabled = true; + + if ((tmp = virXMLPropString(server, "path"))) + def->server.path = virFileSanitizePath(tmp); + VIR_FREE(tmp); + } + + if ((msi = virXPathNode("./msi", ctxt))) {
Same here.
Nice catch. Since we are past the release anyway, I'm going to push this in a while and whoever wants (e.g. Maxime) have the whole release cycle to test this. Thank you, Martin
Hi Martin, On Fri, Oct 3, 2014 at 10:45 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Oct 02, 2014 at 09:42:36AM +0200, Michal Privoznik wrote:
On 26.09.2014 12:43, Martin Kletzander wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b114737..51bdd31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[..]
Nice catch.
Since we are past the release anyway, I'm going to push this in a while and whoever wants (e.g. Maxime) have the whole release cycle to test this.
Thanks for pushing/cleaning the shmem patches support (i.e http://www.redhat.com/archives/libvir-list/2014-August/msg01032.html) But you did miss the following: 1. the ivshmem server autostart feature: you wanted me to develop it into the v1. (i.e. http://www.redhat.com/archives/libvir-list/2014-August/msg01432.html) 2. You did not wait for my tests. Why should I ask you to wait for my tests? Lucky enough, now I have the results of the tests, everything works fine ;) I don't understand why it became so urgent to push these patches. Anyway, I am glad that libvirt supports ivshmem. Since now you pushed these patches, do you plan to provide the ivshmem autostart feature like you requested previously ? I'll be glad to review it and to provide feedbacks based on my tests. Maxime
On 08.10.2014 18:57, Maxime Leroy wrote:
Hi Martin,
On Fri, Oct 3, 2014 at 10:45 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Oct 02, 2014 at 09:42:36AM +0200, Michal Privoznik wrote:
On 26.09.2014 12:43, Martin Kletzander wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b114737..51bdd31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[..]
Nice catch.
Since we are past the release anyway, I'm going to push this in a while and whoever wants (e.g. Maxime) have the whole release cycle to test this.
Thanks for pushing/cleaning the shmem patches support (i.e http://www.redhat.com/archives/libvir-list/2014-August/msg01032.html)
But you did miss the following:
1. the ivshmem server autostart feature: you wanted me to develop it into the v1. (i.e. http://www.redhat.com/archives/libvir-list/2014-August/msg01432.html)
2. You did not wait for my tests. Why should I ask you to wait for my tests? Lucky enough, now I have the results of the tests, everything works fine ;)
I don't understand why it became so urgent to push these patches.
Well, I'm not intending to speak on Martin's behalf, but my workflow is as follows: when developing a new code I create a local branch and put the commits there. And as the development goes on, I keep the branch rebased onto the current master. However, depending on the complexity of the code, rebase conflicts are likely to occur - and that's rather unpleasant. So once the patches are ready I'm pushing them nearly ASAP for two reasons: to get rid of the rebasing (others may be working in the same area in near future too, so I'd spare them having rebase conflicts), and to test the feature. As soon as an pubclic API is not released, there's still an possibility to revert the patches if they turn out to be rubbish. So I'd say you both (you and Martin) obeyed the process. You've tested an upstream code. If it turned out that there's something wrong we could just fix it. Michal
On Wed, Oct 08, 2014 at 06:57:45PM +0200, Maxime Leroy wrote:
Hi Martin,
On Fri, Oct 3, 2014 at 10:45 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Oct 02, 2014 at 09:42:36AM +0200, Michal Privoznik wrote:
On 26.09.2014 12:43, Martin Kletzander wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b114737..51bdd31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[..]
Nice catch.
Since we are past the release anyway, I'm going to push this in a while and whoever wants (e.g. Maxime) have the whole release cycle to test this.
Thanks for pushing/cleaning the shmem patches support (i.e http://www.redhat.com/archives/libvir-list/2014-August/msg01032.html)
But you did miss the following:
1. the ivshmem server autostart feature: you wanted me to develop it into the v1. (i.e. http://www.redhat.com/archives/libvir-list/2014-August/msg01432.html)
Yes, I know. That was the idea that when the patches are split logically, it's possible to push one part without waiting for other one.
2. You did not wait for my tests. Why should I ask you to wait for my tests? Lucky enough, now I have the results of the tests, everything works fine ;)
I don't understand why it became so urgent to push these patches.
I'm sorry if I disappointed you somehow. At first I wanted to make it for the 1.2.9 release, so we might get some upstream and usage from libvirt users. Even though I didn't make it, I already had many reviews and it was easy to fix what people requested. And because I had some ACKs and it was after release, there's a whole release cycle to try out for everyone else if they want (unfortunately they can't try that with the official release, but have to go with git). If you doubt that, it already worked. Thanks to pushing it, Eric found some typos, Peter found out that there's no ABI stability check in qemu driver and that's just a start.
Anyway, I am glad that libvirt supports ivshmem.
Since now you pushed these patches, do you plan to provide the ivshmem autostart feature like you requested previously ?
I'll be glad to review it and to provide feedbacks based on my tests.
I did not plan to, so feel free to continue with that, I'm focusing on different things now. But if it's updated in qemu and there's no progress, I might look into that because it would be nice to have libvirt supporting the server as well. Have a nice day, Martin
On 09/26/2014 04:43 AM, 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.
+++ b/docs/formatdomain.html.in @@ -5582,6 +5582,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>
You missed 1.2.9. Can you do a quick followup to document this as 1.2.10? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 09/26/2014 04:43 AM, 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.
static int +virDomainShmemDefFormat(virBufferPtr buf, + virDomainShmemDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, "<shmem name='%s'", def->name); +
Need to use virBufferEscape, to protect against the user naming their memory something that conflicts with XML.
+ + if (def->size) + virBufferAsprintf(buf, "<size unit='M'>%llu</size>\n", + VIR_DIV_UP(def->size, 1024 * 1024));
I thought the docs said this already had to be a multiple of a megabyte. If that's true, then it might be simpler to just report 'def->size >> 20', rather than having me scratch my head at why we need to round the result to the next higher boundary. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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 + .../qemuxml2argv-shmem-invalid-size.xml | 24 +++++ .../qemuxml2argv-shmem-small-size.xml | 24 +++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 +++ tests/qemuxml2argvtest.c | 5 + 6 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-small-size.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb72451..8a96a0e 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; @@ -7496,6 +7500,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->name) < 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, @@ -9690,6 +9802,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; @@ -9897,7 +10014,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/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-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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e55dc50..af9ed4c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1464,6 +1464,11 @@ 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_FAILURE("shmem-invalid-size", NONE); + DO_TEST_FAILURE("shmem-small-size", NONE); DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); virObjectUnref(driver.config); -- 2.1.1
Hi, On Fri, Sep 26, 2014 at 12:43:08PM +0200, 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> ---
-
Nitpicks shall follow! :) What's this?
/* * 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); +
And this whole hunk? Thanks, Levente.
On Fri, Sep 26, 2014 at 02:47:48PM +0200, Levente Kurusa wrote:
Hi,
On Fri, Sep 26, 2014 at 12:43:08PM +0200, 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> ---
-
Nitpicks shall follow! :)
What's this?
/* * 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); +
And this whole hunk?
Good catches, both removed, thanks ;) Martin
On 09/26/2014 04:43 AM, 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> ---
+ 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
s/inthe/in the/
+ * 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));
Similar comment as before; since you already validated sizing and minimum value, you could use simpler >>20 instead of making me guess whether rounding up is occurring.
@@ -9897,7 +10014,6 @@ qemuBuildChrDeviceStr(char **deviceStr, return ret; }
- /*
Spurious line deletion; you might as well restore the two lines between functions when doing the other touchups I've pointed out. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Oct 03, 2014 at 03:36:19PM -0600, Eric Blake wrote:
On 09/26/2014 04:43 AM, 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> ---
+ 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
s/inthe/in the/
+ * 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));
Similar comment as before; since you already validated sizing and minimum value, you could use simpler >>20 instead of making me guess whether rounding up is occurring.
@@ -9897,7 +10014,6 @@ qemuBuildChrDeviceStr(char **deviceStr, return ret; }
- /*
Spurious line deletion; you might as well restore the two lines between functions when doing the other touchups I've pointed out.
This wasn't pushed, I fixed it thanks to Levente's comment. And in this file the spaces are unequal, sometimes there are two lines, sometimes one, so this would be a bigger follow-up unrelated to this series. All the other things are in a commit I'll push in a while. Thanks for the check on these. Martin
Hi, thanks for the work on this, looks great! You can add my: Reviewed-by: Levente Kurusa <levex@linux.com> Thanks, Levente.
On 26.09.2014 12:43, Martin Kletzander wrote:
v3: - removed the dead code for server start attribute - fixed default path in documentation - removed unnecessary check for shmem size
v2 is here: https://www.redhat.com/archives/libvir-list/2014-September/msg01519.html
v1 (Maxime's) is here: https://www.redhat.com/archives/libvir-list/2014-August/msg01032.html
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 | 198 ++++++++++++++++++++- 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-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 | 7 + tests/qemuxml2xmltest.c | 1 + 24 files changed, 598 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-size.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
ACK series Michal
participants (5)
-
Eric Blake -
Levente Kurusa -
Martin Kletzander -
Maxime Leroy -
Michal Privoznik