[libvirt] [PATCH 0/4 v2] ivshmem support
v1 - v2: * Change attribute "model" to be a sub-element instead. NOTE: Since the invshmem server socket path is not created by QEMU, but by an external app called "ivshmem_server", It's not good to construct the socket path in libvirt with a solid rule, and force the user to figure out what the path is libvirt uses first and use that for "ivshmem_server". Thus it's the user's business to set the selinux context on the socket path so that the qemu process could be started successfully when selinux is enabled. 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 | 40 +++++ docs/schemas/domaincommon.rng | 40 +++++ src/conf/domain_conf.c | 195 +++++++++++++++++++++- 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 | 34 ++++ tests/qemuxml2argvtest.c | 2 + 14 files changed, 450 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml -- 1.7.7.6
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 type='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. --- 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 c8da33d..dedfa17 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4088,6 +4088,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 2beb035..a99a1d4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2788,6 +2788,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> @@ -2955,6 +2992,7 @@ <ref name="hub"/> <ref name="redirdev"/> <ref name="redirfilter"/> + <ref name="memory"/> </choice> </zeroOrMore> <optional> -- 1.7.7.6
Il 16/11/2012 10:59, Osier Yang ha scritto:
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
I think <doorbell/> is a better name than ioeventfd. That is somewhat kvm-specific, but there's no reason for it to be. Paolo
On 2012年11月17日 06:47, Paolo Bonzini wrote:
Il 16/11/2012 10:59, Osier Yang ha scritto:
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
I think<doorbell/> is a better name than ioeventfd.
That's a magic name from my p.o.v, because it can indicate any boolean options. How should I get it documented? :-)
That is somewhat kvm-specific, but there's no reason for it to be.
Being general is the rule to introduce new XML tags, but sometimes the better way is to keep it specific. It's not that bad because at least we can ensure the specific tag only works for the specific driver. Regards, Osier
Il 19/11/2012 04:45, Osier Yang ha scritto:
On 2012年11月17日 06:47, Paolo Bonzini wrote:
Il 16/11/2012 10:59, Osier Yang ha scritto:
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
I'm not sure that ioeventfd=on adds anything when no chardev is in effect, actually.
I think<doorbell/> is a better name than ioeventfd.
That's a magic name from my p.o.v, because it can indicate any boolean options. How should I get it documented? :-)
The "doorbell" element, if present, lets each guest send an interrupt to other guests. For the ivshmem model, this requires the path to be specified and ivshmem_server to be running. Paolo
That is somewhat kvm-specific, but there's no reason for it to be.
Being general is the rule to introduce new XML tags, but sometimes the better way is to keep it specific. It's not that bad because at least we can ensure the specific tag only works for the specific driver.
How can we simply modify xml configuration file of VM to add ivshmem device without interrupts and start vm from virsh command line??
On 01/04/2013 12:56 AM, Mrugani wrote:
How can we simply modify xml configuration file of VM to add ivshmem device without interrupts and start vm from virsh command line??
I assume you are replying to this thread: https://www.redhat.com/archives/libvir-list/2012-November/msg00731.html That work is not yet incorporated into libvirt (it is still under discussion); but in the meantime, while waiting for it to be resolved, you can use qemu passthrough to specify the arguments that libvirt does not map: http://libvirt.org/drvqemu.html#qemucommand -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2013年01月04日 22:09, Eric Blake wrote:
On 01/04/2013 12:56 AM, Mrugani wrote:
How can we simply modify xml configuration file of VM to add ivshmem device without interrupts and start vm from virsh command line??
I assume you are replying to this thread: https://www.redhat.com/archives/libvir-list/2012-November/msg00731.html
That work is not yet incorporated into libvirt (it is still under discussion); but in the meantime, while waiting for it to be resolved, you can use qemu passthrough to specify the arguments that libvirt does not map: http://libvirt.org/drvqemu.html#qemucommand
Yeah, the plan is to integrate the ivshmem server into libvirt too. And after that this set will be update, both of the work are still undergoing.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
"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) --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 8 +- src/conf/domain_conf.c | 195 ++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 27 ++++++ src/libvirt_private.syms | 3 + src/util/util.c | 5 + src/util/util.h | 2 + 7 files changed, 240 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dedfa17..4e8b0db 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4099,7 +4099,8 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <memory mode='ivshmem'> + <memory'> + <model type='ivshmem'/> <source id='nahanni' path='/tmp/nahanni'/> <size unit='KiB'>10240</size> <ioeventfd/> @@ -4110,8 +4111,8 @@ qemu-kvm -net nic,model=? /dev/null <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 mandatory element <code>model</code> has one attribute, + <code>type</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 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a99a1d4..838efe0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2791,9 +2791,11 @@ <define name="memory"> <element name="memory"> <group> - <attribute name="model"> - <value>ivshmem</value> - </attribute> + <element name="model"> + <attribute name="type"> + <value>ivshmem</value> + </attribute> + </element> <interleave> <element name="source"> <attribute name="id"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 99f03a9..828228e 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 @@ -1382,6 +1387,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) @@ -1715,6 +1740,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); @@ -6974,7 +7002,6 @@ error: goto cleanup; } - static virDomainMemballoonDefPtr virDomainMemballoonDefParseXML(const xmlNodePtr node, unsigned int flags) @@ -7013,6 +7040,151 @@ 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 (!virXPathNode("./model", ctxt)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("memory model must be specified")); + goto cleanup; + } + + if (!(model = virXPathString("string(./model/@type)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Memory model type 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 "model")) { + cur = cur->next; + continue; + } else 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>\n"); + + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_IVSHMEM: + virBufferAsprintf(buf, " <model type='%s'/>\n", + virDomainMemoryModelTypeToString(def->model)); + 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) @@ -9987,6 +10159,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; @@ -14149,6 +14337,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 6539281..c959757 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; @@ -1350,6 +1353,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, @@ -1778,6 +1801,9 @@ struct _virDomainDef { size_t nseclabels; virSecurityLabelDefPtr *seclabels; + size_t nmemorys; + virDomainMemoryDefPtr *memorys; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2248,6 +2274,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 5a07139..e29af26 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -434,6 +434,8 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemDumpTypeFromString; virDomainMemDumpTypeToString; +virDomainMemoryModelTypeFromString; +virDomainMemoryModelTypeToString; virDomainNetDefFree; virDomainNetFind; virDomainNetFindIdx; @@ -1267,6 +1269,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 5ce93f2..2bf422a 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 { @@ -1286,6 +1287,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 | 34 +++++++++ tests/qemuxml2argvtest.c | 2 + 4 files changed, 128 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 440fd62..ffd260f 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: @@ -4410,6 +4420,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. @@ -6598,6 +6650,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..15040e4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml @@ -0,0 +1,34 @@ +<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 type='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 48e09ab..8a176d0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -847,6 +847,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
Osier, Thanks for including me in the review of this patch. I can tell right away that your implementation is better. It's more modular and well documented. When I submitted my patch about a month ago, it was basically just me submitting the changes that I made to get ivshmem working for my lab. We were on a tight deadline and so a lot of the code is rushed and poorly pieced together. However, I am glad that my submission has helped at least get the ball rolling on memory device support for Libvirt as it is very much needed. Thanks for your help, Shawn Furrow On Fri, Nov 16, 2012 at 4:59 AM, Osier Yang <jyang@redhat.com> wrote:
v1 - v2: * Change attribute "model" to be a sub-element instead.
NOTE: Since the invshmem server socket path is not created by QEMU, but by an external app called "ivshmem_server", It's not good to construct the socket path in libvirt with a solid rule, and force the user to figure out what the path is libvirt uses first and use that for "ivshmem_server". Thus it's the user's business to set the selinux context on the socket path so that the qemu process could be started successfully when selinux is enabled.
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 | 40 +++++ docs/schemas/domaincommon.rng | 40 +++++ src/conf/domain_conf.c | 195 +++++++++++++++++++++- 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 | 34 ++++ tests/qemuxml2argvtest.c | 2 + 14 files changed, 450 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml
-- 1.7.7.6
-- Virginia Tech Bradley Department of Electrical and Computer Engineering B.S. Electrical Engineering B.S. Computer Engineering
Hi, Shawn, On 2012年11月16日 21:24, Shawn Furrow wrote:
Osier,
Thanks for including me in the review of this patch. I can tell right away that your implementation is better. It's more modular and well documented. When I submitted my patch about a month ago, it was basically just me submitting the changes that I made to get ivshmem working for my lab. We were on a tight deadline and so a lot of the code is rushed and poorly pieced together. However, I am glad that my submission has helped at least get the ball rolling on memory device support for Libvirt as it is very much needed.
So do you see if the implementation satisfies your needs? I.e. Does it work well in your lab? Regards, Osier
participants (5)
-
Eric Blake -
Mrugani -
Osier Yang -
Paolo Bonzini -
Shawn Furrow