Patchwork block: Print its file name if backing file opening failed

login
register
mail settings
Submitter Fam Zheng
Date Nov. 7, 2013, 7:34 a.m.
Message ID <1383809692-11158-1-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/289215/
State New
Headers show

Comments

Fam Zheng - Nov. 7, 2013, 7:34 a.m.
If backing file doesn't exist, the error message is confusing and
misleading:

    $ qemu /tmp/a.qcow2
    qemu: could not open disk image /tmp/a.qcow2: Could not open file: No
    such file or directory

But...

    $ ls /tmp/a.qcow2
    /tmp/a.qcow2

    $ qemu-img info /tmp/a.qcow2
    image: /tmp/a.qcow2
    file format: qcow2
    virtual size: 8.0G (8589934592 bytes)
    disk size: 196K
    cluster_size: 65536
    backing file: /tmp/b.qcow2

Because...

    $ ls /tmp/b.qcow2
    ls: cannot access /tmp/b.qcow2: No such file or directory

This is not intuitive. It's better to have the missing file's name in
the error message. With this patch:

    $ qemu-io -c 'read 0 512' /tmp/a.qcow2
    qemu-io: can't open device /tmp/a.qcow2: Could not open backing
    file: Could not open '/stor/vm/arch.raw': No such file or directory
    no file open, try 'help open'

Which is a little bit better.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c                    | 3 ++-
 block/raw-posix.c          | 1 -
 block/raw-win32.c          | 1 -
 tests/qemu-iotests/051.out | 2 +-
 tests/qemu-iotests/069.out | 2 +-
 5 files changed, 4 insertions(+), 5 deletions(-)
Benoît Canet - Nov. 7, 2013, 12:12 p.m.
Le Thursday 07 Nov 2013 à 15:34:52 (+0800), Fam Zheng a écrit :
> If backing file doesn't exist, the error message is confusing and
> misleading:
> 
>     $ qemu /tmp/a.qcow2
>     qemu: could not open disk image /tmp/a.qcow2: Could not open file: No
>     such file or directory
> 
> But...
> 
>     $ ls /tmp/a.qcow2
>     /tmp/a.qcow2
> 
>     $ qemu-img info /tmp/a.qcow2
>     image: /tmp/a.qcow2
>     file format: qcow2
>     virtual size: 8.0G (8589934592 bytes)
>     disk size: 196K
>     cluster_size: 65536
>     backing file: /tmp/b.qcow2
> 
> Because...
> 
>     $ ls /tmp/b.qcow2
>     ls: cannot access /tmp/b.qcow2: No such file or directory
> 
> This is not intuitive. It's better to have the missing file's name in
> the error message. With this patch:
> 
>     $ qemu-io -c 'read 0 512' /tmp/a.qcow2
>     qemu-io: can't open device /tmp/a.qcow2: Could not open backing
>     file: Could not open '/stor/vm/arch.raw': No such file or directory
>     no file open, try 'help open'
> 
> Which is a little bit better.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c                    | 3 ++-
>  block/raw-posix.c          | 1 -
>  block/raw-win32.c          | 1 -
>  tests/qemu-iotests/051.out | 2 +-
>  tests/qemu-iotests/069.out | 2 +-
>  5 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f706634..a8dbcfc 100644
> --- a/block.c
> +++ b/block.c
> @@ -1009,7 +1009,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>          bdrv_unref(bs->backing_hd);
>          bs->backing_hd = NULL;
>          bs->open_flags |= BDRV_O_NO_BACKING;
> -        error_propagate(errp, local_err);
> +        error_setg(errp, "Could not open backing file: %s",
> +                   error_get_pretty(local_err));
>          return ret;
>      }
>      pstrcpy(bs->backing_file, sizeof(bs->backing_file),
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index f6d48bb..e1b1ba2 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -310,7 +310,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>          if (ret == -EROFS) {
>              ret = -EACCES;
>          }
> -        error_setg_errno(errp, -ret, "Could not open file");
>          goto fail;
>      }
>      s->fd = fd;
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 2741e4d..2bad5a3 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -280,7 +280,6 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
>          } else {
>              ret = -EINVAL;
>          }
> -        error_setg_errno(errp, -ret, "Could not open file");
>          goto fail;
>      }
>  
> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
> index 15deef6..d351935 100644
> --- a/tests/qemu-iotests/051.out
> +++ b/tests/qemu-iotests/051.out
> @@ -226,6 +226,6 @@ Testing: -drive file=foo:bar
>  QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol
>  
>  Testing: -drive file.filename=foo:bar
> -QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open file: No such file or directory
> +QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
>  
>  *** done
> diff --git a/tests/qemu-iotests/069.out b/tests/qemu-iotests/069.out
> index 3648814..b48306d 100644
> --- a/tests/qemu-iotests/069.out
> +++ b/tests/qemu-iotests/069.out
> @@ -4,5 +4,5 @@ QA output created by 069
>  
>  Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=131072 
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file='TEST_DIR/t.IMGFMT.base' 
> -qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open file: No such file or directory
> +qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open backing file: Could not open 'TEST_DIR/t.IMGFMT.base': No such file or directory
>  *** done
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Eric Blake - Nov. 7, 2013, 1:17 p.m.
On 11/07/2013 12:34 AM, Fam Zheng wrote:
> If backing file doesn't exist, the error message is confusing and
> misleading:
> 

