[libvirt] [PATCH v2 0/4] Introduce support for virtio-blk-pci iothreads
v1: http://www.redhat.com/archives/libvir-list/2014-August/msg01155.html Changes since v1 Patches 1-3 - purely from code review Patch 4 - rework the checking of the to be added disk that has the iothread property set to be done during qemuBuildDriveDevStr() after the config check. This way the same checks are done for both start and hotplug. Only set the "inuse" bit after qemuBuildDriveDevStr() returns successfully for both start and hotplug. This also enforces only setting for this path Since the only way a disk with the property can be added is if the current emulator supports the feature, the calls to set/clear the bit if iothread is set should be safe from not needing to also ensure iothreadmap exists. John Ferlan (4): domain_conf: Introduce iothreads XML qemu: Add support for iothreads domain_conf: Add support for iothreads in disk definition qemu: Allow use of iothreads for disk definitions docs/formatdomain.html.in | 34 ++++++++++++ docs/schemas/domaincommon.rng | 14 +++++ src/conf/domain_conf.c | 47 +++++++++++++++- src/conf/domain_conf.h | 4 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 64 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 ++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++ .../qemuxml2argv-iothreads-disk.xml | 40 ++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 +++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++ tests/qemuxml2argvtest.c | 4 ++ tests/qemuxml2xmltest.c | 2 + 14 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml -- 1.9.3
Introduce XML to allowing adding iothreads to the domain. These can be used by virtio-blk-pci devices in order to assign a specific thread to handle the workload for the device. The iothreads are the official implementation of the virtio-blk Data Plane that's been in tech preview for QEMU. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 26 ++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 6 ++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 4 files changed, 54 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de4e4eb..b584a08 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -470,6 +470,32 @@ </dd> </dl> + <h3><a name="elementsIOThreadsAllocation">IOThreads Allocation</a></h3> + <p> + IOThreads are a QEMU feature that will allow supported disk + devices to be configured to use a dedicated event loop thread + to handle block I/O requests. + <span class="since">Since 1.2.8 (QEMU only)</span> + </p> + +<pre> +<domain> + ... + <iothreads>4</iothreads> + ... +</domain> +</pre> + + <dl> + <dt><code>iothreads</code></dt> + <dd> + The content of this optional element defines the number + of IOThreads to be assigned to the domain for use by + virtio-blk-pci target storage devices. There should be + only 1 or 2 IOThreads per host CPU and 1 IOThread per + supported device. + </dd> + </dl> <h3><a name="elementsCPUTuning">CPU Tuning</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3170db2..5f1c226 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -632,6 +632,12 @@ </optional> <optional> + <element name="iothreads"> + <ref name="unsignedInt"/> + </element> + </optional> + + <optional> <ref name="blkiotune"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dd512ca..81a3fdf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11957,6 +11957,15 @@ virDomainDefParseXML(xmlDocPtr xml, } } + /* Optional - iothreads */ + tmp = virXPathString("string(./iothreads[1])", ctxt); + if (tmp && virStrToLong_uip(tmp, NULL, 0, &def->iothreads) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid iothreads count '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + /* Extract cpu tunables. */ if ((n = virXPathULong("string(./cputune/shares[1])", ctxt, &def->cputune.shares)) < -1) { @@ -14537,6 +14546,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } + if (src->iothreads != dst->iothreads) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain iothreads count %u does not " + "match source %u"), + dst->iothreads, src->iothreads); + goto error; + } + if (STRNEQ(src->os.type, dst->os.type)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain OS type %s does not match source %s"), @@ -17950,6 +17967,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " current='%u'", def->vcpus); virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); + if (def->iothreads > 0) + virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", def->iothreads); + if (def->cputune.sharesSpecified || (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || def->cputune.period || def->cputune.quota || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index aead903..23b117d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1915,6 +1915,8 @@ struct _virDomainDef { int placement_mode; virBitmapPtr cpumask; + unsigned int iothreads; + struct { unsigned long shares; bool sharesSpecified; -- 1.9.3
On Tue, Aug 26, 2014 at 06:15:46PM -0400, John Ferlan wrote:
Introduce XML to allowing adding iothreads to the domain. These can be used by virtio-blk-pci devices in order to assign a specific thread to handle the workload for the device. The iothreads are the official implementation of the virtio-blk Data Plane that's been in tech preview for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 26 ++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 6 ++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 4 files changed, 54 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de4e4eb..b584a08 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -470,6 +470,32 @@ </dd> </dl>
+ <h3><a name="elementsIOThreadsAllocation">IOThreads Allocation</a></h3> + <p> + IOThreads are a QEMU feature that will allow supported disk
s/QEMU feature that will allow/feature that allow/ because you say it's QEMU only three lines down the page. TBH I'm not sure it should be "allow" (plural "IOThreads") or "allow" (singular "feature"), but that I leave to you since I'm not a native speaker. ACK with that line fixed up. Martin
On Tue, Aug 26, 2014 at 06:15:46PM -0400, John Ferlan wrote:
Introduce XML to allowing adding iothreads to the domain. These can be used by virtio-blk-pci devices in order to assign a specific thread to handle the workload for the device. The iothreads are the official implementation of the virtio-blk Data Plane that's been in tech preview for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 26 ++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 6 ++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 4 files changed, 54 insertions(+)
I forgot two things in the review.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dd512ca..81a3fdf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11957,6 +11957,15 @@ virDomainDefParseXML(xmlDocPtr xml, } }
+ /* Optional - iothreads */ + tmp = virXPathString("string(./iothreads[1])", ctxt); + if (tmp && virStrToLong_uip(tmp, NULL, 0, &def->iothreads) < 0) {
One of the things is that you use '0' here (as I suggested, sorry for that), which would not that big of a deal, it would even be awesome, but unfortunately we use '10' everywhere in the XML parsing code :( Even when parsing the iothread="" in 3/4 you are using '10', so this should be changed to '10'. The second thing is that there might be a qemuxml2xmltest case for the qemuxml2argv-iothreads.xml (which can be added in this patch), but that's just a bike-shedding. The ACK still stands, of course. Martin
On 08/26/2014 06:15 PM, John Ferlan wrote:
Introduce XML to allowing adding iothreads to the domain. These can be used by virtio-blk-pci devices in order to assign a specific thread to handle the workload for the device. The iothreads are the official implementation of the virtio-blk Data Plane that's been in tech preview for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 26 ++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 6 ++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 4 files changed, 54 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de4e4eb..b584a08 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -470,6 +470,32 @@ </dd> </dl>
+ <h3><a name="elementsIOThreadsAllocation">IOThreads Allocation</a></h3> + <p> + IOThreads are a QEMU feature that will allow supported disk + devices to be configured to use a dedicated event loop thread + to handle block I/O requests. + <span class="since">Since 1.2.8 (QEMU only)</span> + </p>
Changed the text to: IOThreads are dedicated event loop threads for supported disk devices to perform block I/O requests in order to improve scalability especially on an SMP host/guest with many LUNs. Which probably is the best description I could find from my bz. <...snip...>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dd512ca..81a3fdf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11957,6 +11957,15 @@ virDomainDefParseXML(xmlDocPtr xml, } }
+ /* Optional - iothreads */ + tmp = virXPathString("string(./iothreads[1])", ctxt); + if (tmp && virStrToLong_uip(tmp, NULL, 0, &def->iothreads) < 0) {
And yes s/10/0 was done here as well - so much for cut-n-paste :-) <...snip...> Tks, John
On Tue, Aug 26, 2014 at 11:15 PM, John Ferlan <jferlan@redhat.com> wrote:
+ The content of this optional element defines the number + of IOThreads to be assigned to the domain for use by + virtio-blk-pci target storage devices. There should be + only 1 or 2 IOThreads per host CPU and 1 IOThread per + supported device.
Fine for now but in the future devices may support multiple IOThreads assigned. This is necessary for multiqueue block layer support, where a storage controller can submit I/O from multiple host CPUs at the same time. Stefan
Add a new capability to ensure the iothreads feature exists for the qemu emulator being run - requires the "query-iothreads" QMP command. Using the domain XML add correspoding command argument in order to generate the threads. The iothreads will use a name space "iothread#" where, the future patch to add support for using an iothread to a disk definition to merely define which of the available threads to use. Add tests to ensure the xml/argv processing is correct. Note that no change was made to qemuargv2xmltest.c as processing the -object element would require knowing more than just iothreads. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 13 ++++++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 ++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 56 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 410086b..70c28e0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -268,6 +268,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "rtc-reset-reinjection", "splash-timeout", /* 175 */ + "iothread", ); @@ -1491,6 +1492,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "memory-backend-ram", QEMU_CAPS_OBJECT_MEMORY_RAM }, { "memory-backend-file", QEMU_CAPS_OBJECT_MEMORY_FILE }, { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO }, + { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 48a4eaa..0980c00 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -215,6 +215,7 @@ typedef enum { QEMU_CAPS_OBJECT_USB_AUDIO = 173, /* usb-audio device support */ QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor command */ QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */ + QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9241f57..ba0b977 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7541,6 +7541,19 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, smp); VIR_FREE(smp); + if (def->iothreads > 0 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + /* Create named iothread objects starting with 1. These may be used + * by a disk definition which will associate to an iothread by + * supplying a value of 1 up to the number of iothreads available + * (since 0 would indicate to not use the feature). + */ + for (i = 1; i <= def->iothreads; i++) { + virCommandAddArg(cmd, "-object"); + virCommandAddArgFormat(cmd, "iothread,id=iothread%zu", i); + } + } + if (def->cpu && def->cpu->ncells) if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps) < 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args new file mode 100644 index 0000000..4ef34cf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.args @@ -0,0 +1,8 @@ +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 2 \ +-object iothread,id=iothread1 \ +-object iothread,id=iothread2 \ +-nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml new file mode 100644 index 0000000..12a92e7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml @@ -0,0 +1,29 @@ +<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'>2</vcpu> + <iothreads>2</iothreads> + <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'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1a8201a..64a4d3d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1185,6 +1185,8 @@ mymain(void) DO_TEST("smp", QEMU_CAPS_SMP_TOPOLOGY); + DO_TEST("iothreads", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST("cpu-topology1", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("cpu-topology2", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("cpu-topology3", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index b2d2eed..796977b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -301,6 +301,7 @@ mymain(void) DO_TEST("cputune-zero-shares"); DO_TEST("smp"); + DO_TEST("iothreads"); DO_TEST("lease"); DO_TEST("event_idx"); DO_TEST("vhost_queues"); -- 1.9.3
On Tue, Aug 26, 2014 at 06:15:47PM -0400, John Ferlan wrote:
Add a new capability to ensure the iothreads feature exists for the qemu emulator being run - requires the "query-iothreads" QMP command. Using the domain XML add correspoding command argument in order to generate the threads. The iothreads will use a name space "iothread#" where, the future patch to add support for using an iothread to a disk definition to merely define which of the available threads to use.
Add tests to ensure the xml/argv processing is correct. Note that no change was made to qemuargv2xmltest.c as processing the -object element would require knowing more than just iothreads.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 13 ++++++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 ++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 56 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
ACK, Martin
Add a new disk "driver" attribute "iothread" to be parsed as the thread number for the disk to use. In order to more easily facilitate the usage and configuration of the iothread, a "zero" for the attribute indicates iothreads are not supported for the device and a positive value indicates the specific thread to try and use. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 8 ++++++++ docs/schemas/domaincommon.rng | 8 ++++++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b584a08..132952d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2160,6 +2160,14 @@ (ignore the discard request). <span class='since'>Since 1.0.6 (QEMU and KVM only)</span> </li> + <li> + The optional <code>iothread</code> attribute will assign the + disk to an IOThread as defined by the range for the domain + <a href="#elementsIOThreadsAllocation"><code>iothreads</code></a> + value. Each device must use a unique IOThread and threads will + be numbered from 1 to the domain iothreads value. + <span class='since'>Since 1.2.8 (QEMU only)</span> + </li> </ul> </dd> <dt><code>boot</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5f1c226..cedceae 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1572,6 +1572,9 @@ <optional> <ref name="discard"/> </optional> + <optional> + <ref name="driverIOThread"/> + </optional> <empty/> </element> </define> @@ -1659,6 +1662,11 @@ </choice> </attribute> </define> + <define name="driverIOThread"> + <attribute name='iothread'> + <ref name="unsignedInt"/> + </attribute> + </define> <define name="controller"> <element name="controller"> <attribute name="index"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81a3fdf..14b8f4a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2109,6 +2109,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->name); virBitmapFree(def->cpumask); + virBitmapFree(def->iothreadmap); VIR_FREE(def->emulator); VIR_FREE(def->description); VIR_FREE(def->title); @@ -5403,6 +5404,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *ioeventfd = NULL; char *event_idx = NULL; char *copy_on_read = NULL; + char *driverIOThread = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -5551,6 +5553,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, event_idx = virXMLPropString(cur, "event_idx"); copy_on_read = virXMLPropString(cur, "copy_on_read"); discard = virXMLPropString(cur, "discard"); + driverIOThread = virXMLPropString(cur, "iothread"); } else if (!def->mirror && xmlStrEqual(cur->name, BAD_CAST "mirror") && !(flags & VIR_DOMAIN_XML_INACTIVE)) { @@ -6084,6 +6087,15 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (driverIOThread) { + if (virStrToLong_uip(driverIOThread, NULL, 10, &def->iothread) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid iothread attribute in disk driver " + "element: %s"), driverIOThread); + goto error; + } + } + if (devaddr) { if (virDomainParseLegacyDeviceAddress(devaddr, &def->info.addr.pci) < 0) { @@ -6184,6 +6196,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(event_idx); VIR_FREE(copy_on_read); VIR_FREE(discard); + VIR_FREE(driverIOThread); VIR_FREE(devaddr); VIR_FREE(serial); virStorageEncryptionFree(encryption); @@ -11966,6 +11979,16 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(tmp); + if (def->iothreads) { + /* Create a bitmap for inuse threads - noting that entries are + * numbered 1..def->iothreads since 0 (zero) iothreads means + * nothing and assigning a disk to an IOThread requires at least a + * thread# > 0 since a zero would indicate no IOThread for the disk + */ + if (!(def->iothreadmap = virBitmapNew(def->iothreads+1))) + goto error; + } + /* Extract cpu tunables. */ if ((n = virXPathULong("string(./cputune/shares[1])", ctxt, &def->cputune.shares)) < -1) { @@ -15606,7 +15629,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->src->driverName || def->src->format > 0 || def->cachemode || def->error_policy || def->rerror_policy || def->iomode || def->ioeventfd || def->event_idx || def->copy_on_read || - def->discard) { + def->discard || def->iothread) { virBufferAddLit(buf, "<driver"); if (def->src->driverName) virBufferAsprintf(buf, " name='%s'", def->src->driverName); @@ -15629,6 +15652,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " copy_on_read='%s'", copy_on_read); if (def->discard) virBufferAsprintf(buf, " discard='%s'", discard); + if (def->iothread) + virBufferAsprintf(buf, " iothread='%u'", def->iothread); virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 23b117d..a543fc7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -667,6 +667,7 @@ struct _virDomainDiskDef { int rawio; /* no = 0, yes = 1 */ int sgio; /* enum virDomainDeviceSGIO */ int discard; /* enum virDomainDiskDiscard */ + unsigned int iothread; /* unused = 0, > 0 specific thread # */ }; @@ -1916,6 +1917,7 @@ struct _virDomainDef { virBitmapPtr cpumask; unsigned int iothreads; + virBitmapPtr iothreadmap; struct { unsigned long shares; -- 1.9.3
On Tue, Aug 26, 2014 at 06:15:48PM -0400, John Ferlan wrote:
Add a new disk "driver" attribute "iothread" to be parsed as the thread number for the disk to use. In order to more easily facilitate the usage and configuration of the iothread, a "zero" for the attribute indicates iothreads are not supported for the device and a positive value indicates the specific thread to try and use.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 8 ++++++++ docs/schemas/domaincommon.rng | 8 ++++++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b584a08..132952d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2160,6 +2160,14 @@ (ignore the discard request). <span class='since'>Since 1.0.6 (QEMU and KVM only)</span> </li> + <li> + The optional <code>iothread</code> attribute will assign the
I'm rather fan of "assigns" then "will assign", but both make sense here. ACK either way. Martin
On 08/27/2014 02:42 AM, Martin Kletzander wrote:
On Tue, Aug 26, 2014 at 06:15:48PM -0400, John Ferlan wrote:
Add a new disk "driver" attribute "iothread" to be parsed as the thread number for the disk to use. In order to more easily facilitate the usage and configuration of the iothread, a "zero" for the attribute indicates iothreads are not supported for the device and a positive value indicates the specific thread to try and use.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 8 ++++++++ docs/schemas/domaincommon.rng | 8 ++++++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b584a08..132952d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2160,6 +2160,14 @@ (ignore the discard request). <span class='since'>Since 1.0.6 (QEMU and KVM only)</span> </li> + <li> + The optional <code>iothread</code> attribute will assign the
I'm rather fan of "assigns" then "will assign", but both make sense here.
ACK either way.
Martin
"assigns" it is then :-) (use of 'will' is probably that passive side of things) John
For virtio-blk-pci disks with the disk iothread attribute that are running the correct emulator, add the "iothread=iothread#" to the -device command line in order to enable iothreads for the disk. This code will check both the "start" and "hotplug" paths for the capability, whether the iothreadsmap has been defined, and whether there's an available iothread to be used for the disk. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 +++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++++ .../qemuxml2argv-iothreads-disk.xml | 40 +++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba0b977..763bb74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3673,6 +3673,49 @@ qemuBuildDriveStr(virConnectPtr conn, return NULL; } + +static bool +qemuCheckIothreads(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainDiskDefPtr disk) +{ + bool inuse; + + /* Have capability */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported for this QEMU")); + return false; + } + + /* Right "type" of disk" */ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not available for this disk")); + return false; + } + + /* Value larger than iothreads available? */ + if (disk->iothread > def->iothreads) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk iothread '%u' invalid only %u IOThreads"), + disk->iothread, def->iothreads); + return false; + } + + /* Is the requested entry available */ + if (virBitmapGetBit(def->iothreadmap, disk->iothread, &inuse) < 0 || + inuse) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOThread '%u' is already in use"), disk->iothread); + return false; + } + + return true; +} + + char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -3697,6 +3740,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } } + if (disk->iothread && !qemuCheckIothreads(def, qemuCaps, disk)) + goto error; + switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: if (disk->info.addr.drive.target != 0) { @@ -3874,6 +3920,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virBufferAddLit(&opt, "virtio-blk-device"); } else { virBufferAddLit(&opt, "virtio-blk-pci"); + if (disk->iothread) + virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); } qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps); if (disk->event_idx && @@ -8288,6 +8336,9 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; virCommandAddArg(cmd, optstr); VIR_FREE(optstr); + if (disk->iothread) + ignore_value(virBitmapSetBit(def->iothreadmap, + disk->iothread)); } } } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a364c52..71f47b2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -379,6 +379,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virFreeError(orig_err); } } + if (ret == 0 && disk->iothread) + ignore_value(virBitmapSetBit(vm->def->iothreadmap, + disk->iothread)); } } else if (!disk->info.type || disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { @@ -2539,6 +2542,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } + if (disk->iothread) + ignore_value(virBitmapClearBit(vm->def->iothreadmap, disk->iothread)); + qemuDomainReleaseDeviceAddress(vm, &disk->info, src); if (virSecurityManagerRestoreDiskLabel(driver->securityManager, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args new file mode 100644 index 0000000..4d271ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args @@ -0,0 +1,17 @@ +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 2 \ +-object iothread,id=iothread1 \ +-object iothread,id=iothread2 \ +-nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=/var/lib/libvirt/images/iothrtest1.img,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,iothread=iothread1,bus=pci.0,addr=0x4,\ +drive=drive-virtio-disk1,id=virtio-disk1 \ +-drive file=/var/lib/libvirt/images/iothrtest2.img,if=none,\ +id=drive-virtio-disk2 \ +-device virtio-blk-pci,iothread=iothread2,bus=pci.0,addr=0x3,\ +drive=drive-virtio-disk2,id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml new file mode 100644 index 0000000..72122fb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml @@ -0,0 +1,40 @@ +<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'>2</vcpu> + <iothreads>2</iothreads> + <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'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' iothread='1'/> + <source file='/var/lib/libvirt/images/iothrtest1.img'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' iothread='2'/> + <source file='/var/lib/libvirt/images/iothrtest2.img'/> + <target dev='vdc' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 64a4d3d..3feb2fe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1186,6 +1186,8 @@ mymain(void) DO_TEST("smp", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("iothreads", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST("iothreads-disk", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE, + QEMU_CAPS_DRIVE); DO_TEST("cpu-topology1", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("cpu-topology2", QEMU_CAPS_SMP_TOPOLOGY); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 796977b..b4ab671 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -302,6 +302,7 @@ mymain(void) DO_TEST("smp"); DO_TEST("iothreads"); + DO_TEST("iothreads-disk"); DO_TEST("lease"); DO_TEST("event_idx"); DO_TEST("vhost_queues"); -- 1.9.3
On Tue, Aug 26, 2014 at 06:15:49PM -0400, John Ferlan wrote:
For virtio-blk-pci disks with the disk iothread attribute that are running the correct emulator, add the "iothread=iothread#" to the -device command line in order to enable iothreads for the disk.
This code will check both the "start" and "hotplug" paths for the capability, whether the iothreadsmap has been defined, and whether there's an available iothread to be used for the disk.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 +++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++++ .../qemuxml2argv-iothreads-disk.xml | 40 +++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba0b977..763bb74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3673,6 +3673,49 @@ qemuBuildDriveStr(virConnectPtr conn, return NULL; }
+ +static bool +qemuCheckIothreads(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainDiskDefPtr disk) +{ + bool inuse; + + /* Have capability */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported for this QEMU")); + return false; + } + + /* Right "type" of disk" */ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not available for this disk"));
Sorry to say that, but this is like another extreme, it feels too generic. If there are multiple disks, the user might not be sure which one you mean. Listing the allowed ones is fine, I'd say something like "IOThreads only available for virtio pci disk" is enough. ACK with that changed. Martin
On 08/27/2014 03:00 AM, Martin Kletzander wrote:
On Tue, Aug 26, 2014 at 06:15:49PM -0400, John Ferlan wrote:
For virtio-blk-pci disks with the disk iothread attribute that are running the correct emulator, add the "iothread=iothread#" to the -device command line in order to enable iothreads for the disk.
This code will check both the "start" and "hotplug" paths for the capability, whether the iothreadsmap has been defined, and whether there's an available iothread to be used for the disk.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 +++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++++ .../qemuxml2argv-iothreads-disk.xml | 40 +++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba0b977..763bb74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3673,6 +3673,49 @@ qemuBuildDriveStr(virConnectPtr conn, return NULL; }
+ +static bool +qemuCheckIothreads(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainDiskDefPtr disk) +{ + bool inuse; + + /* Have capability */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported for this QEMU")); + return false; + } + + /* Right "type" of disk" */ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not available for this disk"));
Sorry to say that, but this is like another extreme, it feels too generic. If there are multiple disks, the user might not be sure which one you mean. Listing the allowed ones is fine, I'd say something like "IOThreads only available for virtio pci disk" is enough.
ACK with that changed.
OK - I left it at "virtio pci", but can change it to "virtio-pci" or "virtio-blk-pci" based on the answer to the following question... Is this safe enough to be pushed with the freeze now in effect. I know the review started before and all that, but I figured I'd ask before pushing in any case! Tks, John
On Wed, Aug 27, 2014 at 10:41:45AM -0400, John Ferlan wrote:
On 08/27/2014 03:00 AM, Martin Kletzander wrote:
On Tue, Aug 26, 2014 at 06:15:49PM -0400, John Ferlan wrote:
For virtio-blk-pci disks with the disk iothread attribute that are running the correct emulator, add the "iothread=iothread#" to the -device command line in order to enable iothreads for the disk.
This code will check both the "start" and "hotplug" paths for the capability, whether the iothreadsmap has been defined, and whether there's an available iothread to be used for the disk.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 +++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++++ .../qemuxml2argv-iothreads-disk.xml | 40 +++++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 6 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba0b977..763bb74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3673,6 +3673,49 @@ qemuBuildDriveStr(virConnectPtr conn, return NULL; }
+ +static bool +qemuCheckIothreads(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virDomainDiskDefPtr disk) +{ + bool inuse; + + /* Have capability */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported for this QEMU")); + return false; + } + + /* Right "type" of disk" */ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not available for this disk"));
Sorry to say that, but this is like another extreme, it feels too generic. If there are multiple disks, the user might not be sure which one you mean. Listing the allowed ones is fine, I'd say something like "IOThreads only available for virtio pci disk" is enough.
ACK with that changed.
OK - I left it at "virtio pci", but can change it to "virtio-pci" or "virtio-blk-pci" based on the answer to the following question...
Whatever you like, all of them are understandable.
Is this safe enough to be pushed with the freeze now in effect. I know the review started before and all that, but I figured I'd ask before pushing in any case!
Since the patches (both versions) were sent before the freeze started, and reviewed as well, if I'm counting correctly, it is OK for 1.2.8. Martin
On 08/26/2014 06:15 PM, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2014-August/msg01155.html
Changes since v1
Patches 1-3 - purely from code review Patch 4 - rework the checking of the to be added disk that has the iothread property set to be done during qemuBuildDriveDevStr() after the config check. This way the same checks are done for both start and hotplug.
Only set the "inuse" bit after qemuBuildDriveDevStr() returns successfully for both start and hotplug. This also enforces only setting for this path
Since the only way a disk with the property can be added is if the current emulator supports the feature, the calls to set/clear the bit if iothread is set should be safe from not needing to also ensure iothreadmap exists.
John Ferlan (4): domain_conf: Introduce iothreads XML qemu: Add support for iothreads domain_conf: Add support for iothreads in disk definition qemu: Allow use of iothreads for disk definitions
docs/formatdomain.html.in | 34 ++++++++++++ docs/schemas/domaincommon.rng | 14 +++++ src/conf/domain_conf.c | 47 +++++++++++++++- src/conf/domain_conf.h | 4 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 64 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 ++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++ .../qemuxml2argv-iothreads-disk.xml | 40 ++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 +++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++ tests/qemuxml2argvtest.c | 4 ++ tests/qemuxml2xmltest.c | 2 + 14 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
Given Stephan's comment about allowing more than 1 disk per IOThread - I have removed the iothreadmap (and other remnants including docs). If folks want to see the changes before pushing that's fine, but since I figure I'm just removing code/checks - it'd still be "OK" for push. I will "wait" for an answer knowing the sleep/work cycles of the various interested parties are not the same as mine :-) John
On 08/27/2014 05:35 PM, John Ferlan wrote:
On 08/26/2014 06:15 PM, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2014-August/msg01155.html
Changes since v1
Patches 1-3 - purely from code review Patch 4 - rework the checking of the to be added disk that has the iothread property set to be done during qemuBuildDriveDevStr() after the config check. This way the same checks are done for both start and hotplug.
Only set the "inuse" bit after qemuBuildDriveDevStr() returns successfully for both start and hotplug. This also enforces only setting for this path
Since the only way a disk with the property can be added is if the current emulator supports the feature, the calls to set/clear the bit if iothread is set should be safe from not needing to also ensure iothreadmap exists.
John Ferlan (4): domain_conf: Introduce iothreads XML qemu: Add support for iothreads domain_conf: Add support for iothreads in disk definition qemu: Allow use of iothreads for disk definitions
docs/formatdomain.html.in | 34 ++++++++++++ docs/schemas/domaincommon.rng | 14 +++++ src/conf/domain_conf.c | 47 +++++++++++++++- src/conf/domain_conf.h | 4 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 64 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 6 ++ .../qemuxml2argv-iothreads-disk.args | 17 ++++++ .../qemuxml2argv-iothreads-disk.xml | 40 ++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.args | 8 +++ tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml | 29 ++++++++++ tests/qemuxml2argvtest.c | 4 ++ tests/qemuxml2xmltest.c | 2 + 14 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-disk.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads.xml
Given Stephan's comment about allowing more than 1 disk per IOThread - I have removed the iothreadmap (and other remnants including docs).
If folks want to see the changes before pushing that's fine, but since I figure I'm just removing code/checks - it'd still be "OK" for push. I will "wait" for an answer knowing the sleep/work cycles of the various interested parties are not the same as mine :-)
I pushed the changes without the iothreadmap John
participants (3)
-
John Ferlan -
Martin Kletzander -
Stefan Hajnoczi