Re: [Qemu-devel] [PATCH for-1.7 v2] block: Print its file name if backing file opening failed

2013-11-11 Thread Stefan Hajnoczi
On Fri, Nov 08, 2013 at 11:26:49AM +0800, Fam Zheng wrote:
 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 f...@redhat.com
 
 ---
 v2: Don't leak local_err (Eric).
 
 Signed-off-by: Fam Zheng f...@redhat.com
 ---
  block.c| 4 +++-
  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, 5 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com



Re: [Qemu-devel] [PATCH for-1.7 v2] block: Print its file name if backing file opening failed

2013-11-11 Thread Kevin Wolf
Am 08.11.2013 um 04:26 hat Fam Zheng geschrieben:
 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 f...@redhat.com

Thanks, applied to the block branch.

However, while this may be an improvement, it's certainly not what we'd
want the error message to look like in the final state. Consider a chain
with multiple backing files:

qemu-system-x86_64: -drive file=/tmp/d.qcow2: could not open disk image
/tmp/d.qcow2: Could not open backing file: Could not open backing file:
Could not open backing file: Could not open backing file: Could not open
'/tmp/blubber': No such file or directory

Kevin



Re: [Qemu-devel] [PATCH for-1.7 v2] block: Print its file name if backing file opening failed

2013-11-08 Thread Eric Blake
On 11/07/2013 08:26 PM, Fam Zheng wrote:
 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
 

 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 f...@redhat.com
 
 ---
 v2: Don't leak local_err (Eric).

Thanks.

Reviewed-by: Eric Blake ebl...@redhat.com

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH for-1.7 v2] block: Print its file name if backing file opening failed

2013-11-07 Thread Fam Zheng
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 f...@redhat.com

---
v2: Don't leak local_err (Eric).

Signed-off-by: Fam Zheng f...@redhat.com
---
 block.c| 4 +++-
 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, 5 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index f706634..7ffb08b 100644
--- a/block.c
+++ b/block.c
@@ -1009,7 +1009,9 @@ 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));
+error_free(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