[Qemu-devel] [PATCH] Clarify error msg when open of backing file fails

[Qemu-devel] [PATCH] Clarify error msg when open of backing file fails

[Beth Kon]

This patch clarifies the error message when a problem is encountered
opening a backing file. Currently the only message printed in this case
is:

" qemu: could not open disk image /path/to/image"

This can be confusing if the real error lies in finding the backing file
rather than the primary file.


diffstat output:
 block.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

Signed-off-by: Beth Kon <eak@us.ibm.com>

---
diff --git a/block.c b/block.c
index bd647a6..c99626f 100644
--- a/block.c
+++ b/block.c
@@ -416,8 +416,11 @@ int bdrv_open2(BlockDriverState *bs, const char
*filename, 
         }
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
-        if (bdrv_open(bs->backing_hd, backing_filename, 0) < 0)
-            goto fail;
+        if (bdrv_open(bs->backing_hd, backing_filename, 0) < 0) {
+            fprintf(stderr, "qemu: could not open backing file %s\n",
+                                             backing_filename);
+            goto fail; 
+        }    
     }
 
     /* call the change callback */


-- 
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak@us.ibm.com

Re: [Qemu-devel] [PATCH] Clarify error msg when open of backing file fails

[Aurelien Jarno]

On Thu, May 08, 2008 at 01:50:08PM -0400, Beth Kon wrote:
> This patch clarifies the error message when a problem is encountered
> opening a backing file. Currently the only message printed in this case
> is:
> 
> " qemu: could not open disk image /path/to/image"
> 
> This can be confusing if the real error lies in finding the backing file
> rather than the primary file.

While I agree this is very confusing (and backing file not found is not
the only case), I don't think the current patch is correct. It will 
print the error message on the console, even if the action causing the 
problem is not done from there (from the monitor for example).

I think we should rework the error handling in in block* to correctly
display errors on the console or on the monitor.

One solution could be to define negative constants for each of the
errors, and propagate it back to the original caller. That way it can
display the error message at the correct place.

I haven't thinked a lot about that, better solutions may exist.
 
> diffstat output:
>  block.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> Signed-off-by: Beth Kon <eak@us.ibm.com>
> 
> ---
> diff --git a/block.c b/block.c
> index bd647a6..c99626f 100644
> --- a/block.c
> +++ b/block.c
> @@ -416,8 +416,11 @@ int bdrv_open2(BlockDriverState *bs, const char
> *filename, 
>          }
>          path_combine(backing_filename, sizeof(backing_filename),
>                       filename, bs->backing_file);
> -        if (bdrv_open(bs->backing_hd, backing_filename, 0) < 0)
> -            goto fail;
> +        if (bdrv_open(bs->backing_hd, backing_filename, 0) < 0) {
> +            fprintf(stderr, "qemu: could not open backing file %s\n",
> +                                             backing_filename);
> +            goto fail; 
> +        }    
>      }
>  
>      /* call the change callback */
> 
> 
> -- 
> Elizabeth Kon (Beth)
> IBM Linux Technology Center
> Open Hypervisor Team
> email: eak@us.ibm.com
> 
> 
> 
> 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net