Using g_autoptr() for @iothrid variable inside virDomainDefParseIOThreads() allows us to drop explicit call to virDomainIOThreadIDDefFree() in one case. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5d0d436a40..3b4274e037 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17046,7 +17046,8 @@ virDomainDefParseIOThreads(virDomainDef *def, def->iothreadids = g_new0(virDomainIOThreadIDDef *, n); for (i = 0; i < n; i++) { - virDomainIOThreadIDDef *iothrid = NULL; + g_autoptr(virDomainIOThreadIDDef) iothrid = NULL; + if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i]))) return -1; @@ -17054,10 +17055,9 @@ virDomainDefParseIOThreads(virDomainDef *def, virReportError(VIR_ERR_XML_ERROR, _("duplicate iothread id '%u' found"), iothrid->iothread_id); - virDomainIOThreadIDDefFree(iothrid); return -1; } - def->iothreadids[def->niothreadids++] = iothrid; + def->iothreadids[def->niothreadids++] = g_steal_pointer(&iothrid); } return virDomainIOThreadIDDefArrayInit(def, iothreads); -- 2.35.1

Formatting iothreads is currently open coded inside of virDomainDefFormatInternalSetRootName(). While this works, it makes the function needlessly long, especially if the formatting code will expand in near future. Therefore, move it into a separate function. At the same time, make virDomainDefIothreadShouldFormat() accept const domain definition so that the new function can also accept const domain definition. Formatters shouldn't need to change definition. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eec5941089..c60ab4e064 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27569,7 +27569,7 @@ virDomainCpuDefFormat(virBuffer *buf, static bool -virDomainDefIothreadShouldFormat(virDomainDef *def) +virDomainDefIothreadShouldFormat(const virDomainDef *def) { size_t i; @@ -27582,6 +27582,31 @@ virDomainDefIothreadShouldFormat(virDomainDef *def) } +static void +virDomainDefIOThreadsFormat(virBuffer *buf, + const virDomainDef *def) +{ + g_auto(virBuffer) childrenBuf = VIR_BUFFER_INIT_CHILD(buf); + size_t i; + + if (def->niothreadids == 0) + return; + + virBufferAsprintf(buf, "<iothreads>%zu</iothreads>\n", + def->niothreadids); + + if (!virDomainDefIothreadShouldFormat(def)) + return; + + for (i = 0; i < def->niothreadids; i++) { + virBufferAsprintf(&childrenBuf, "<iothread id='%u'/>\n", + def->iothreadids[i]->iothread_id); + } + + virXMLFormatElement(buf, "iothreadids", NULL, &childrenBuf); +} + + static void virDomainIOMMUDefFormat(virBuffer *buf, const virDomainIOMMUDef *iommu) @@ -28227,20 +28252,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (virDomainCpuDefFormat(buf, def) < 0) return -1; - if (def->niothreadids > 0) { - virBufferAsprintf(buf, "<iothreads>%zu</iothreads>\n", - def->niothreadids); - if (virDomainDefIothreadShouldFormat(def)) { - virBufferAddLit(buf, "<iothreadids>\n"); - virBufferAdjustIndent(buf, 2); - for (i = 0; i < def->niothreadids; i++) { - virBufferAsprintf(buf, "<iothread id='%u'/>\n", - def->iothreadids[i]->iothread_id); - } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</iothreadids>\n"); - } - } + virDomainDefIOThreadsFormat(buf, def); if (virDomainCputuneDefFormat(buf, def, flags) < 0) return -1; -- 2.35.1

