Currently we allow configuring the 'poll-max-ns', 'poll-grow', and 'poll-shrink' parameters of qemu iothreads only during runtime and they are not persisted. Add XML machinery to persist them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.rst | 11 ++++- src/conf/domain_conf.c | 41 ++++++++++++++++++- src/conf/domain_conf.h | 7 ++++ src/conf/schemas/domaincommon.rng | 19 +++++++++ .../iothreads-ids-pool-sizes.xml | 12 ++++-- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 27f83e254d..e3ebf1c718 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -740,7 +740,9 @@ host/guest with many LUNs. :since:`Since 1.2.8 (QEMU only)` <iothread id="2"/> <iothread id="4"/> <iothread id="6"/> - <iothread id="8" thread_pool_min="2" thread_pool_max="32"/> + <iothread id="8" thread_pool_min="2" thread_pool_max="32"> + <poll max='123' grow='456' shrink='789'/> + </iothread> </iothreadids> <defaultiothread thread_pool_min="8" thread_pool_max="16"/> ... @@ -766,6 +768,13 @@ 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` + :since:`Since 9.1.0` an optional sub-element ``poll`` with can be used to + override the hypervisor-default interval of polling for the iothread before + it switches back to events. The optional attribute ``max`` sets the maximum + time polling should be used in nanoseconds. Setting ``max`` to ``0`` disables + polling. Attributes ``grow`` and ``shrink`` override (or disable when set to + ``0`` the default steps for increasing/decreasing the polling interval if + the set interval is deemed insufficient or extensive. ``defaultiothread`` This element represents the default event loop within hypervisor, where I/O requests from devices not assigned to a specific IOThread are processed. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 08527964d1..060ab7b41f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15787,6 +15787,7 @@ static virDomainIOThreadIDDef * virDomainIOThreadIDDefParseXML(xmlNodePtr node) { g_autoptr(virDomainIOThreadIDDef) iothrid = virDomainIOThreadIDDefNew(); + xmlNodePtr pollNode; if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, @@ -15803,6 +15804,28 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node) &iothrid->thread_pool_max, -1) < 0) return NULL; + if ((pollNode = virXMLNodeGetSubelement(node, "poll"))) { + int rc; + + if ((rc = virXMLPropULongLong(pollNode, "max", 10, VIR_XML_PROP_NONE, + &iothrid->poll_max_ns)) < 0) + return NULL; + + iothrid->set_poll_max_ns = rc == 1; + + if ((rc = virXMLPropUInt(pollNode, "grow", 10, VIR_XML_PROP_NONE, + &iothrid->poll_grow)) < 0) + return NULL; + + iothrid->set_poll_grow = rc == 1; + + if ((rc = virXMLPropUInt(pollNode, "shrink", 10, VIR_XML_PROP_NONE, + &iothrid->poll_shrink)) < 0) + return NULL; + + iothrid->set_poll_shrink = rc == 1; + } + return g_steal_pointer(&iothrid); } @@ -26735,6 +26758,9 @@ virDomainDefIothreadShouldFormat(const virDomainDef *def) for (i = 0; i < def->niothreadids; i++) { if (!def->iothreadids[i]->autofill || + def->iothreadids[i]->set_poll_max_ns || + def->iothreadids[i]->set_poll_grow || + def->iothreadids[i]->set_poll_shrink || def->iothreadids[i]->thread_pool_min >= 0 || def->iothreadids[i]->thread_pool_max >= 0) return true; @@ -26783,6 +26809,8 @@ virDomainDefIOThreadsFormat(virBuffer *buf, for (i = 0; i < def->niothreadids; i++) { virDomainIOThreadIDDef *iothread = def->iothreadids[i]; g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) iothreadChildBuf = VIR_BUFFER_INIT_CHILD(&childrenBuf); + g_auto(virBuffer) pollAttrBuf = VIR_BUFFER_INITIALIZER; virBufferAsprintf(&attrBuf, " id='%u'", iothread->iothread_id); @@ -26797,7 +26825,18 @@ virDomainDefIOThreadsFormat(virBuffer *buf, iothread->thread_pool_max); } - virXMLFormatElement(&childrenBuf, "iothread", &attrBuf, NULL); + if (iothread->set_poll_max_ns) + virBufferAsprintf(&pollAttrBuf, " max='%llu'", iothread->poll_max_ns); + + if (iothread->set_poll_grow) + virBufferAsprintf(&pollAttrBuf, " grow='%u'", iothread->poll_grow); + + if (iothread->set_poll_shrink) + virBufferAsprintf(&pollAttrBuf, " shrink='%u'", iothread->poll_shrink); + + virXMLFormatElement(&iothreadChildBuf, "poll", &pollAttrBuf, NULL); + + virXMLFormatElement(&childrenBuf, "iothread", &attrBuf, &iothreadChildBuf); } virXMLFormatElement(buf, "iothreadids", NULL, &childrenBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5a2c70f012..8ac76f25d3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2713,6 +2713,13 @@ struct _virDomainIOThreadIDDef { virDomainThreadSchedParam sched; + unsigned long long poll_max_ns; + bool set_poll_max_ns; + unsigned int poll_grow; + bool set_poll_grow; + unsigned int poll_shrink; + bool set_poll_shrink; + int thread_pool_min; int thread_pool_max; }; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 6158ed79ac..57fb4a5e33 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -885,6 +885,25 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <element name="poll"> + <optional> + <attribute name="max"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="grow"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="shrink"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </element> + </optional> </element> </zeroOrMore> </element> diff --git a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml index df4b291a7a..63fb4a52f6 100644 --- a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.xml @@ -7,9 +7,15 @@ <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='4' thread_pool_min='1' thread_pool_max='1'> + <poll max='123'/> + </iothread> + <iothread id='1'> + <poll grow='456' shrink='789'/> + </iothread> + <iothread id='3'> + <poll max='123000' grow='456' shrink='789'/> + </iothread> <iothread id='5'/> </iothreadids> <defaultiothread thread_pool_min='8' thread_pool_max='16'/> -- 2.39.2

On Fri, Apr 14, 2023 at 15:44:10 +0200, Martin Kletzander wrote: I've actually copied these from struct _qemuMonitorIOThreadInfo withouth thinking too much about it. We already have code which can set the parameters during runtime but we never remebered them. So if we really want to use 64 bit values for those the existing code needs to be modified first.

Implement the support for the persisted poll parameters and remove restrictions on saving config when modifying them during runtime. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 18 +++++++++++ src/qemu/qemu_driver.c | 30 ++++++++++--------- ...othreads-ids-pool-sizes.x86_64-latest.args | 6 ++-- 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7e75354902..0693369faa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7300,6 +7300,24 @@ qemuBuildIOThreadCommandLine(virCommand *cmd, NULL) < 0) return -1; + if (iothread->set_poll_max_ns && + virJSONValueObjectAdd(&props, + "U:poll-max-ns", iothread->poll_max_ns, + NULL) < 0) + return -1; + + if (iothread->set_poll_grow && + virJSONValueObjectAdd(&props, + "u:poll-grow", iothread->poll_grow, + NULL) < 0) + return -1; + + if (iothread->set_poll_shrink && + virJSONValueObjectAdd(&props, + "U:poll-shrink", iothread->poll_shrink, + NULL) < 0) + return -1; + if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0) return -1; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 09ae893daf..3412c5d6f2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5071,23 +5071,30 @@ qemuDomainHotplugModIOThread(virDomainObj *vm, } -static int +static void 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_poll_max_ns) { + def->poll_max_ns = mondef.poll_max_ns; + def->set_poll_max_ns = true; + } + + if (mondef.set_poll_grow) { + def->poll_grow = mondef.poll_grow; + def->set_poll_grow = true; + } + + if (mondef.set_poll_shrink) { + def->poll_shrink = mondef.poll_shrink; + def->set_poll_shrink = true; + } 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; } @@ -5406,12 +5413,7 @@ qemuDomainChgIOThread(virQEMUDriver *driver, 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; - } - + qemuDomainHotplugModIOThreadIDDef(iothreaddef, iothread); break; } } 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 4f5a11aab1..1422e94e35 100644 --- a/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args +++ b/tests/qemuxml2argvdata/iothreads-ids-pool-sizes.x86_64-latest.args @@ -18,9 +18,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -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":"iothread4","thread-pool-min":1,"thread-pool-max":1,"poll-max-ns":123}' \ +-object '{"qom-type":"iothread","id":"iothread1","poll-grow":456,"poll-shrink":789}' \ +-object '{"qom-type":"iothread","id":"iothread3","poll-max-ns":123000,"poll-grow":456,"poll-shrink":789}' \ -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 \ -- 2.39.2

Indent the example XML block so that it belongs to the paragraph talking about it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatdomain.rst | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index e3ebf1c718..dcc5844b7b 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3275,28 +3275,26 @@ paravirtualized driver is specified via the ``disk`` element. to the ``qemu`` `qcow2 cache docs <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt>`__ - **Example:** - -:: - - <disk type='file' device='disk'> - <driver name='qemu' type='qcow2'> - <metadata_cache> - <max_size unit='bytes'>1234</max_size> - </metadata_cache> - </driver> - <source file='/var/lib/libvirt/images/domain.qcow'/> - <backingStore type='file'> - <format type='qcow2'> - <metadata_cache> - <max_size unit='bytes'>1234</max_size> - </metadata_cache> - </format> - <source file='/var/lib/libvirt/images/snapshot.qcow'/> - <backingStore/> - </backingStore> - <target dev='vdd' bus='virtio'/> - </disk> + **Example**:: + + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </driver> + <source file='/var/lib/libvirt/images/domain.qcow'/> + <backingStore type='file'> + <format type='qcow2'> + <metadata_cache> + <max_size unit='bytes'>1234</max_size> + </metadata_cache> + </format> + <source file='/var/lib/libvirt/images/snapshot.qcow'/> + <backingStore/> + </backingStore> + <target dev='vdd' bus='virtio'/> + </disk> ``backenddomain`` The optional ``backenddomain`` element allows specifying a backend domain -- 2.39.2