Patchwork [v3] block: output more error messages if failed to create temporary snapshot

login
register
mail settings
Submitter dunrong huang
Date Sept. 5, 2012, 1:24 p.m.
Message ID <1346851493-9149-1-git-send-email-riegamaths@gmail.com>
Download mbox | patch
Permalink /patch/181856/
State New
Headers show

Comments

dunrong huang - Sept. 5, 2012, 1:24 p.m.
From: Dunrong Huang <riegamaths@gmail.com>

If we failed to create temporary snapshot, the error message did not match
with the error, for example:

$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image /home/mathslinux/Images/debian.qcow2: No such file or directory

Indeed, the file which cant be created is /tmp/bad_path/vl.xxxxxx,  not
debian.qcow2. so the error message makes users feel confused.

Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
---
v1 -> v2:
   Output error message only if fd < 0
v2 -> v3:
   Output error message in the caller of get_tmp_filename()
 block.c | 2 ++
 1 个文件被修改,插入 2 行(+)
Paolo Bonzini - Sept. 5, 2012, 3:40 p.m.
Il 05/09/2012 15:24, riegamaths@gmail.com ha scritto:
> From: Dunrong Huang <riegamaths@gmail.com>
> 
> If we failed to create temporary snapshot, the error message did not match
> with the error, for example:
> 
> $ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
> qemu-system-x86_64: -enable-kvm: could not open disk image /home/mathslinux/Images/debian.qcow2: No such file or directory
> 
> Indeed, the file which cant be created is /tmp/bad_path/vl.xxxxxx,  not
> debian.qcow2. so the error message makes users feel confused.
> 
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> ---
> v1 -> v2:
>    Output error message only if fd < 0
> v2 -> v3:
>    Output error message in the caller of get_tmp_filename()
>  block.c | 2 ++
>  1 个文件被修改,插入 2 行(+)
> 
> diff --git a/block.c b/block.c
> index 470bdcc..074987e 100644
> --- a/block.c
> +++ b/block.c
> @@ -764,6 +764,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>  
>          ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
>          if (ret < 0) {
> +            fprintf(stderr, "Could not create temporary snapshot %s: %s\n",
> +                    tmp_filename, strerror(errno));
>              return ret;
>          }
>  
> 

Looks good.

Paolo
Kevin Wolf - Sept. 7, 2012, 1:12 p.m.
Am 05.09.2012 15:24, schrieb riegamaths@gmail.com:
> From: Dunrong Huang <riegamaths@gmail.com>
> 
> If we failed to create temporary snapshot, the error message did not match
> with the error, for example:
> 
> $ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
> qemu-system-x86_64: -enable-kvm: could not open disk image /home/mathslinux/Images/debian.qcow2: No such file or directory
> 
> Indeed, the file which cant be created is /tmp/bad_path/vl.xxxxxx,  not
> debian.qcow2. so the error message makes users feel confused.
> 
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> ---
> v1 -> v2:
>    Output error message only if fd < 0
> v2 -> v3:
>    Output error message in the caller of get_tmp_filename()
>  block.c | 2 ++
>  1 个文件被修改,插入 2 行(+)
> 
> diff --git a/block.c b/block.c
> index 470bdcc..074987e 100644
> --- a/block.c
> +++ b/block.c
> @@ -764,6 +764,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>  
>          ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
>          if (ret < 0) {
> +            fprintf(stderr, "Could not create temporary snapshot %s: %s\n",
> +                    tmp_filename, strerror(errno));

This should be strerror(-ret). Also consider using error_report()
instead of fprintf().

>              return ret;
>          }
>  

Kevin

Patch

diff --git a/block.c b/block.c
index 470bdcc..074987e 100644
--- a/block.c
+++ b/block.c
@@ -764,6 +764,8 @@  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
 
         ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
         if (ret < 0) {
+            fprintf(stderr, "Could not create temporary snapshot %s: %s\n",
+                    tmp_filename, strerror(errno));
             return ret;
         }