At least in case of QEMU an IOThread is actually a pool of threads (see iothread_set_aio_context_params() in QEMU's code base). As such, it can have minimal and maximal number of worker threads. Allow setting them in domain XML. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 5 +- src/conf/domain_conf.c | 34 ++++++++++- src/conf/domain_conf.h | 3 + src/conf/schemas/domaincommon.rng | 10 +++ .../iothreads-ids-pool-sizes.xml | 61 +++++++++++++++++++ ...iothreads-ids-pool-sizes.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml create mode 120000 tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 312b605a8b..2aa39b2f63 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -675,7 +675,7 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` <iothread id="2"/> <iothread id="4"/> <iothread id="6"/> - <iothread id="8"/> + <iothread id="8" thread_pool_min="2" thread_pool_max="32"/> </iothreadids> ... </domain> @@ -696,6 +696,9 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` any predefined ``id``. If there are more ``iothreadids`` defined than ``iothreads`` defined for the domain, then the ``iothreads`` value will be adjusted accordingly. :since:`Since 1.2.15` + The element has two optional attributes ``thread_pool_min`` and + ``hread_pool_max`` which allow setting lower and upper boundary for number + of worker threads for given IOThread. :since:`Since 8.5.0` CPU Tuning diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d4b27dbc0..8891289866 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3481,6 +3481,9 @@ virDomainIOThreadIDDefNew(void) { virDomainIOThreadIDDef *def = g_new0(virDomainIOThreadIDDef, 1); + def->thread_pool_min = -1; + def->thread_pool_max = -1; + return def; } @@ -17008,7 +17011,7 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, * * <iothreads>4</iothreads> * <iothreadids> - * <iothread id='1'/> + * <iothread id='1' thread_pool_min="0" thread_pool_max="60"/> * <iothread id='3'/> * <iothread id='5'/> * <iothread id='7'/> @@ -17024,6 +17027,16 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node) &iothrid->iothread_id) < 0) return NULL; + if (virXMLPropInt(node, "thread_pool_min", 10, + VIR_XML_PROP_NONNEGATIVE, + &iothrid->thread_pool_min, -1) < 0) + return NULL; + + if (virXMLPropInt(node, "thread_pool_max", 10, + VIR_XML_PROP_NONNEGATIVE, + &iothrid->thread_pool_max, -1) < 0) + return NULL; + return g_steal_pointer(&iothrid); } @@ -27607,8 +27620,23 @@ virDomainDefIOThreadsFormat(virBuffer *buf, return; for (i = 0; i < def->niothreadids; i++) { - virBufferAsprintf(&childrenBuf, "<iothread id='%u'/>\n", - def->iothreadids[i]->iothread_id); + virDomainIOThreadIDDef *iothread = def->iothreadids[i]; + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&attrBuf, " id='%u'", + iothread->iothread_id); + + if (iothread->thread_pool_min >= 0) { + virBufferAsprintf(&attrBuf, " thread_pool_min='%d'", + iothread->thread_pool_min); + } + + if (iothread->thread_pool_max >= 0) { + virBufferAsprintf(&attrBuf, " thread_pool_max='%d'", + iothread->thread_pool_max); + } + + virXMLFormatElement(&childrenBuf, "iothread", &attrBuf, NULL); } virXMLFormatElement(buf, "iothreadids", NULL, &childrenBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e7e0f24443..2a5001f15c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2646,6 +2646,9 @@ struct _virDomainIOThreadIDDef { virBitmap *cpumask; virDomainThreadSchedParam sched; + + int thread_pool_min; + int thread_pool_max; }; void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def); diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index cc598212a8..7e6310832b 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -829,6 +829,16 @@ <attribute name="id"> <ref name="unsignedInt"/> </attribute> + <optional> + <attribute name="thread_pool_min"> + <ref name="unsignedLong"/> + </attribute> + </optional> + <optional> + <attribute name="thread_pool_max"> + <ref name="unsignedLong"/> + </attribute> + </optional> </element> </zeroOrMore> </element> diff --git a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml new file mode 100644 index 0000000000..0f93d14f12 --- /dev/null +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml @@ -0,0 +1,61 @@ +<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'>6</vcpu> + <iothreads>5</iothreads> + <iothreadids> + <iothread id='2' thread_pool_min='0' thread_pool_max='60'/> + <iothread id='4' thread_pool_min='1' thread_pool_max='1'/> + <iothread id='1'/> + <iothread id='3'/> + <iothread id='5'/> + </iothreadids> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml b/tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml new file mode 120000 index 0000000000..6ed642fe5f --- /dev/null +++ b/tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/iothreads-ids-pool-sizes.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3bd57306cc..b85d5fb757 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -597,6 +597,7 @@ mymain(void) DO_TEST_NOCAPS("smp"); DO_TEST_NOCAPS("iothreads"); DO_TEST_NOCAPS("iothreads-ids"); + DO_TEST_CAPS_LATEST("iothreads-ids-pool-sizes"); DO_TEST_NOCAPS("iothreads-ids-partial"); DO_TEST_NOCAPS("cputune-iothreads"); DO_TEST_NOCAPS("iothreads-disk"); -- 2.35.1

This capability reflects whether QEMU allows setting thread-pool-min and thread-pool-max attributes on iothread object. Since both attributes were introduced in the same commit (v7.0.0-878-g71ad4713cc) and can't exist independently of each other we can stick with one capability covering both of them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d0c8217825..e8c96ffc1f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -676,6 +676,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 430 */ "chardev.qemu-vdagent", /* QEMU_CAPS_CHARDEV_QEMU_VDAGENT */ "display-dbus", /* QEMU_CAPS_DISPLAY_DBUS */ + "iothread.thread-pool-max", /* QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX */ ); @@ -1627,6 +1628,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "calc-dirty-rate/arg-type/mode", QEMU_CAPS_DIRTYRATE_MODE }, { "chardev-add/arg-type/backend/+qemu-vdagent", QEMU_CAPS_CHARDEV_QEMU_VDAGENT }, { "query-display-options/ret-type/+dbus", QEMU_CAPS_DISPLAY_DBUS }, + { "object-add/arg-type/+iothread/thread-pool-max", QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 26cd655db3..9bb81f87c2 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -651,6 +651,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 430 */ QEMU_CAPS_CHARDEV_QEMU_VDAGENT, /* -chardev qemu-vdagent */ QEMU_CAPS_DISPLAY_DBUS, /* -display dbus */ + QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX, /* -object iothread.thread-pool-max */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml index 04ba103688..95d61bc274 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml @@ -243,6 +243,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='display-dbus'/> + <flag name='iothread.thread-pool-max'/> <version>7000050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> -- 2.35.1

On Tue, Jun 07, 2022 at 14:52:52 +0200, Michal Privoznik wrote: You'll also need a check that 'thread_pool_max', if supplied, must be more or equal to 'thread_pool_min', otherwise: $ virsh dumpxml cd | grep thread_pool <iothread id='3' thread_pool_min='4' thread_pool_max='3'/> $ virsh start cd error: Failed to start domain 'cd' error: internal error: process exited while connecting to monitor: 2022-06-07T14:58:52.420554Z qemu-system-x86_64: bad thread-pool-min/thread-pool-max values

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 10 ++++- ...othreads-ids-pool-sizes.x86_64-latest.args | 44 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4f4d8cf25c..2734be7679 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7441,9 +7441,15 @@ qemuBuildIOThreadCommandLine(virCommand *cmd, for (i = 0; i < def->niothreadids; i++) { g_autoptr(virJSONValue) props = NULL; - g_autofree char *alias = g_strdup_printf("iothread%u", def->iothreadids[i]->iothread_id); + const virDomainIOThreadIDDef *iothread = def->iothreadids[i]; + g_autofree char *alias = NULL; - if (qemuMonitorCreateObjectProps(&props, "iothread", alias, NULL) < 0) + alias = g_strdup_printf("iothread%u", iothread->iothread_id); + + if (qemuMonitorCreateObjectProps(&props, "iothread", alias, + "k:thread-pool-min", iothread->thread_pool_min, + "k:thread-pool-max", iothread->thread_pool_max, + NULL) < 0) return -1; if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0) diff --git a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args new file mode 100644 index 0000000000..32517e3d29 --- /dev/null +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args @@ -0,0 +1,44 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine q35,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 6,sockets=6,cores=1,threads=1 \ +-object '{"qom-type":"iothread","id":"iothread2","thread-pool-min":0,"thread-pool-max":60}' \ +-object '{"qom-type":"iothread","id":"iothread4","thread-pool-min":1,"thread-pool-max":1}' \ +-object '{"qom-type":"iothread","id":"iothread1"}' \ +-object '{"qom-type":"iothread","id":"iothread3"}' \ +-object '{"qom-type":"iothread","id":"iothread5"}' \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ +-device '{"driver":"pcie-root-port","port":10,"chassis":3,"id":"pci.3","bus":"pcie.0","addr":"0x1.0x2"}' \ +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci.1","addr":"0x0"}' \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.2","addr":"0x0","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index bffe7aef8a..22e2661793 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2038,6 +2038,7 @@ mymain(void) DO_TEST("iothreads", QEMU_CAPS_OBJECT_IOTHREAD); DO_TEST("iothreads-ids", QEMU_CAPS_OBJECT_IOTHREAD); DO_TEST("iothreads-ids-partial", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST_CAPS_LATEST("iothreads-ids-pool-sizes"); DO_TEST_FAILURE_NOCAPS("iothreads-nocap"); DO_TEST("iothreads-disk", QEMU_CAPS_OBJECT_IOTHREAD); DO_TEST("iothreads-disk-virtio-ccw", QEMU_CAPS_OBJECT_IOTHREAD, -- 2.35.1

Our public API offers virDomainSetIOThreadParams() function which allows users to set various aspects of IOThreads. Introduce two new typed parameters: VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX which will allow users to modify the thread-pool-min and thread-pool-max attributes of an iothread. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 24846046aa..b0020d247e 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2493,6 +2493,28 @@ int virDomainDelIOThread(virDomainPtr domain, */ # define VIR_DOMAIN_IOTHREAD_POLL_SHRINK "poll_shrink" +/** + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN: + * + * Sets the lower bound for thread pool size. A value of -1 disables this bound + * leaving hypervisor use its default value. Accepted type is + * VIR_TYPED_PARAM_INT. + * + * Since: 8.5.0 + */ +# define VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN "thread_pool_min" + +/** + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX: + * + * Sets the upper bound for thread pool size. A value of -1 disables this bound + * leaving hypervisor use its default value. Accepted type is + * VIR_TYPED_PARAM_INT. + * + * Since: 8.5.0 + */ +# define VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX "thread_pool_max" + int virDomainSetIOThreadParams(virDomainPtr domain, unsigned int iothread_id, virTypedParameterPtr params, -- 2.35.1

Introduced in previous commit, QEMU driver needs to be taught how to set VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX parameters on given IOThread. Fortunately, this is fairly trivial to do and since these two parameters are exposed in domain XML too the update of inactive XML can be wired up too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 2 ++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c6645ed89..1770a1882b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5318,6 +5318,26 @@ qemuDomainHotplugModIOThread(virQEMUDriver *driver, } +static int +qemuDomainHotplugModIOThreadIDDef(virDomainIOThreadIDDef *def, + qemuMonitorIOThreadInfo mondef) +{ + /* These have no representation in domain XML */ + if (mondef.set_poll_grow || + mondef.set_poll_max_ns || + mondef.set_poll_shrink) + return -1; + + if (mondef.set_thread_pool_min) + def->thread_pool_min = mondef.thread_pool_min; + + if (mondef.set_thread_pool_max) + def->thread_pool_max = mondef.thread_pool_max; + + return 0; +} + + static int qemuDomainHotplugDelIOThread(virQEMUDriver *driver, virDomainObj *vm, @@ -5425,6 +5445,10 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, VIR_TYPED_PARAM_UINT, VIR_DOMAIN_IOTHREAD_POLL_SHRINK, VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN, + VIR_TYPED_PARAM_INT, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX, + VIR_TYPED_PARAM_INT, NULL) < 0) return -1; @@ -5449,6 +5473,20 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, if (rc == 1) iothread->set_poll_shrink = true; + if ((rc = virTypedParamsGetInt(params, nparams, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN, + &iothread->thread_pool_min)) < 0) + return -1; + if (rc == 1) + iothread->set_thread_pool_min = true; + + if ((rc = virTypedParamsGetInt(params, nparams, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX, + &iothread->thread_pool_max)) < 0) + return -1; + if (rc == 1) + iothread->set_thread_pool_max = true; + if (iothread->set_poll_max_ns && iothread->poll_max_ns > INT_MAX) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("poll-max-ns (%llu) must be less than or equal to %d"), @@ -5491,6 +5529,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver, qemuDomainObjPrivate *priv; virDomainDef *def; virDomainDef *persistentDef; + virDomainIOThreadIDDef *iothreaddef = NULL; int ret = -1; cfg = virQEMUDriverGetConfig(driver); @@ -5530,7 +5569,9 @@ qemuDomainChgIOThread(virQEMUDriver *driver, break; case VIR_DOMAIN_IOTHREAD_ACTION_MOD: - if (!(virDomainIOThreadIDFind(def, iothread.iothread_id))) { + iothreaddef = virDomainIOThreadIDFind(def, iothread.iothread_id); + + if (!iothreaddef) { virReportError(VIR_ERR_INVALID_ARG, _("cannot find IOThread '%u' in iothreadids"), iothread.iothread_id); @@ -5540,6 +5581,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver, if (qemuDomainHotplugModIOThread(driver, vm, iothread) < 0) goto endjob; + qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread); break; } @@ -5567,10 +5609,20 @@ qemuDomainChgIOThread(virQEMUDriver *driver, break; case VIR_DOMAIN_IOTHREAD_ACTION_MOD: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("configuring persistent polling values is " - "not supported")); - goto endjob; + iothreaddef = virDomainIOThreadIDFind(def, iothread.iothread_id); + + if (!iothreaddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in iothreadids"), + iothread.iothread_id); + goto endjob; + } + + if (qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread) < 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("configuring persistent polling values is not supported")); + goto endjob; + } break; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index be341d5196..997d8737f8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1308,9 +1308,13 @@ struct _qemuMonitorIOThreadInfo { unsigned long long poll_max_ns; unsigned int poll_grow; unsigned int poll_shrink; + int thread_pool_min; + int thread_pool_max; bool set_poll_max_ns; bool set_poll_grow; bool set_poll_shrink; + bool set_thread_pool_min; + bool set_thread_pool_max; }; int qemuMonitorGetIOThreads(qemuMonitor *mon, qemuMonitorIOThreadInfo ***iothreads, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dc05dfd047..e8fe1eceae 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7431,6 +7431,8 @@ qemuMonitorJSONSetIOThread(qemuMonitor *mon, VIR_IOTHREAD_SET_PROP("poll-max-ns", poll_max_ns); VIR_IOTHREAD_SET_PROP("poll-grow", poll_grow); VIR_IOTHREAD_SET_PROP("poll-shrink", poll_shrink); + VIR_IOTHREAD_SET_PROP("thread-pool-min", thread_pool_min); + VIR_IOTHREAD_SET_PROP("thread-pool-max", thread_pool_max); #undef VIR_IOTHREAD_SET_PROP -- 2.35.1

On Tue, Jun 07, 2022 at 17:06:50 +0200, Peter Krempa wrote: So the second happens only if you pick a minimum which is greater than the maximum auto-picked by qemu. Also since VIR_IOTHREAD_SET_PROP is a discrete command the following compound operation doesn't work properly: virsh iothreadset cd --id 3 --thread-pool-max 1234 --thread-pool-min 123 error: internal error: unable to execute QEMU command 'qom-set': bad thread-pool-min/thread-pool-max values As it would IMO be a bit too overkill to try to order them properly when changing the size I suggest you ask users to do it sequentially in the docs.

Since virsh implements a wrapper over virDomainSetIOThreadParams() (command iothreadset) let's wire up new typed parameters there too. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/manpages/virsh.rst | 7 ++++++- tools/virsh-domain.c | 24 +++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 965b89c99d..8c066c9176 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2993,7 +2993,8 @@ iothreadset :: iothreadset domain iothread_id [[--poll-max-ns ns] [--poll-grow factor] - [--poll-shrink divisor]] + [--poll-shrink divisor] [--thread-pool-min value] + [--thread-pool-max value]] [[--config] [--live] | [--current]] Modifies an existing iothread of the domain using the specified @@ -3010,6 +3011,10 @@ for a running guest. Saving, destroying, stopping, etc. the guest will result in the polling values returning to hypervisor defaults at the next start, restore, etc. +The *--thread-pool-min* and *--thread-pool-max* options then set lower and +upper bound, respectively of number of threads in worker pool of given +iothread. + If *--live* is specified, affect a running guest. If the guest is not running an error is returned. If *--current* is specified or *--live* is not specified, then handle diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bac4cc0fb6..53e85ddfac 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7782,7 +7782,15 @@ static const vshCmdOptDef opts_iothreadset[] = { }, {.name = "poll-shrink", .type = VSH_OT_INT, - .help = N_("set the value for reduction of the IOThread polling time ") + .help = N_("set the value for reduction of the IOThread polling time") + }, + {.name = "thread-pool-min", + .type = VSH_OT_INT, + .help = N_("lower boundary for worker thread pool") + }, + {.name = "thread-pool-max", + .type = VSH_OT_INT, + .help = N_("upper boundary for worker thread pool") }, VIRSH_COMMON_OPT_DOMAIN_LIVE, VIRSH_COMMON_OPT_DOMAIN_CURRENT, @@ -7802,6 +7810,7 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd) int maxparams = 0; unsigned long long poll_max; unsigned int poll_val; + int thread_val; int rc; if (live) @@ -7839,6 +7848,19 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd) #undef VSH_IOTHREAD_SET_UINT_PARAMS +#define VSH_IOTHREAD_SET_INT_PARAMS(opt, param) \ + thread_val = -1; \ + if ((rc = vshCommandOptInt(ctl, cmd, opt, &thread_val)) < 0) \ + goto cleanup; \ + if (rc > 0 && \ + virTypedParamsAddInt(&params, &nparams, &maxparams, \ + param, thread_val) < 0) \ + goto save_error; + + VSH_IOTHREAD_SET_INT_PARAMS("thread-pool-min", VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN) + VSH_IOTHREAD_SET_INT_PARAMS("thread-pool-max", VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX) +#undef VSH_IOTHREAD_SET_INT_PARAMS + if (virDomainSetIOThreadParams(dom, id, params, nparams, flags) < 0) goto cleanup; -- 2.35.1

As of v7.0.0-877-g70ac26b9e5 QEMU exposes its main event loop as an QMP object. In the very next commit (v7.0.0-878-g71ad4713cc) it was extended for thread-pool-min and thread-pool-max attributes. Expose them under new <mainloop/> element. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 9 +++ src/conf/domain_conf.c | 63 +++++++++++++++++++ src/conf/domain_conf.h | 8 +++ src/conf/schemas/domaincommon.rng | 15 +++++ src/conf/virconftypes.h | 2 + .../iothreads-ids-pool-sizes.xml | 1 + 6 files changed, 98 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2aa39b2f63..22eb540ba3 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -677,6 +677,7 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` <iothread id="6"/> <iothread id="8" thread_pool_min="2" thread_pool_max="32"/> </iothreadids> + <defaultiothread thread_pool_min="8" thread_pool_max="16"> ... </domain> @@ -699,6 +700,14 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` The element has two optional attributes ``thread_pool_min`` and ``hread_pool_max`` which allow setting lower and upper boundary for number of worker threads for given IOThread. :since:`Since 8.5.0` +``defaultiothread`` + The element then can have ``thread_pool_min`` and/or ``thread_pool_max`` + attributes, which control the lower and upper boundary for number of worker + threads for the emulator. Emulator might be multithreaded and spawn so + called worker threads on demand. In general neither of these attributes + should be set (leaving the emulator use its own default values), unless the + emulator runs in a real time workload and thus can't afford unpredictability + of time it takes to spawn new worker threads. :since:`Since 8.5.0` CPU Tuning diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8891289866..b1e7c2ef22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3824,6 +3824,8 @@ void virDomainDefFree(virDomainDef *def) virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids); + g_free(def->defaultIOThread); + virBitmapFree(def->cputune.emulatorpin); g_free(def->cputune.emulatorsched); @@ -17016,6 +17018,7 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, * <iothread id='5'/> * <iothread id='7'/> * </iothreadids> + * <defaultiothread thread_pool_min="8" thread_pool_max="8"/> */ static virDomainIOThreadIDDef * virDomainIOThreadIDDefParseXML(xmlNodePtr node) @@ -17041,6 +17044,38 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node) } +static int +virDomainDefaultIOThreadDefParse(virDomainDef *def, + xmlXPathContextPtr ctxt) +{ + xmlNodePtr node = NULL; + g_autofree virDomainDefaultIOThreadDef *thrd = NULL; + + node = virXPathNode("./defaultiothread", ctxt); + if (!node) + return 0; + + thrd = g_new0(virDomainDefaultIOThreadDef, 1); + + if (virXMLPropInt(node, "thread_pool_min", 10, + VIR_XML_PROP_NONNEGATIVE, + &thrd->thread_pool_min, -1) < 0) + return -1; + + if (virXMLPropInt(node, "thread_pool_max", 10, + VIR_XML_PROP_NONNEGATIVE, + &thrd->thread_pool_max, -1) < 0) + return -1; + + if (thrd->thread_pool_min == -1 && + thrd->thread_pool_max == -1) + return 0; + + def->defaultIOThread = g_steal_pointer(&thrd); + return 0; +} + + static int virDomainDefParseIOThreads(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17058,6 +17093,9 @@ virDomainDefParseIOThreads(virDomainDef *def, return -1; } + if (virDomainDefaultIOThreadDefParse(def, ctxt) < 0) + return -1; + /* Extract any iothread id's defined */ if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes)) < 0) return -1; @@ -27603,6 +27641,29 @@ virDomainDefIothreadShouldFormat(const virDomainDef *def) } +static void +virDomainDefaultIOThreadDefFormat(virBuffer *buf, + const virDomainDef *def) +{ + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + + if (!def->defaultIOThread) + return; + + if (def->defaultIOThread->thread_pool_min >= 0) { + virBufferAsprintf(&attrBuf, " thread_pool_min='%d'", + def->defaultIOThread->thread_pool_min); + } + + if (def->defaultIOThread->thread_pool_max >= 0) { + virBufferAsprintf(&attrBuf, " thread_pool_max='%d'", + def->defaultIOThread->thread_pool_max); + } + + virXMLFormatElement(buf, "defaultiothread", &attrBuf, NULL); +} + + static void virDomainDefIOThreadsFormat(virBuffer *buf, const virDomainDef *def) @@ -27640,6 +27701,8 @@ virDomainDefIOThreadsFormat(virBuffer *buf, } virXMLFormatElement(buf, "iothreadids", NULL, &childrenBuf); + + virDomainDefaultIOThreadDefFormat(buf, def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2a5001f15c..98b8ce703c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2655,6 +2655,12 @@ void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainIOThreadIDDef, virDomainIOThreadIDDefFree); +struct _virDomainDefaultIOThreadDef { + int thread_pool_min; + int thread_pool_max; +}; + + struct _virDomainCputune { unsigned long long shares; bool sharesSpecified; @@ -2863,6 +2869,8 @@ struct _virDomainDef { size_t niothreadids; virDomainIOThreadIDDef **iothreadids; + virDomainDefaultIOThreadDef *defaultIOThread; + virDomainCputune cputune; virDomainResctrlDef **resctrls; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 7e6310832b..e64900d0cf 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -844,6 +844,21 @@ </element> </optional> + <optional> + <element name="defaultiothread"> + <optional> + <attribute name="thread_pool_min"> + <ref name="unsignedLong"/> + </attribute> + </optional> + <optional> + <attribute name="thread_pool_max"> + <ref name="unsignedLong"/> + </attribute> + </optional> + </element> + </optional> + <optional> <ref name="blkiotune"/> </optional> diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 21420ba8ea..c3f1c5fa01 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -142,6 +142,8 @@ typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef; +typedef struct _virDomainDefaultIOThreadDef virDomainDefaultIOThreadDef; + typedef struct _virDomainIdMapDef virDomainIdMapDef; typedef struct _virDomainIdMapEntry virDomainIdMapEntry; diff --git a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml index 0f93d14f12..4cebdfada9 100644 --- a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml @@ -12,6 +12,7 @@ <iothread id='3'/> <iothread id='5'/> </iothreadids> + <defaultiothread thread_pool_min='8' thread_pool_max='16'/> <os> <type arch='x86_64' machine='q35'>hvm</type> <boot dev='hd'/> -- 2.35.1

Since the main-loop and iothread classes are derived from the same class (EventLoopBaseClass) we don't need new capability and can use QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX directly to check whether QEMU's capable of setting worker pool size. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_validate.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7d11ae2c92..c2495fb6b8 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -401,6 +401,15 @@ qemuValidateDomainDefIOThreads(const virDomainDef *def, } } + if (def->defaultIOThread && + ((def->defaultIOThread->thread_pool_min >= 0 || + def->defaultIOThread->thread_pool_max >= 0) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pool_min and pool_max is not supported by this QEMU binary")); + return -1; + } + return 0; } -- 2.35.1

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2059511 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 16 +++++++++++++--- .../iothreads-ids-pool-sizes.x86_64-latest.args | 1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2734be7679..be20053c0d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7436,9 +7436,6 @@ qemuBuildIOThreadCommandLine(virCommand *cmd, { size_t i; - if (def->niothreadids == 0) - return 0; - for (i = 0; i < def->niothreadids; i++) { g_autoptr(virJSONValue) props = NULL; const virDomainIOThreadIDDef *iothread = def->iothreadids[i]; @@ -7456,6 +7453,19 @@ qemuBuildIOThreadCommandLine(virCommand *cmd, return -1; } + if (def->defaultIOThread) { + g_autoptr(virJSONValue) props = NULL; + + if (qemuMonitorCreateObjectProps(&props, "main-loop", "main-loop", + "k:thread-pool-min", def->defaultIOThread->thread_pool_min, + "k:thread-pool-max", def->defaultIOThread->thread_pool_max, + NULL) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args index 32517e3d29..12747b51f0 100644 --- a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args @@ -22,6 +22,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -object '{"qom-type":"iothread","id":"iothread1"}' \ -object '{"qom-type":"iothread","id":"iothread3"}' \ -object '{"qom-type":"iothread","id":"iothread5"}' \ +-object '{"qom-type":"main-loop","id":"main-loop","thread-pool-min":8,"thread-pool-max":16}' \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -display none \ -no-user-config \ -- 2.35.1