Now that we have a capability that reflects whether QEMU is
capable of setting iothread pool size, let's introduce a
validator check to make sure users are not trying to use this
feature with QEMU that doesn't support it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_validate.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 9b6245e6d7..7d11ae2c92 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -384,6 +384,27 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
 }
 
 
+static int
+qemuValidateDomainDefIOThreads(const virDomainDef *def,
+                               virQEMUCaps *qemuCaps)
+{
+    size_t i;
+
+    for (i = 0; i < def->niothreadids; i++) {
+        virDomainIOThreadIDDef *iothread = def->iothreadids[i];
+
+        if ((iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("thread_pool_min and thread_pool_max is not supported by this QEMU binary"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 static int
 qemuValidateDomainDefClockTimers(const virDomainDef *def,
                                  virQEMUCaps *qemuCaps)
@@ -1168,6 +1189,9 @@ qemuValidateDomainDef(const virDomainDef *def,
     if (qemuDomainDefValidateMemoryHotplug(def, NULL) < 0)
         return -1;
 
+    if (qemuValidateDomainDefIOThreads(def, qemuCaps) < 0)
+        return -1;
+
     if (qemuValidateDomainDefClockTimers(def, qemuCaps) < 0)
         return -1;
 
-- 
2.35.1

On Tue, Jun 07, 2022 at 14:52:52 +0200, Michal Privoznik wrote:
> Now that we have a capability that reflects whether QEMU is
> capable of setting iothread pool size, let's introduce a
> validator check to make sure users are not trying to use this
> feature with QEMU that doesn't support it.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_validate.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 9b6245e6d7..7d11ae2c92 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -384,6 +384,27 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
>  }
>  
>  
> +static int
> +qemuValidateDomainDefIOThreads(const virDomainDef *def,
> +                               virQEMUCaps *qemuCaps)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->niothreadids; i++) {
> +        virDomainIOThreadIDDef *iothread = def->iothreadids[i];
> +
> +        if ((iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) &&
> +            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("thread_pool_min and thread_pool_max is not supported by this QEMU binary"));
> +            return -1;
> +        }

You'll also need a check that 'thread_pool_max', if supplied, must be
more or equal to 'thread_pool_min', otherwise:

 $ virsh dumpxml cd | grep thread_pool
     <iothread id='3' thread_pool_min='4' thread_pool_max='3'/>
 $ virsh start cd
 error: Failed to start domain 'cd'
 error: internal error: process exited while connecting to monitor: 2022-06-07T14:58:52.420554Z qemu-system-x86_64: bad thread-pool-min/thread-pool-max values

On 6/7/22 16:59, Peter Krempa wrote:
> On Tue, Jun 07, 2022 at 14:52:52 +0200, Michal Privoznik wrote:
>> Now that we have a capability that reflects whether QEMU is
>> capable of setting iothread pool size, let's introduce a
>> validator check to make sure users are not trying to use this
>> feature with QEMU that doesn't support it.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>>  src/qemu/qemu_validate.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index 9b6245e6d7..7d11ae2c92 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -384,6 +384,27 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver,
>>  }
>>  
>>  
>> +static int
>> +qemuValidateDomainDefIOThreads(const virDomainDef *def,
>> +                               virQEMUCaps *qemuCaps)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 0; i < def->niothreadids; i++) {
>> +        virDomainIOThreadIDDef *iothread = def->iothreadids[i];
>> +
>> +        if ((iothread->thread_pool_min != -1 || iothread->thread_pool_max != -1) &&
>> +            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX)) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                           _("thread_pool_min and thread_pool_max is not supported by this QEMU binary"));
>> +            return -1;
>> +        }
> 
> You'll also need a check that 'thread_pool_max', if supplied, must be
> more or equal to 'thread_pool_min', otherwise:
> 
>  $ virsh dumpxml cd | grep thread_pool
>      <iothread id='3' thread_pool_min='4' thread_pool_max='3'/>
>  $ virsh start cd
>  error: Failed to start domain 'cd'
>  error: internal error: process exited while connecting to monitor: 2022-06-07T14:58:52.420554Z qemu-system-x86_64: bad thread-pool-min/thread-pool-max values
> 

Ah, good point. But there's nothing QEMU specific about this so let me
put it into hypervisor agnostic validator. Which in turn allows me to
amend this change to the 06/15 patch where parser/formatter is introduced.

Michal