[PATCH v3 00/15] qemu: Allow setting EventLoopBaseProperties
v3 of: https://listman.redhat.com/archives/libvir-list/2022-June/232288.html diff to v2: - More validation checks (e.g. max != 0, min <= max), - Disallow -1 for virDomainSetIOThreadParams on a live VM, - Extended docs, - Reworded some commit messages. Michal Prívozník (15): virml: Introduce VIR_XML_PROP_NONNEGATIVE flag virDomainDefParseIOThreads: Use g_autoptr() for @iothrid virDomainIOThreadIDDefArrayInit: Decrease scope of @iothrid conf: Move iothread formatter into a separate function conf: Introduce allocator for virDomainIOThreadIDDef conf: Introduce thread_pool_min and thread_pool_max attributes to IOThread qemu: Introduce QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX qemu_validate: Check if QEMU's capable of setting iothread pool size qemu: Generate command line for IOThread pool size include: Introduce typed params for virDomainSetIOThreadParams wrt pool size qemu: Wire up new virDomainSetIOThreadParams parameters virsh: Wire up new virDomainSetIOThreadParams parameters conf: Introduce <defaultiothread/> qemu_validate: Check if QEMU's capable of setting <defaultiothread/> pool size qemu: Generate command line for <defaultiothread/> pool size docs/formatdomain.rst | 17 +- docs/manpages/virsh.rst | 13 +- include/libvirt/libvirt-domain.h | 28 +++ src/conf/domain_conf.c | 162 +++++++++++++++--- src/conf/domain_conf.h | 11 ++ src/conf/domain_validate.c | 45 +++++ src/conf/schemas/domaincommon.rng | 25 +++ src/conf/virconftypes.h | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 26 ++- src/qemu/qemu_driver.c | 140 ++++++++++++++- src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 2 + src/qemu/qemu_validate.c | 36 ++++ src/util/virxml.c | 7 + src/util/virxml.h | 3 + .../caps_7.1.0.x86_64.xml | 1 + ...othreads-ids-pool-sizes.x86_64-latest.args | 45 +++++ .../iothreads-ids-pool-sizes.xml | 62 +++++++ tests/qemuxml2argvtest.c | 1 + ...iothreads-ids-pool-sizes.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + tools/virsh-domain.c | 24 ++- 24 files changed, 621 insertions(+), 38 deletions(-) create mode 100644 tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml create mode 120000 tests/qemuxml2xmloutdata/iothreads-ids-pool-sizes.x86_64-latest.xml -- 2.35.1
For easier attribute parsing we have virXMLProp*() family of functions. These accept flags through which a caller can pose some conditions onto the attribute value, for instance: VIR_XML_PROP_NONZERO when the attribute may not be zero, etc. What we are missing is VIR_XML_PROP_NONNEGATIVE when the attribute value may be non-negative. Obviously, this flag makes sense only for some members of the virXMLProp*() family. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 7 +++++++ src/util/virxml.h | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/util/virxml.c b/src/util/virxml.c index 12b2ef635a..d6e2e5dd91 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -643,6 +643,13 @@ virXMLPropInt(xmlNodePtr node, return -1; } + if ((flags & VIR_XML_PROP_NONNEGATIVE) && (val < 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected non-negative value"), + name, node->name, tmp); + return -1; + } + if ((flags & VIR_XML_PROP_NONZERO) && (val == 0)) { virReportError(VIR_ERR_XML_ERROR, _("Invalid value for attribute '%s' in element '%s': Zero is not permitted"), diff --git a/src/util/virxml.h b/src/util/virxml.h index 5d49056bc7..539228a9ba 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -38,6 +38,9 @@ typedef enum { VIR_XML_PROP_NONE = 0, VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ VIR_XML_PROP_NONZERO = 1 << 1, /* Attribute may not be zero */ + VIR_XML_PROP_NONNEGATIVE = 1 << 2, /* Attribute may not be negative, makes + sense only for some virXMLProp*() + functions. */ } virXMLPropFlags; -- 2.35.1
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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- 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 9f75012f1f..cc1f765ca9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17047,7 +17047,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; @@ -17055,10 +17056,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
In virDomainIOThreadIDDefArrayInit() the variable @iothrid is used only inside a loop but is declared for whole function. Bring the variable into the loop so that it's obvious that the variable is not used elsewhere. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc1f765ca9..22b58e3212 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3509,7 +3509,6 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def, { size_t i; ssize_t nxt = -1; - virDomainIOThreadIDDef *iothrid = NULL; g_autoptr(virBitmap) thrmap = NULL; /* Same value (either 0 or some number), then we have none to fill in or @@ -3534,6 +3533,8 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def, /* Populate iothreadids[] using the set bit number from thrmap */ while (def->niothreadids < iothreads) { + g_autoptr(virDomainIOThreadIDDef) iothrid = NULL; + if ((nxt = virBitmapNextSetBit(thrmap, nxt)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to populate iothreadids")); @@ -3542,7 +3543,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def, iothrid = g_new0(virDomainIOThreadIDDef, 1); iothrid->iothread_id = nxt; iothrid->autofill = true; - def->iothreadids[def->niothreadids++] = iothrid; + def->iothreadids[def->niothreadids++] = g_steal_pointer(&iothrid); } return 0; -- 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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- 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 22b58e3212..f7b86be5fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27570,7 +27570,7 @@ virDomainCpuDefFormat(virBuffer *buf, static bool -virDomainDefIothreadShouldFormat(virDomainDef *def) +virDomainDefIothreadShouldFormat(const virDomainDef *def) { size_t i; @@ -27583,6 +27583,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) @@ -28228,20 +28253,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
So far, iothread configuration structure (virDomainIOThreadIDDef) is allocated by plain g_new0(). This is perfectly okay because all members of the struct default to value 0 anyway. But soon this is going to change. Therefore, replace those g_new0() with a function so that the default value can be set consistently in one place. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7b86be5fc..f3923cf2ba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3477,6 +3477,15 @@ virDomainIOThreadIDArrayHasPin(virDomainDef *def) } +static virDomainIOThreadIDDef * +virDomainIOThreadIDDefNew(void) +{ + virDomainIOThreadIDDef *def = g_new0(virDomainIOThreadIDDef, 1); + + return def; +} + + void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def) { @@ -3540,7 +3549,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def, _("failed to populate iothreadids")); return -1; } - iothrid = g_new0(virDomainIOThreadIDDef, 1); + iothrid = virDomainIOThreadIDDefNew(); iothrid->iothread_id = nxt; iothrid->autofill = true; def->iothreadids[def->niothreadids++] = g_steal_pointer(&iothrid); @@ -17009,7 +17018,7 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, static virDomainIOThreadIDDef * virDomainIOThreadIDDefParseXML(xmlNodePtr node) { - g_autoptr(virDomainIOThreadIDDef) iothrid = g_new0(virDomainIOThreadIDDef, 1); + g_autoptr(virDomainIOThreadIDDef) iothrid = virDomainIOThreadIDDefNew(); if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, @@ -22932,8 +22941,7 @@ virDomainIOThreadIDAdd(virDomainDef *def, { virDomainIOThreadIDDef *iothrid = NULL; - iothrid = g_new0(virDomainIOThreadIDDef, 1); - + iothrid = virDomainIOThreadIDDefNew(); iothrid->iothread_id = iothread_id; VIR_APPEND_ELEMENT_COPY(def->iothreadids, def->niothreadids, iothrid); -- 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@redhat.com> --- docs/formatdomain.rst | 6 +- src/conf/domain_conf.c | 34 ++++++++++- src/conf/domain_conf.h | 3 + src/conf/domain_validate.c | 28 +++++++++ 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 + 8 files changed, 140 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..07837220ed 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,10 @@ 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 + ``thread_pool_max`` which allow setting lower and upper boundary for number + of worker threads for given IOThread. While the former can be value of zero, + the latter can't. :since:`Since 8.5.0` CPU Tuning diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f3923cf2ba..ea8061dc3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3482,6 +3482,9 @@ virDomainIOThreadIDDefNew(void) { virDomainIOThreadIDDef *def = g_new0(virDomainIOThreadIDDef, 1); + def->thread_pool_min = -1; + def->thread_pool_max = -1; + return def; } @@ -17009,7 +17012,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'/> @@ -17025,6 +17028,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); } @@ -27608,8 +27621,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]; + g_auto(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 1efdb439ac..035edc0710 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/domain_validate.c b/src/conf/domain_validate.c index f3910f08a4..3ada739ea7 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1716,6 +1716,31 @@ virDomainDefFSValidate(const virDomainDef *def) } +static int +virDomainDefValidateIOThreads(const virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->niothreadids; i++) { + virDomainIOThreadIDDef *iothread = def->iothreadids[i]; + + if (iothread->thread_pool_max == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_max must be a positive integer")); + return -1; + } + + if (iothread->thread_pool_min > iothread->thread_pool_max) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_min must be smaller or equal to thread_pool_max")); + return -1; + } + } + + return 0; +} + + static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOption *xmlopt) @@ -1771,6 +1796,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefFSValidate(def) < 0) return -1; + if (virDomainDefValidateIOThreads(def) < 0) + return -1; + return 0; } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index cc598212a8..fad70d3857 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="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="thread_pool_max"> + <ref name="unsignedInt"/> + </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
On Wed, Jun 08, 2022 at 15:43:00 +0200, Michal Privoznik wrote:
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@redhat.com> --- docs/formatdomain.rst | 6 +- src/conf/domain_conf.c | 34 ++++++++++- src/conf/domain_conf.h | 3 + src/conf/domain_validate.c | 28 +++++++++ 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 + 8 files changed, 140 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
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- 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
Now that we have a capability that reflects whether QEMU is capable of setting iothread pool size, let's introduce a validator check to make sure users are not trying to use this feature with QEMU that doesn't support it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9b6245e6d7..7d11ae2c92 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -384,6 +384,27 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver, } +static int +qemuValidateDomainDefIOThreads(const virDomainDef *def, + virQEMUCaps *qemuCaps) +{ + size_t i; + + for (i = 0; i < def->niothreadids; i++) { + virDomainIOThreadIDDef *iothread = def->iothreadids[i]; + + if ((iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_min and thread_pool_max is not supported by this QEMU binary")); + return -1; + } + } + + return 0; +} + + static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCaps *qemuCaps) @@ -1168,6 +1189,9 @@ qemuValidateDomainDef(const virDomainDef *def, if (qemuDomainDefValidateMemoryHotplug(def, NULL) < 0) return -1; + if (qemuValidateDomainDefIOThreads(def, qemuCaps) < 0) + return -1; + if (qemuValidateDomainDefClockTimers(def, qemuCaps) < 0) return -1; -- 2.35.1
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- 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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2aec69bc54..1ea3284e63 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2499,6 +2499,34 @@ 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, though this value is not accepted + * for running domains. Due to internal implementation it's recommended to set + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX + * separately. 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, though this value is not accepted + * for running domains. Since the upper band has to be equal to or greater than + * lower bound value of 0 is not accepted. Due to internal implementation it's + * recommended to set VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX separately. 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
On Wed, Jun 08, 2022 at 03:43:04PM +0200, Michal Privoznik wrote:
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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2aec69bc54..1ea3284e63 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2499,6 +2499,34 @@ 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, though this value is not accepted + * for running domains. Due to internal implementation it's recommended to set + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX + * separately. Accepted type is VIR_TYPED_PARAM_INT.
What's the story with this comment about setting pool-min and pool-max separately ? This feels like a impl detail that should never be exposed in the API. If we need to set them separately with QEMU, then the QEMU driver should make separate QMP calls to set them as needed. The app should never have to care about this. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 6/29/22 16:19, Daniel P. Berrangé wrote:
On Wed, Jun 08, 2022 at 03:43:04PM +0200, Michal Privoznik wrote:
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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2aec69bc54..1ea3284e63 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2499,6 +2499,34 @@ 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, though this value is not accepted + * for running domains. Due to internal implementation it's recommended to set + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX + * separately. Accepted type is VIR_TYPED_PARAM_INT.
What's the story with this comment about setting pool-min and pool-max separately ?
This feels like a impl detail that should never be exposed in the API.
If we need to set them separately with QEMU, then the QEMU driver should make separate QMP calls to set them as needed. The app should never have to care about this.
The story is that min and max values are set in separate monitor commands (qom-set). So depending on the current values, min has to be set before max or vice verca. So we would need a logic that gets the current values and acts accordingly. While I agree it's not user friendly, we can implement the logic later. Or do you want to make it before the release? Michal
On Wed, Jun 29, 2022 at 05:08:01PM +0200, Michal Prívozník wrote:
On 6/29/22 16:19, Daniel P. Berrangé wrote:
On Wed, Jun 08, 2022 at 03:43:04PM +0200, Michal Privoznik wrote:
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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2aec69bc54..1ea3284e63 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2499,6 +2499,34 @@ 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, though this value is not accepted + * for running domains. Due to internal implementation it's recommended to set + * VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN and VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX + * separately. Accepted type is VIR_TYPED_PARAM_INT.
What's the story with this comment about setting pool-min and pool-max separately ?
This feels like a impl detail that should never be exposed in the API.
If we need to set them separately with QEMU, then the QEMU driver should make separate QMP calls to set them as needed. The app should never have to care about this.
The story is that min and max values are set in separate monitor commands (qom-set). So depending on the current values, min has to be set before max or vice verca. So we would need a logic that gets the current values and acts accordingly. While I agree it's not user friendly, we can implement the logic later. Or do you want to make it before the release?
I'd rather we at least removed this sentence from the public API docs, and simply treat it as a pending bug fix for next release. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 6/29/22 17:12, Daniel P. Berrangé wrote:
I'd rather we at least removed this sentence from the public API docs, and simply treat it as a pending bug fix for next release.
Yep, makes sense. I've posted a patch here: https://listman.redhat.com/archives/libvir-list/2022-June/232653.html and filled issue to track the bug here: https://gitlab.com/libvirt/libvirt/-/issues/339 Michal
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@redhat.com> --- src/qemu/qemu_driver.c | 140 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 2 + 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee1adb0300..ded34e97cd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5323,6 +5323,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, @@ -5430,6 +5450,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; @@ -5454,6 +5478,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"), @@ -5475,6 +5513,78 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, return -1; } + if (iothread->set_thread_pool_min && iothread->thread_pool_min < -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("thread_pool_min (%d) must be equal to or greater than -1"), + iothread->thread_pool_min); + return -1; + } + + if (iothread->set_thread_pool_max && + (iothread->thread_pool_max < -1 || iothread->thread_pool_max == 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("thread_pool_max (%d) must be a positive number or -1"), + iothread->thread_pool_max); + return -1; + } + + return 0; +} + + +/** + * qemuDomainIOThreadValidate: + * iothreaddef: IOThread definition in domain XML + * iothread: new values to set + * live: whether this is update of active domain + * + * Validate that changes to be made to an IOThread (as expressed by @iothread) + * are consistent with the current state of the IOThread (@iothreaddef). + * For instance, that thread_pool_min won't end up greater than thread_pool_max. + * + * Returns: 0 on success, + * -1 on error, with error message reported. + */ +static int +qemuDomainIOThreadValidate(virDomainIOThreadIDDef *iothreaddef, + qemuMonitorIOThreadInfo iothread, + bool live) +{ + int thread_pool_min = iothreaddef->thread_pool_min; + int thread_pool_max = iothreaddef->thread_pool_max; + + /* For live change we don't have a way to let QEMU return to its + * defaults. Therefore, deny setting -1. */ + + if (iothread.set_thread_pool_min) { + if (live && iothread.thread_pool_min < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("thread_pool_min (%d) must be equal to or greater than 0 for live change"), + iothread.thread_pool_min); + return -1; + } + + thread_pool_min = iothread.thread_pool_min; + } + + if (iothread.set_thread_pool_max) { + if (live && iothread.thread_pool_max < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("thread_pool_max (%d) must be equal to or greater than 0 for live change"), + iothread.thread_pool_max); + return -1; + } + + thread_pool_max = iothread.thread_pool_max; + } + + if (thread_pool_min > thread_pool_max) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("thread_pool_min (%d) can't be greater than thread_pool_max (%d)"), + thread_pool_min, thread_pool_max); + return -1; + } + return 0; } @@ -5496,6 +5606,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver, qemuDomainObjPrivate *priv; virDomainDef *def; virDomainDef *persistentDef; + virDomainIOThreadIDDef *iothreaddef = NULL; int ret = -1; cfg = virQEMUDriverGetConfig(driver); @@ -5535,16 +5646,22 @@ 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); goto endjob; } + if (qemuDomainIOThreadValidate(iothreaddef, iothread, true) < 0) + goto endjob; + if (qemuDomainHotplugModIOThread(driver, vm, iothread) < 0) goto endjob; + qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread); break; } @@ -5572,10 +5689,23 @@ 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(persistentDef, iothread.iothread_id); + + if (!iothreaddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in iothreadids"), + iothread.iothread_id); + goto endjob; + } + + if (qemuDomainIOThreadValidate(iothreaddef, iothread, false) < 0) + 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 91f2d0941c..06822d6642 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1314,9 +1314,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 8b81a07429..6b3acab0d2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7447,6 +7447,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 Wed, Jun 08, 2022 at 15:43:05 +0200, Michal Privoznik wrote:
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@redhat.com> --- src/qemu/qemu_driver.c | 140 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 2 + 3 files changed, 141 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since virsh implements a wrapper over virDomainSetIOThreadParams() (command iothreadset) let's wire up new typed parameters there too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/manpages/virsh.rst | 13 ++++++++++++- tools/virsh-domain.c | 24 +++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index faac996536..580245adb1 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2999,7 +2999,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 @@ -3016,6 +3017,16 @@ 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. For changes to an inactive configuration -1 can be specified to +remove corresponding boundary from the domain configuration. For changes to a +running guest it's recommended to set the upper boundary first +(*--thread-pool-max*) and only after that set the lower boundary +(*--thread-pool-min*). It is allowed for the lower boundary to be the same as +the upper boundary, however it's not allowed for the upper boundary to be value +of zero. + 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 cfdaac1942..06ed6ba9cf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7796,7 +7796,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, @@ -7816,6 +7824,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) @@ -7853,6 +7862,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(¶ms, &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 (nparams == 0) { vshError(ctl, _("Not enough arguments passed, nothing to set")); goto cleanup; -- 2.35.1
On Wed, Jun 08, 2022 at 15:43:06 +0200, Michal Privoznik wrote:
Since virsh implements a wrapper over virDomainSetIOThreadParams() (command iothreadset) let's wire up new typed parameters there too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/manpages/virsh.rst | 13 ++++++++++++- tools/virsh-domain.c | 24 +++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As of v7.0.0-877-g70ac26b9e5 QEMU exposes its default event loop for devices with no IOThread assigned 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 <defaultiothread/> element. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 11 ++++ src/conf/domain_conf.c | 63 +++++++++++++++++++ src/conf/domain_conf.h | 8 +++ src/conf/domain_validate.c | 37 ++++++++--- src/conf/schemas/domaincommon.rng | 15 +++++ src/conf/virconftypes.h | 2 + .../iothreads-ids-pool-sizes.xml | 1 + 7 files changed, 127 insertions(+), 10 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 07837220ed..7da625380c 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> @@ -700,6 +701,16 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` ``thread_pool_max`` which allow setting lower and upper boundary for number of worker threads for given IOThread. While the former can be value of zero, the latter can't. :since:`Since 8.5.0` +``defaultiothread`` + This element represents the default event loop within hypervisor, where I/O + requests from devices not assigned to a specific IOThread are processed. + 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 of the default event loop. 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 ea8061dc3d..761c3f4d87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3825,6 +3825,8 @@ void virDomainDefFree(virDomainDef *def) virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids); + g_free(def->defaultIOThread); + virBitmapFree(def->cputune.emulatorpin); g_free(def->cputune.emulatorsched); @@ -17017,6 +17019,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) @@ -17042,6 +17045,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) @@ -17059,6 +17094,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; @@ -27604,6 +27642,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) @@ -27641,6 +27702,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 035edc0710..da9e281214 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/domain_validate.c b/src/conf/domain_validate.c index 3ada739ea7..1e18826bc1 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1716,6 +1716,26 @@ virDomainDefFSValidate(const virDomainDef *def) } +static int +virDomainDefValidateIOThreadsThreadPool(int thread_pool_min, + int thread_pool_max) +{ + if (thread_pool_max == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_max must be a positive integer")); + return -1; + } + + if (thread_pool_min > thread_pool_max) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_min must be smaller or equal to thread_pool_max")); + return -1; + } + + return 0; +} + + static int virDomainDefValidateIOThreads(const virDomainDef *def) { @@ -1724,19 +1744,16 @@ virDomainDefValidateIOThreads(const virDomainDef *def) for (i = 0; i < def->niothreadids; i++) { virDomainIOThreadIDDef *iothread = def->iothreadids[i]; - if (iothread->thread_pool_max == 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("thread_pool_max must be a positive integer")); + if (virDomainDefValidateIOThreadsThreadPool(iothread->thread_pool_min, + iothread->thread_pool_max) < 0) return -1; - } - - if (iothread->thread_pool_min > iothread->thread_pool_max) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("thread_pool_min must be smaller or equal to thread_pool_max")); - return -1; - } } + if (def->defaultIOThread && + virDomainDefValidateIOThreadsThreadPool(def->defaultIOThread->thread_pool_min, + def->defaultIOThread->thread_pool_max) < 0) + return -1; + return 0; } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index fad70d3857..efd49cfa01 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="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="thread_pool_max"> + <ref name="unsignedInt"/> + </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 defaultiothread pool size. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7d11ae2c92..654850f925 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -389,18 +389,30 @@ qemuValidateDomainDefIOThreads(const virDomainDef *def, virQEMUCaps *qemuCaps) { size_t i; + bool needsThreadPoolCap = false; for (i = 0; i < def->niothreadids; i++) { virDomainIOThreadIDDef *iothread = def->iothreadids[i]; - if ((iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("thread_pool_min and thread_pool_max is not supported by this QEMU binary")); - return -1; + if (iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) { + needsThreadPoolCap = true; + break; } } + if (def->defaultIOThread && + (def->defaultIOThread->thread_pool_min >= 0 || + def->defaultIOThread->thread_pool_max >= 0)) { + needsThreadPoolCap = true; + } + + if (needsThreadPoolCap && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("thread_pool_min and thread_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@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- 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
participants (4)
-
Daniel P. Berrangé -
Michal Privoznik -
Michal Prívozník
-
Peter Krempa