Date: Fri, 9 May 2008 20:43:26 +0200
From: Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH] Clarify error msg when open of backing
	file fails
Message-ID: <20080509184326.GA9188@volta.aurel32.net>
References: <1210269009.6551.17.camel@beth-ubuntu>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1210269009.6551.17.camel@beth-ubuntu>
Reply-To: qemu-devel@nongnu.org
To: qemu-devel@nongnu.org

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