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@xxxxxxxxxx> >> --- >> 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@xxxxxxxxxx> > Thanks, Michal