[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
From: |
Ben Taylor |
Subject: |
Re: [Qemu-devel] [PATCH] bdrv_flush error handling |
Date: |
Thu, 21 Feb 2008 12:19:22 -0500 |
---- "Daniel P. Berrange" <address@hidden> wrote:
> 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.
Would it make sense to have a log messages screen associated with
the monitor (like Ctrl-Alt-7) to deal with those sorts of things?
Ben
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, (continued)
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Ian Jackson, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Paul Brook, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Ian Jackson, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Jamie Lokier, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Paul Brook, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Jamie Lokier, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Daniel P. Berrange, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Anthony Liguori, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Rick Vernam, 2008/02/20
- Re: [Qemu-devel] [PATCH] bdrv_flush error handling, Thomas Irlet, 2008/02/21
Re: [Qemu-devel] [PATCH] bdrv_flush error handling,
Ben Taylor <=