| Message ID | 1383809692-11158-1-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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>
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.
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
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(-)