virsh: Check whether enough arguments was passed to iothreadset

[PATCH] virsh: Check whether enough arguments was passed to iothreadset

Posted by Michal Privoznik 1 year, 11 months ago

Virsh has iothreadset command which allows setting various
attributes of IOThreads. However, when the command is called
without any arguments (besides domain and IOThread IDs), then
@params stays NULL and is passed to virDomainSetIOThreadParams()
which produces rather user unfriendly error message:

  error: params in virDomainSetIOThreadParams must not be NULL

Introduce a check and produce better error message.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/virsh-domain.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5c4a7d7b8d..cfdaac1942 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7853,6 +7853,11 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd)
 
 #undef VSH_IOTHREAD_SET_UINT_PARAMS
 
+    if (nparams == 0) {
+        vshError(ctl, _("Not enough arguments passed, nothing to set"));
+        goto cleanup;
+    }
+
     if (virDomainSetIOThreadParams(dom, id, params, nparams, flags) < 0)
         goto cleanup;
 
-- 
2.35.1

Re: [PATCH] virsh: Check whether enough arguments was passed to iothreadset

Posted by Claudio Fontana 1 year, 11 months ago

Reviewed-by: Claudio Fontana <cfontana@suse.de>

On 6/8/22 15:25, Michal Privoznik wrote:
> Virsh has iothreadset command which allows setting various
> attributes of IOThreads. However, when the command is called
> without any arguments (besides domain and IOThread IDs), then
> @params stays NULL and is passed to virDomainSetIOThreadParams()
> which produces rather user unfriendly error message:
> 
>   error: params in virDomainSetIOThreadParams must not be NULL
> 
> Introduce a check and produce better error message.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/virsh-domain.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 5c4a7d7b8d..cfdaac1942 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -7853,6 +7853,11 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd)
>  
>  #undef VSH_IOTHREAD_SET_UINT_PARAMS
>  
> +    if (nparams == 0) {
> +        vshError(ctl, _("Not enough arguments passed, nothing to set"));
> +        goto cleanup;
> +    }
> +
>      if (virDomainSetIOThreadParams(dom, id, params, nparams, flags) < 0)
>          goto cleanup;
>

Re: [PATCH] virsh: Check whether enough arguments was passed to iothreadset

Posted by Peter Krempa 1 year, 11 months ago

On Wed, Jun 08, 2022 at 15:25:13 +0200, Michal Privoznik wrote:
> Virsh has iothreadset command which allows setting various
> attributes of IOThreads. However, when the command is called
> without any arguments (besides domain and IOThread IDs), then
> @params stays NULL and is passed to virDomainSetIOThreadParams()
> which produces rather user unfriendly error message:
> 
>   error: params in virDomainSetIOThreadParams must not be NULL
> 
> Introduce a check and produce better error message.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/virsh-domain.c | 5 +++++
>  1 file changed, 5 insertions(+)

I've ran into this when testing your series :)

> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 5c4a7d7b8d..cfdaac1942 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -7853,6 +7853,11 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd)
>  
>  #undef VSH_IOTHREAD_SET_UINT_PARAMS
>  
> +    if (nparams == 0) {
> +        vshError(ctl, _("Not enough arguments passed, nothing to set"));
> +        goto cleanup;
> +    }
> +
>      if (virDomainSetIOThreadParams(dom, id, params, nparams, flags) < 0)
>          goto cleanup;

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [PATCH] virsh: Check whether enough arguments was passed to iothreadset

Posted by Michal Prívozník 1 year, 11 months ago

On 6/8/22 15:27, Peter Krempa wrote:
> On Wed, Jun 08, 2022 at 15:25:13 +0200, Michal Privoznik wrote:
>> Virsh has iothreadset command which allows setting various
>> attributes of IOThreads. However, when the command is called
>> without any arguments (besides domain and IOThread IDs), then
>> @params stays NULL and is passed to virDomainSetIOThreadParams()
>> which produces rather user unfriendly error message:
>>
>>   error: params in virDomainSetIOThreadParams must not be NULL
>>
>> Introduce a check and produce better error message.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  tools/virsh-domain.c | 5 +++++
>>  1 file changed, 5 insertions(+)
> 
> I've ran into this when testing your series :)

Yeah, I've tested my v3 that I'm about to send thoroughly and found this
issue. And of course it's at the beginning of my branch, so thanks for
quick review - I can now just send v3.

Michal