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

[risc@volumehost.com]

On Thu, Feb 21, 2008 at 05:24:10PM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 21, 2008 at 12:19:22PM -0500, Ben Taylor wrote:
> > 
> > ---- "Daniel P. Berrange" <berrange@redhat.com> 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?
> 
> Why invent a new special QEMU log screen, when stderr works just fine. If an
> app wants to capture log messages they just capture stderr and persist it.
> We already capture stderr for exactly this reason in libvirt when managing
> QEMU instances.
> 
> 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  -=| 
> 

Please excuse my intrusion in this thread, but I'm a user of the new
ncurses user interface. when ssh'd in, running qemu, I don't believe 
having messages pop out of stderr and over the current screen contents is 
the appropriate behavior, as it sounds to me like it would cause redraw 
defects in the normal text console (via ncurses)

Julia Longtin <risc@volumehost.com>