[PATCH 07/16] conf: Introduce pool_min and pool_max attributes to IOThread
Michal Prívozník
mprivozn at redhat.com
Thu Jun 2 14:39:55 UTC 2022
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 at 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 at redhat.com>
>
Thanks,
Michal
More information about the libvir-list
mailing list