Re: [Qemu-devel] [PATCH] bdrv_flush error handling

[Daniel P. Berrange]

On Wed, Feb 20, 2008 at 03:53:46PM +0000, Ian Jackson wrote:
Content-Description: message body text
> bdrv_flush is declared to return void, but this is wrong because it
> means that the implementations have nowhere to report their errors.
> Indeed, the implementations generally ignore errors.
> 
> This patch corrects this by making it return int (implicitly, either 0
> or -errno, as for other similar functions).  All of the
> implementations and callers are adjusted too.
> 
> 
> While looking at this I was surprised to see this:
> 
>     static void scsi_write_complete(void * opaque, int ret)
>     ...
>       if (ret) {
>           fprintf(stderr, "scsi-disc: IO write error\n");
>           exit(1);
>       }
> 
> Surely that is overkill ?

Yes, any i/o errors I'd expect to be propagated back up to the guest
OS as the most appropriate IDE / SCSI error code.

> Also, in block-raw-posix.c, raw_pwrite et al seem to return -1 on
> error (the return value from write) whereas the other block read/write
> methods return errno values.  This is a mistake, surely ?  -1 would be
> -EPERM.  If any of the callers did anything with these return values
> you'd get incorrect error indications.
> 
> 
> Finally, it would perhaps be best if the block device emulators wrote
> to the qemu console to complain if they give write errors.  Otherwise
> the errno value and other important information will be lost, which
> makes debugging hard.

If by 'qemu console' you mean stderr, then fine, but please don't
spew log messages to the  monitor console, because that'll make it
very hard to interact with reliably from management tools. 

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|