qemu-img: Improve commit invalid base message
diff mbox

Message ID 20161201020508.24417-1-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Dec. 1, 2016, 2:05 a.m. UTC
When trying to invoke qemu-img commit with a base image file name that
is not part of the top image's backing chain, the user receives a rather
plain "Base not found" error message. This is not really helpful because
it does not explain what "not found" means, potentially leaving the user
wondering why qemu cannot find a file despite it clearly existing in the
file system.

Improve the error message by clarifying that "not found" means "not
found in the top image's backing chain".

Reported-by: Ala Hino <ahino@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
Reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1390991
---
 qemu-img.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Eric Blake Dec. 1, 2016, 2:36 a.m. UTC | #1
On 11/30/2016 08:05 PM, Max Reitz wrote:
> When trying to invoke qemu-img commit with a base image file name that
> is not part of the top image's backing chain, the user receives a rather
> plain "Base not found" error message. This is not really helpful because
> it does not explain what "not found" means, potentially leaving the user
> wondering why qemu cannot find a file despite it clearly existing in the
> file system.
> 
> Improve the error message by clarifying that "not found" means "not
> found in the top image's backing chain".
> 
> Reported-by: Ala Hino <ahino@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1390991
> ---
>  qemu-img.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Safe for 2.8, if the block maintainers agree.

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz Dec. 1, 2016, 11:38 p.m. UTC | #2
On 01.12.2016 03:36, Eric Blake wrote:
> On 11/30/2016 08:05 PM, Max Reitz wrote:
>> When trying to invoke qemu-img commit with a base image file name that
>> is not part of the top image's backing chain, the user receives a rather
>> plain "Base not found" error message. This is not really helpful because
>> it does not explain what "not found" means, potentially leaving the user
>> wondering why qemu cannot find a file despite it clearly existing in the
>> file system.
>>
>> Improve the error message by clarifying that "not found" means "not
>> found in the top image's backing chain".
>>
>> Reported-by: Ala Hino <ahino@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> Reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1390991
>> ---
>>  qemu-img.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Safe for 2.8, if the block maintainers agree.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

I agree that it's safe, but we're at a point where only critical things
should go into the release. Getting this into 2.9 will be sufficient.

Max
Max Reitz Jan. 16, 2017, 5:16 p.m. UTC | #3
On 01.12.2016 03:05, Max Reitz wrote:
> When trying to invoke qemu-img commit with a base image file name that
> is not part of the top image's backing chain, the user receives a rather
> plain "Base not found" error message. This is not really helpful because
> it does not explain what "not found" means, potentially leaving the user
> wondering why qemu cannot find a file despite it clearly existing in the
> file system.
> 
> Improve the error message by clarifying that "not found" means "not
> found in the top image's backing chain".
> 
> Reported-by: Ala Hino <ahino@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Reported in: https://bugzilla.redhat.com/show_bug.cgi?id=1390991
> ---
>  qemu-img.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied to my block tree.

Max

Patch
diff mbox

diff --git a/qemu-img.c b/qemu-img.c
index 6949b73..d020e9a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -912,7 +912,9 @@  static int img_commit(int argc, char **argv)
     if (base) {
         base_bs = bdrv_find_backing_image(bs, base);
         if (!base_bs) {
-            error_setg(&local_err, QERR_BASE_NOT_FOUND, base);
+            error_setg(&local_err,
+                       "Did not find '%s' in the backing chain of '%s'",
+                       base, filename);
             goto done;
         }
     } else {