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 | 32 +++++++++-
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, 110 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..de085f616a 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" pool_min="2" 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 ``pool_min`` and ``pool_max`` which
+ allow setting lower and upper boundary for number of worker threads for
+ given IOThread. :since:`Since 8.4.0`
+
CPU Tuning
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8d4b27dbc0..7572e62db1 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->pool_min = -1;
+ def->pool_max = -1;
+
return def;
}
@@ -17008,7 +17011,7 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
*
* <iothreads>4</iothreads>
* <iothreadids>
- * <iothread id='1'/>
+ * <iothread id='1' pool_min="0" pool_max="60"/>
* <iothread id='3'/>
* <iothread id='5'/>
* <iothread id='7'/>
@@ -17024,6 +17027,14 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node)
&iothrid->iothread_id) < 0)
return NULL;
+ if (virXMLPropLongLong(node, "pool_min", 10,
+ VIR_XML_PROP_NONNEGATIVE, &iothrid->pool_min, -1) < 0)
+ return NULL;
+
+ if (virXMLPropLongLong(node, "pool_max", 10,
+ VIR_XML_PROP_NONNEGATIVE, &iothrid->pool_max, -1) < 0)
+ return NULL;
+
return g_steal_pointer(&iothrid);
}
@@ -27607,8 +27618,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->pool_min >= 0) {
+ virBufferAsprintf(&attrBuf, " pool_min='%lld'",
+ iothread->pool_min);
+ }
+
+ if (iothread->pool_max >= 0) {
+ virBufferAsprintf(&attrBuf, " pool_max='%lld'",
+ iothread->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..c502100d45 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2646,6 +2646,9 @@ struct _virDomainIOThreadIDDef {
virBitmap *cpumask;
virDomainThreadSchedParam sched;
+
+ long long pool_min;
+ long long pool_max;
};
void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def);
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index cc598212a8..94035c38e7 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="pool_min">
+ <ref name="unsignedLong"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="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..5ff3b4b18e
--- /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' pool_min='0' pool_max='60'/>
+ <iothread id='4' pool_min='1' 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.1On Thu, Jun 02, 2022 at 09:17:57 +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 | 32 +++++++++- > 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, 110 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..de085f616a 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst [...] > @@ -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 ``pool_min`` and ``pool_max`` which > + allow setting lower and upper boundary for number of worker threads for > + given IOThread. :since:`Since 8.4.0` 8.5.0 > + Drop this extra line. Also I'd go probably for 'thread_pool_min/max'. In case of iothreads it doesn't matter too much, but later I'll object to 'mainloop' qemuism being introduced as a term rivaling our established 'emulator'. Using just 'pool' there would be a bit too ambiguous IMO. > > > CPU Tuning [...] > void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def); > diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng > index cc598212a8..94035c38e7 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="pool_min"> > + <ref name="unsignedLong"/> > + </attribute> > + </optional> > + <optional> > + <attribute name="pool_max"> > + <ref name="unsignedLong"/> > + </attribute> > + </optional> I wondered how we could have distinguished between a long and int in the schema and the answer is ... we don't. Additionally using long long even feels a bit overkill. 2 billion threads ought to be enough for everybody. With the doc fixes ... and potential re-name: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On 6/2/22 10:08, Peter Krempa wrote: > On Thu, Jun 02, 2022 at 09:17:57 +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 | 32 +++++++++- >> 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, 110 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..de085f616a 100644 >> --- a/docs/formatdomain.rst >> +++ b/docs/formatdomain.rst > > [...] > >> @@ -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 ``pool_min`` and ``pool_max`` which >> + allow setting lower and upper boundary for number of worker threads for >> + given IOThread. :since:`Since 8.4.0` > > 8.5.0 > Ooops yes. This is what you get when you write patches but don't finish them before entering freeze :-) >> + > > Drop this extra line. > > Also I'd go probably for 'thread_pool_min/max'. In case of iothreads it > doesn't matter too much, but later I'll object to 'mainloop' qemuism > being introduced as a term rivaling our established 'emulator'. > > Using just 'pool' there would be a bit too ambiguous IMO. > >> >> >> CPU Tuning > > [...] > >> void virDomainIOThreadIDDefFree(virDomainIOThreadIDDef *def); >> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng >> index cc598212a8..94035c38e7 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="pool_min"> >> + <ref name="unsignedLong"/> >> + </attribute> >> + </optional> >> + <optional> >> + <attribute name="pool_max"> >> + <ref name="unsignedLong"/> >> + </attribute> >> + </optional> > > I wondered how we could have distinguished between a long and int in the > schema and the answer is ... we don't. > > Additionally using long long even feels a bit overkill. 2 billion > threads ought to be enough for everybody. I can switch to int. I don't care that much honestly. I just looked at what QEMU has and mimicked that. Do you want me to switch? That would make patch 2 obsolete as nobody we already have virXMLPropInt(). > > With the doc fixes ... and potential re-name: > > Reviewed-by: Peter Krempa <pkrempa@redhat.com> > Thanks, Michal
© 2016 - 2026 Red Hat, Inc.