Re: [PATCH 07/16] conf: Introduce pool_min and pool_max attributes to IOThread

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;