[libvirt] [PATCH 0/4] ivshmem support
Shawn Furrow proposed a patch more than a month ago: https://www.redhat.com/archives/libvir-list/2012-September/msg01612.html But this is a complete different implementation. Considering there could be other memory related devices in futuer, this introduces a new device model, called "memory device", instead of a specific device like "ivshmem", though only "ivshmem" is supported currently. Please refer to PATCH 1/4 for more details. CC'ed to Cam and Shawn, to see if there is advise on the documents. Osier Yang (4): docs: Add documents for memory device conf: Parse and format memory device XML qemu: Add cap flag QEMU_CAPS_IVSHMEM qemu: Build command line for ivshmem device docs/formatdomain.html.in | 39 +++++ docs/schemas/domaincommon.rng | 38 +++++ src/conf/domain_conf.c | 184 +++++++++++++++++++++- src/conf/domain_conf.h | 27 +++ src/libvirt_private.syms | 3 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 85 ++++++++++ src/util/util.c | 5 + src/util/util.h | 2 + tests/qemuhelptest.c | 12 +- tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 7 + tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 33 ++++ tests/qemuxml2argvtest.c | 2 + 14 files changed, 435 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml -- 1.7.7.6 Regards, Osier
Considering there could be other memory related devices in future, this introduces a general device model, called memory device, instead of a specific device like "ivshmem", currently only "ivshmem" is supported though. An example of the XML: <memory model='ivshmem'> <source id='nahanni' path='/tmp/nahanni'/> <size unit='M'>512</size> <vectors>8</vectors> </ioeventfd> </memory> "source id" is required, as QEMU uses it to indentify the shared memory device. If "path" for "ivshmem" memory device is specified, it's going to be setup with interrupts enabled. Otherwise interrupts is disabled. To be more clear, let's see the QEMU command line of both cases: * With interrupts (/tmp/nahanni is the ivshmem server socket path) /* This starts the ivshmem server */ % ivshmem_server -m 512 -p /tmp/foo /* qemu command line */ -chardev socket,path=/tmp/foo,id=nahanni -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on * Without interrupts (no ivshmem server is needed) -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on Not all ivshmem properties are supported by this patch set, but let's support the basic ones first. --- There is nearly no documents for ivshmem, so there might be some places which are not proper (actually I'm not comfortable with it myself), welcomed to any advise. --- docs/formatdomain.html.in | 39 +++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cc9e871..7f5efce 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4098,6 +4098,45 @@ qemu-kvm -net nic,model=? /dev/null <code>model='none'</code> may be used. </p> + <h4><a name="elementsMemory">Memory devices</a></h4> + + <p> + Element <code>memory</code> is used to specify memory related + device. <span class="since">Only QEMU driver supports + since 1.0.1</span> + </p> + +<pre> + ... + <devices> + <memory mode='ivshmem'> + <source id='nahanni' path='/tmp/nahanni'/> + <size unit='KiB'>10240</size> + <ioeventfd/> + </memory> + </devices> + ...</pre> + + <dl> + <dt><code>memory</code></dt> + <dd> + The <code>memory</code> element has one mandatory attribute, + <code>model</code>, its value can only be 'ivshmem' currently. + The optional element <code>source</code> has two attributes. + Attribute <code>id</code> is mandatory, to specify the name of + the memory device; Attribute <code>path</code> specifies the socket + path of the 'ivshmem' memory server. If <code>path</code> is not + specified, the memory device is setup without using interrupts. + The element <code>size</code> specifies the memory device size, + attribute <code>unit</code> behaves the same as for element + <code>memory</code>. Its value must be power of 2 in bytes. + The element <code>vectors</code> to specify how many vectors for + the 'ivshmem' memory device. + The element <code>ioeventfd</code> controls whether or not + ioeventfd is turned on for 'ivshmem' memory device. + </dd> + </dl> + <p> Example automatically added device with KVM </p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ed7d1d0..c55b6a3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2798,6 +2798,43 @@ </zeroOrMore> </element> </define> + <define name="memory"> + <element name="memory"> + <group> + <attribute name="model"> + <value>ivshmem</value> + </attribute> + <interleave> + <element name="source"> + <attribute name="id"> + <text/> + </attribute> + <optional> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + </optional> + </element> + <element name="size"> + <ref name="scaledInteger"/> + </element> + <optional> + <element name="vectors"> + <ref name="unsignedInt"/> + </element> + </optional> + <optional> + <element name="ioeventfd"> + <empty/> + </element> + </optional> + <optional> + <ref name="address"/> + </optional> + </interleave> + </group> + </element> + </define> <define name="hostdev"> <element name="hostdev"> <optional> @@ -2965,6 +3002,7 @@ <ref name="hub"/> <ref name="redirdev"/> <ref name="redirfilter"/> + <ref name="memory"/> </choice> </zeroOrMore> <optional> -- 1.7.7.6
On Wed, Nov 07, 2012 at 05:40:41PM +0800, Osier Yang wrote:
Considering there could be other memory related devices in future, this introduces a general device model, called memory device, instead of a specific device like "ivshmem", currently only "ivshmem" is supported though.
An example of the XML:
<memory model='ivshmem'>
Since we have many sub-elements, I think it is preferrable to also have the model in a subelement as we do with things like disk/network/etc
<source id='nahanni' path='/tmp/nahanni'/>
Use '/dev/shm' as the example path since that's more usual
<size unit='M'>512</size> <vectors>8</vectors> </ioeventfd>
Typo with the '/' location Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 2012年11月08日 22:16, Daniel P. Berrange wrote:
On Wed, Nov 07, 2012 at 05:40:41PM +0800, Osier Yang wrote:
Considering there could be other memory related devices in future, this introduces a general device model, called memory device, instead of a specific device like "ivshmem", currently only "ivshmem" is supported though.
An example of the XML:
<memory model='ivshmem'>
Since we have many sub-elements, I think it is preferrable to also have the model in a subelement as we do with things like disk/network/etc
Agreed.
<source id='nahanni' path='/tmp/nahanni'/>
Use '/dev/shm' as the example path since that's more usual
No, the path is created by ivshmem_server, as I replied in 4/4.
<size unit='M'>512</size> <vectors>8</vectors> </ioeventfd>
Typo with the '/' location
Okay. :-) Regards, Osier
"source id" must be specified for 'ivshmem' memory device, and the size must be power of 2 in bytes (required by QEMU ivshmem device). "ivshmem" device is exposed to guest as a PCI device, so device address is necessary. * src/conf/domain_conf.h (New data struct for memory devices) * src/conf/domain_conf.c (Implement functions to parse, format memory device XML). * src/util/util.h (Declare helper virIsPowerOfTwo) * src/util/util.c (Implement virIsPowerOfTwo) * src/src/libvirt_private.syms (Export the private symbols) --- src/conf/domain_conf.c | 184 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 27 +++++++ src/libvirt_private.syms | 3 + src/util/util.c | 5 + src/util/util.h | 2 + 5 files changed, 220 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db6608e..dbcfcaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -670,6 +670,11 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto"); +VIR_ENUM_IMPL(virDomainMemoryModel, + VIR_DOMAIN_MEMORY_MODEL_LAST, + "ivshmem"); + + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -1384,6 +1389,26 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +static void +virDomainMemoryDefFree(virDomainMemoryDefPtr def) +{ + if (!def) + return; + + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_IVSHMEM: + VIR_FREE(def->data.ivshmem.id); + VIR_FREE(def->data.ivshmem.path); + virDomainDeviceInfoClear(&def->data.ivshmem.info); + break; + + default: + break; + } + + VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1717,6 +1742,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainMemballoonDefFree(def->memballoon); + for (i = 0; i < def->nmemorys; i++) + virDomainMemoryDefFree(def->memorys[i]); + for (i = 0; i < def->nseclabels; i++) virSecurityLabelDefFree(def->seclabels[i]); VIR_FREE(def->seclabels); @@ -7004,7 +7032,6 @@ error: goto cleanup; } - static virDomainMemballoonDefPtr virDomainMemballoonDefParseXML(const xmlNodePtr node, unsigned int flags) @@ -7043,6 +7070,140 @@ error: goto cleanup; } +static virDomainMemoryDefPtr +virDomainMemoryDefParseXML(const xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + virDomainMemoryDefPtr def = NULL; + xmlNodePtr cur = NULL; + char *model = NULL; + + xmlNodePtr oldnode = ctxt->node; + ctxt->node = node; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (!(model = virXMLPropString(node, "model"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Memory model must be specified")); + goto cleanup; + } + + if ((def->model == virDomainMemoryModelTypeFromString(model)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unknown memory model '%s'"), model); + goto cleanup; + } + + if ((def->model == VIR_DOMAIN_MEMORY_MODEL_IVSHMEM) && + !virXPathNode("./source", ctxt)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("memory source must be specified")); + goto cleanup; + } + + cur = node->children; + while (cur != NULL) { + if (def->model == VIR_DOMAIN_MEMORY_MODEL_IVSHMEM) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "source")) { + if (!(def->data.ivshmem.id = virXMLPropString(cur, "id"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("memory source id must be specified")); + goto cleanup; + } + def->data.ivshmem.path = virXMLPropString(cur, "path"); + } else if (xmlStrEqual(cur->name, BAD_CAST "size")) { + if (virDomainParseScaledValue("./size[1]", ctxt, + &def->data.ivshmem.size, 1, + ULLONG_MAX, true) < 0) + goto cleanup; + + if (!virIsPowerOfTwo(def->data.ivshmem.size)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("size for memory device must be " + "power of 2 in bytes")); + goto cleanup; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "vectors")) { + if (virXPathUInt("string(./vectors)", ctxt, + &def->data.ivshmem.vectors) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Malformed vectors for memory device")); + goto cleanup; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "ioeventfd")) { + def->data.ivshmem.ioeventfd = true; + } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { + cur = cur->next; + continue; + } else { + virReportError(VIR_ERR_XML_ERROR, + _("Unknown XML for memory device '%s'"), + cur->name); + goto cleanup; + } + } + } + cur = cur->next; + } + + if (def->model == VIR_DOMAIN_MEMORY_MODEL_IVSHMEM) + if (virDomainDeviceInfoParseXML(node, NULL, &def->data.ivshmem.info, flags) < 0) + goto cleanup; + + ctxt->node = oldnode; + VIR_FREE(model); + return def; + +cleanup: + ctxt->node = oldnode; + virDomainMemoryDefFree(def); + VIR_FREE(model); + return NULL; +} + +static int +virDomainMemoryDefFormat(virBufferPtr buf, + virDomainMemoryDefPtr def, + unsigned int flags) +{ + virBufferAsprintf(buf, " <memory model='%s'>\n", + virDomainMemoryModelTypeToString(def->model)); + + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_IVSHMEM: + virBufferAsprintf(buf, " <source id='%s'", def->data.ivshmem.id); + virBufferEscapeString(buf, " path='%s'", def->data.ivshmem.path); + virBufferAddLit(buf, "/>\n"); + + virBufferAsprintf(buf, " <size unit='bytes'>%llu</size>\n", + def->data.ivshmem.size); + + if (def->data.ivshmem.vectors) + virBufferAsprintf(buf, " <vectors>%u</vectors>\n", + def->data.ivshmem.vectors); + if (def->data.ivshmem.ioeventfd) + virBufferAddLit(buf, " <ioeventfd/>\n"); + + + if (virDomainDeviceInfoFormat(buf, &def->data.ivshmem.info, flags) < 0) + return -1; + + virBufferAddLit(buf, " </memory>\n"); + break; + + default: + break; + } + + return 0; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -10017,6 +10178,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } } + /* analysis of the memory devices */ + if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0) { + goto error; + } + if (n && VIR_ALLOC_N(def->memorys, n) < 0) + goto no_memory; + for (i = 0 ; i < n ; i++) { + virDomainMemoryDefPtr memory = virDomainMemoryDefParseXML(nodes[i], + ctxt, + flags); + if (!memory) + goto error; + def->memorys[def->nmemorys++] = memory; + } + VIR_FREE(nodes); + /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) { goto error; @@ -14179,6 +14356,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->memballoon) virDomainMemballoonDefFormat(buf, def->memballoon, flags); + for (n = 0; n < def->nmemorys; n++) { + if (virDomainMemoryDefFormat(buf, def->memorys[n], flags) < 0) + goto cleanup; + } + virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c7c1ca6..f68b75f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -109,6 +109,9 @@ typedef virDomainChrDef *virDomainChrDefPtr; typedef struct _virDomainMemballoonDef virDomainMemballoonDef; typedef virDomainMemballoonDef *virDomainMemballoonDefPtr; +typedef struct _virDomainMemoryDef virDomainMemoryDef; +typedef virDomainMemoryDef *virDomainMemoryDefPtr; + typedef struct _virDomainSnapshotObj virDomainSnapshotObj; typedef virDomainSnapshotObj *virDomainSnapshotObjPtr; @@ -1352,6 +1355,26 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; }; +enum virDomainMemoryModel { + VIR_DOMAIN_MEMORY_MODEL_IVSHMEM, + + VIR_DOMAIN_MEMORY_MODEL_LAST, +}; + +struct _virDomainMemoryDef { + int model; + + union { + struct { + char *id; + char *path; + unsigned long long size; + unsigned int vectors; + bool ioeventfd; + virDomainDeviceInfo info; + } ivshmem; + } data; +}; enum virDomainSmbiosMode { VIR_DOMAIN_SMBIOS_NONE, @@ -1780,6 +1803,9 @@ struct _virDomainDef { size_t nseclabels; virSecurityLabelDefPtr *seclabels; + size_t nmemorys; + virDomainMemoryDefPtr *memorys; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2250,6 +2276,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode) VIR_ENUM_DECL(virDomainNumatuneMemMode) VIR_ENUM_DECL(virDomainNumatuneMemPlacementMode) VIR_ENUM_DECL(virDomainHyperv) +VIR_ENUM_DECL(virDomainMemoryModel) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e94b478..e4b6c49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -434,6 +434,8 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemDumpTypeFromString; virDomainMemDumpTypeToString; +virDomainMemoryModelTypeFromString; +virDomainMemoryModelTypeToString; virDomainNetDefFree; virDomainNetFind; virDomainNetFindIdx; @@ -1266,6 +1268,7 @@ virGetUserName; virHexToBin; virIndexToDiskName; virIsDevMapperDevice; +virIsPowerOfTwo; virParseNumber; virParseVersionString; virPipeReadUntilEOF; diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..858d64e 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3114,3 +3114,8 @@ virValidateWWN(const char *wwn) { return true; } + +bool +virIsPowerOfTwo(unsigned long long n) { + return (n & (n - 1)) == 0; +} diff --git a/src/util/util.h b/src/util/util.h index 4316ab1..fd32b0c 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -280,4 +280,6 @@ bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); bool virValidateWWN(const char *wwn); +bool virIsPowerOfTwo(unsigned long long n); + #endif /* __VIR_UTIL_H__ */ -- 1.7.7.6
To indicate whether the underlying QEMU supports ivshmem device. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 12 ++++++++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 97b0b24..4d77cfa 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -191,6 +191,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "vnc", "drive-mirror", /* 115 */ + "ivshmem", ); struct _qemuCaps { @@ -1282,6 +1283,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = { { "scsi-block", QEMU_CAPS_SCSI_BLOCK }, { "scsi-cd", QEMU_CAPS_SCSI_CD }, { "ide-cd", QEMU_CAPS_IDE_CD }, + { "ivshmem", QEMU_CAPS_IVSHMEM }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index fb88aa1..e4c6ffe 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -153,6 +153,7 @@ enum qemuCapsFlags { QEMU_CAPS_BLOCK_COMMIT = 113, /* block-commit */ QEMU_CAPS_VNC = 114, /* Is -vnc available? */ QEMU_CAPS_DRIVE_MIRROR = 115, /* drive-mirror monitor command */ + QEMU_CAPS_IVSHMEM = 116, /* Is ivshmem available? */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index b49c86d..fa775cd 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -495,7 +495,8 @@ mymain(void) QEMU_CAPS_VIRTIO_BLK_SG_IO, QEMU_CAPS_CPU_HOST, QEMU_CAPS_SCSI_LSI, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_IVSHMEM); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -697,7 +698,8 @@ mymain(void) QEMU_CAPS_IDE_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_BLOCKIO, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_IVSHMEM); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -779,7 +781,8 @@ mymain(void) QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI, QEMU_CAPS_BLOCKIO, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_IVSHMEM); DO_TEST("qemu-1.2.0", 1002000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -864,7 +867,8 @@ mymain(void) QEMU_CAPS_SCSI_DISK_WWN, QEMU_CAPS_SECCOMP_SANDBOX, QEMU_CAPS_DUMP_GUEST_CORE, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_IVSHMEM); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.7.6
Depends on whether "source path" is specified, the command line is built like: * 'path' is specified, with interrupts (/tmp/nahanni is the ivshmem server socket path) -chardev socket,path=/tmp/nahanni,id=nahanni -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on * 'path' is not specified, without interrupts -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on * src/qemu/qemu_command.c (New helper qemuBuildMemoryDevStr to to build args of '-device'; Assign PCI address for the device; Build args of '-chardev') * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml: (See below) * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args: (See below) * tests/qemuxml2argvtest.c: (Add tests) --- src/qemu/qemu_command.c | 85 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 7 ++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 33 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b0b81f3..2338d44 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1705,6 +1705,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* Nada - none are PCI based (yet) */ } + for (i = 0; i < def->nmemorys; i++) { + virDomainMemoryDefPtr memory = def->memorys[i]; + + if (memory->model == VIR_DOMAIN_MEMORY_MODEL_IVSHMEM) { + if (qemuDomainPCIAddressSetNextAddr(addrs, + &memory->data.ivshmem.info) < 0) + goto error; + } + } + return 0; error: @@ -4439,6 +4449,48 @@ error: return -1; } +static char * +qemuBuildMemoryDevStr(const virDomainMemoryDefPtr def, + qemuCapsPtr caps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (def->model == VIR_DOMAIN_MEMORY_MODEL_IVSHMEM) { + virBufferAddLit(&buf, "ivshmem"); + + if (def->data.ivshmem.path) + virBufferAsprintf(&buf, ",chardev=%s", + def->data.ivshmem.id); + else + virBufferAsprintf(&buf, ",shm=%s", + def->data.ivshmem.id); + + virBufferAsprintf(&buf, ",size=%llum", + def->data.ivshmem.size / (1024 * 1024)); + + if (def->data.ivshmem.vectors) + virBufferAsprintf(&buf, ",vectors=%u", + def->data.ivshmem.vectors); + + if (def->data.ivshmem.ioeventfd) + virBufferAddLit(&buf, ",ioeventfd=on"); + + if (qemuBuildDeviceAddressStr(&buf, &def->data.ivshmem.info, caps) < 0) + goto error; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -6616,6 +6668,39 @@ qemuBuildCommandLine(virConnectPtr conn, } } + if (def->nmemorys > 0) { + for (i = 0; i < def->nmemorys; i++) { + char *devstr = NULL; + virDomainMemoryDefPtr memory = def->memorys[i]; + + switch (memory->model) { + case VIR_DOMAIN_MEMORY_MODEL_IVSHMEM: + if (!qemuCapsGet(caps, QEMU_CAPS_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem is not supported by this QEMU")); + goto error; + } + + if (memory->data.ivshmem.path) { + virCommandAddArg(cmd, "-chardev"); + virCommandAddArgFormat(cmd, "socket,path=%s,id=%s", + memory->data.ivshmem.path, + memory->data.ivshmem.id); + } + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildMemoryDevStr(memory, caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + break; + default: + break; + } + } + } + if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args new file mode 100644 index 0000000..2f6a096 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ +-name QEMUGuest1 -S -M pc -m 214 -smp 1 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none \ +-chardev socket,path=/tmp/nahanni,id=nahanni \ +-device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml new file mode 100644 index 0000000..170abfc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>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> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <memory model='ivshmem'> + <source id='nahanni' path='/tmp/nahanni'/> + <size unit="M">512</size> + <vectors>8</vectors> + <ioeventfd/> + </memory> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39a7e3f..ebb6dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -845,6 +845,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD, QEMU_CAPS_BLOCKIO); + DO_TEST("ivshmem", QEMU_CAPS_NAME, QEMU_CAPS_IVSHMEM); + VIR_FREE(driver.stateDir); virCapabilitiesFree(driver.caps); VIR_FREE(map); -- 1.7.7.6
On Wed, Nov 07, 2012 at 05:40:44PM +0800, Osier Yang wrote:
Depends on whether "source path" is specified, the command line is built like:
* 'path' is specified, with interrupts (/tmp/nahanni is the ivshmem server socket path) -chardev socket,path=/tmp/nahanni,id=nahanni -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on
* 'path' is not specified, without interrupts -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on
* src/qemu/qemu_command.c (New helper qemuBuildMemoryDevStr to to build args of '-device'; Assign PCI address for the device; Build args of '-chardev') * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml: (See below) * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args: (See below) * tests/qemuxml2argvtest.c: (Add tests)
--- src/qemu/qemu_command.c | 85 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 7 ++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 33 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml
No changes to SELinux ? I'm reasonably sure the policy should forbid QEMU from creating the shared memory backing file. For hugepages, we had to create a per-QEMU directory, label that and then pass it to QEMU. This perhaps implies that our XML should not contain the actual path. Libvirt can just create a path based on the "name" of the device and set a label of the dir as for hugepages. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 2012年11月08日 22:18, Daniel P. Berrange wrote:
On Wed, Nov 07, 2012 at 05:40:44PM +0800, Osier Yang wrote:
Depends on whether "source path" is specified, the command line is built like:
* 'path' is specified, with interrupts (/tmp/nahanni is the ivshmem server socket path) -chardev socket,path=/tmp/nahanni,id=nahanni -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on
* 'path' is not specified, without interrupts -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on
* src/qemu/qemu_command.c (New helper qemuBuildMemoryDevStr to to build args of '-device'; Assign PCI address for the device; Build args of '-chardev') * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml: (See below) * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args: (See below) * tests/qemuxml2argvtest.c: (Add tests)
--- src/qemu/qemu_command.c | 85 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 7 ++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 33 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml
No changes to SELinux ? I'm reasonably sure the policy should forbid QEMU from creating the shared memory backing file. For hugepages, we had to create a per-QEMU directory, label that and then pass it to QEMU.
Ah, I didn't realize that, especially when the selinux is disabled, thanks for pointing it out.
This perhaps implies that our XML should not contain the actual path. Libvirt can just create a path based on the "name" of the device and set a label of the dir as for hugepages.
The shared memory server socket path is not created by QEMU, but by a external application (called "ivshmem_server [1]"), creating it in libvirt implies a force for users. They have to known our rule first, and start the shared memory server with the right socket path, and then starts the domain, generally it should be opposite with the workflow which users want. So IMO it's not a good way to go. [1]: http://gitorious.org/nahanni/guest-code/trees/master/ivshmem-server Regards, Osier
participants (2)
-
Daniel P. Berrange -
Osier Yang