[PATCH 0/3] Setup iothread polling attributes in the XML
Peter Krempa (3): conf: Store the iothread 'poll' settings in the XML qemu: Use configured iothread poll parameters on startup docs: formatdomain: Properly indent example XML for setting 'metadata_cache' docs/formatdomain.rst | 53 +++++++++++-------- src/conf/domain_conf.c | 41 +++++++++++++- src/conf/domain_conf.h | 7 +++ src/conf/schemas/domaincommon.rng | 19 +++++++ src/qemu/qemu_command.c | 18 +++++++ src/qemu/qemu_driver.c | 30 ++++++----- ...othreads-ids-pool-sizes.x86_64-latest.args | 6 +-- .../iothreads-ids-pool-sizes.xml | 12 +++-- 8 files changed, 142 insertions(+), 44 deletions(-) -- 2.39.2
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@redhat.com> --- 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, Mar 31, 2023 at 02:00:32PM +0200, Peter Krempa wrote:
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@redhat.com> --- 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/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; +
All these are int64_t in QEMU, although I understand we don't need to represent all the possible values, but it is weird when each one is different here. I think you can't go wrong here if you make all of them unsigned long long.
On Fri, Apr 14, 2023 at 15:44:10 +0200, Martin Kletzander wrote:
On Fri, Mar 31, 2023 at 02:00:32PM +0200, Peter Krempa wrote:
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@redhat.com> --- 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/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; +
All these are int64_t in QEMU, although I understand we don't need to represent all the possible values, but it is weird when each one is different here.
I think you can't go wrong here if you make all of them unsigned long long.
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.
On Mon, Apr 17, 2023 at 01:16:34PM +0200, Peter Krempa wrote:
On Fri, Apr 14, 2023 at 15:44:10 +0200, Martin Kletzander wrote:
On Fri, Mar 31, 2023 at 02:00:32PM +0200, Peter Krempa wrote:
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@redhat.com> --- 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/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; +
All these are int64_t in QEMU, although I understand we don't need to represent all the possible values, but it is weird when each one is different here.
I think you can't go wrong here if you make all of them unsigned long long.
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.
I think that would be the safer option, yes.
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@redhat.com> --- 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
On Fri, Mar 31, 2023 at 02:00:33PM +0200, Peter Krempa wrote:
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@redhat.com> --- 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,
This is defined as int, but passed as long here, but if you are going to make all of them switch to unsigned long long, then poll-grow above needs a fix the other way around.
Indent the example XML block so that it belongs to the paragraph talking about it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- 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
On Fri, Mar 31, 2023 at 02:00:34PM +0200, Peter Krempa wrote:
Indent the example XML block so that it belongs to the paragraph talking about it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
On Fri, Mar 31, 2023 at 02:00:31PM +0200, Peter Krempa wrote:
Peter Krempa (3): conf: Store the iothread 'poll' settings in the XML qemu: Use configured iothread poll parameters on startup
If you change all of them to the same type (ULL for example) and adjust the type in JSON appropriately, then Reviewed-by: Martin Kletzander <mkletzan@redhat.com> for those two as well, I forgot to mention it in individual replies
docs: formatdomain: Properly indent example XML for setting 'metadata_cache'
docs/formatdomain.rst | 53 +++++++++++-------- src/conf/domain_conf.c | 41 +++++++++++++- src/conf/domain_conf.h | 7 +++ src/conf/schemas/domaincommon.rng | 19 +++++++ src/qemu/qemu_command.c | 18 +++++++ src/qemu/qemu_driver.c | 30 ++++++----- ...othreads-ids-pool-sizes.x86_64-latest.args | 6 +-- .../iothreads-ids-pool-sizes.xml | 12 +++-- 8 files changed, 142 insertions(+), 44 deletions(-)
-- 2.39.2
participants (2)
-
Martin Kletzander -
Peter Krempa