qemu-img: Make resize error message more general

[Qemu-devel] [PATCH] qemu-img: Make resize error message more general

Posted by Max Reitz 6 years, 2 months ago

The issue:

  $ qemu-img resize -f qcow2 foo.qcow2
  qemu-img: Expecting one image file name
  Try 'qemu-img --help' for more information

So we gave an image file name, but we omitted the length.  qemu-img
thinks the last argument is always the size and removes it immediately
from argv (by decrementing argc), and tries to verify that it is a valid
size only at a later point.

So we do not actually know whether that last argument we called "size"
is indeed a size or whether the user instead forgot to specify that size
but did give a file name.

Therefore, the error message should be more general.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1523458
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 68b375f998..2753b63514 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3468,7 +3468,7 @@ static int img_resize(int argc, char **argv)
         }
     }
     if (optind != argc - 1) {
-        error_exit("Expecting one image file name");
+        error_exit("Expecting image file name and size");
     }
     filename = argv[optind++];
 
-- 
2.14.3

Re: [Qemu-devel] [Qemu-block] [PATCH] qemu-img: Make resize error message more general

Posted by John Snow 6 years, 2 months ago


On 02/05/2018 11:27 AM, Max Reitz wrote:
> The issue:
> 
>   $ qemu-img resize -f qcow2 foo.qcow2
>   qemu-img: Expecting one image file name
>   Try 'qemu-img --help' for more information
> 
> So we gave an image file name, but we omitted the length.  qemu-img
> thinks the last argument is always the size and removes it immediately
> from argv (by decrementing argc), and tries to verify that it is a valid
> size only at a later point.
> 
> So we do not actually know whether that last argument we called "size"
> is indeed a size or whether the user instead forgot to specify that size
> but did give a file name.
> 
> Therefore, the error message should be more general.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1523458
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 68b375f998..2753b63514 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3468,7 +3468,7 @@ static int img_resize(int argc, char **argv)
>          }
>      }
>      if (optind != argc - 1) {
> -        error_exit("Expecting one image file name");
> +        error_exit("Expecting image file name and size");
>      }
>      filename = argv[optind++];
>  
> 

Sounds good to me.

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH] qemu-img: Make resize error message more general

Posted by Eric Blake 6 years, 2 months ago

On 02/05/2018 10:27 AM, Max Reitz wrote:
> The issue:
> 
>    $ qemu-img resize -f qcow2 foo.qcow2
>    qemu-img: Expecting one image file name
>    Try 'qemu-img --help' for more information
> 
> So we gave an image file name, but we omitted the length.  qemu-img
> thinks the last argument is always the size and removes it immediately
> from argv (by decrementing argc), and tries to verify that it is a valid
> size only at a later point.
> 
> So we do not actually know whether that last argument we called "size"
> is indeed a size or whether the user instead forgot to specify that size
> but did give a file name.
> 
> Therefore, the error message should be more general.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1523458
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   qemu-img.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 68b375f998..2753b63514 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3468,7 +3468,7 @@ static int img_resize(int argc, char **argv)
>           }
>       }
>       if (optind != argc - 1) {
> -        error_exit("Expecting one image file name");
> +        error_exit("Expecting image file name and size");

Yes, that reads better in the context you described.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qemu-img: Make resize error message more general

Posted by Max Reitz 6 years, 1 month ago

On 2018-02-05 17:27, Max Reitz wrote:
> The issue:
> 
>   $ qemu-img resize -f qcow2 foo.qcow2
>   qemu-img: Expecting one image file name
>   Try 'qemu-img --help' for more information
> 
> So we gave an image file name, but we omitted the length.  qemu-img
> thinks the last argument is always the size and removes it immediately
> from argv (by decrementing argc), and tries to verify that it is a valid
> size only at a later point.
> 
> So we do not actually know whether that last argument we called "size"
> is indeed a size or whether the user instead forgot to specify that size
> but did give a file name.
> 
> Therefore, the error message should be more general.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1523458
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to my block branch.

Max