> 
> This is not intuitive. It's better to have the missing file's name in
> the error message. With this patch:
> 
>     $ qemu-io -c 'read 0 512' /tmp/a.qcow2
>     qemu-io: can't open device /tmp/a.qcow2: Could not open backing
>     file: Could not open '/stor/vm/arch.raw': No such file or directory
>     no file open, try 'help open'
> 
> Which is a little bit better.

Indeed.  Are you trying to get this into 1.7?

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c                    | 3 ++-
>  block/raw-posix.c          | 1 -
>  block/raw-win32.c          | 1 -
>  tests/qemu-iotests/051.out | 2 +-
>  tests/qemu-iotests/069.out | 2 +-
>  5 files changed, 4 insertions(+), 5 deletions(-)

> 
> diff --git a/block.c b/block.c
> index f706634..a8dbcfc 100644
> --- a/block.c
> +++ b/block.c
> @@ -1009,7 +1009,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>          bdrv_unref(bs->backing_hd);
>          bs->backing_hd = NULL;
>          bs->open_flags |= BDRV_O_NO_BACKING;
> -        error_propagate(errp, local_err);
> +        error_setg(errp, "Could not open backing file: %s",
> +                   error_get_pretty(local_err));
>          return ret;

Needs a v2, that doesn't leak local_err.

Patch

diff --git a/block.c b/block.c
index f706634..a8dbcfc 100644
--- a/block.c
+++ b/block.c
@@ -1009,7 +1009,8 @@  int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
         bdrv_unref(bs->backing_hd);
         bs->backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
-        error_propagate(errp, local_err);
+        error_setg(errp, "Could not open backing file: %s",
+                   error_get_pretty(local_err));
         return ret;
     }
     pstrcpy(bs->backing_file, sizeof(bs->backing_file),
diff --git a/block/raw-posix.c b/block/raw-posix.c
index f6d48bb..e1b1ba2 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -310,7 +310,6 @@  static int raw_open_common(BlockDriverState *bs, QDict *options,
         if (ret == -EROFS) {
             ret = -EACCES;
         }
-        error_setg_errno(errp, -ret, "Could not open file");
         goto fail;
     }
     s->fd = fd;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 2741e4d..2bad5a3 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -280,7 +280,6 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
         } else {
             ret = -EINVAL;
         }
-        error_setg_errno(errp, -ret, "Could not open file");
         goto fail;
     }
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 15deef6..d351935 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -226,6 +226,6 @@  Testing: -drive file=foo:bar
 QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol
 
 Testing: -drive file.filename=foo:bar
-QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open file: No such file or directory
+QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
 
 *** done
diff --git a/tests/qemu-iotests/069.out b/tests/qemu-iotests/069.out
index 3648814..b48306d 100644
--- a/tests/qemu-iotests/069.out
+++ b/tests/qemu-iotests/069.out
@@ -4,5 +4,5 @@  QA output created by 069
 
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=131072 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file='TEST_DIR/t.IMGFMT.base' 
-qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open file: No such file or directory
+qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open backing file: Could not open 'TEST_DIR/t.IMGFMT.base': No such file or directory
 *** done