From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ncdc5-0008LE-8r for qemu-devel@nongnu.org; Wed, 03 Feb 2010 06:41:41 -0500 Received: from [199.232.76.173] (port=35530 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ncdc4-0008L6-Nd for qemu-devel@nongnu.org; Wed, 03 Feb 2010 06:41:40 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Ncdc3-0003SC-5p for qemu-devel@nongnu.org; Wed, 03 Feb 2010 06:41:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:61157) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ncdc2-0003Ry-Lk for qemu-devel@nongnu.org; Wed, 03 Feb 2010 06:41:38 -0500 Date: Wed, 3 Feb 2010 09:41:28 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 3/5] ide: Generate BLOCK_IO_ERROR QMP event Message-ID: <20100203094128.0d7f77f8@doriath> In-Reply-To: <4B693DA3.6080702@redhat.com> References: <1265145013-23231-1-git-send-email-lcapitulino@redhat.com> <1265145013-23231-4-git-send-email-lcapitulino@redhat.com> <20100203083107.GA28969@lst.de> <4B693DA3.6080702@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Christoph Hellwig , qemu-devel@nongnu.org On Wed, 03 Feb 2010 10:10:59 +0100 Kevin Wolf wrote: > Am 03.02.2010 09:31, schrieb Christoph Hellwig: > > On Tue, Feb 02, 2010 at 07:10:11PM -0200, Luiz Capitulino wrote: > >> Just call bdrv_mon_event() in the right place. > >> > >> Signed-off-by: Luiz Capitulino > >> --- > >> hw/ide/core.c | 6 +++++- > >> 1 files changed, 5 insertions(+), 1 deletions(-) > >> > >> diff --git a/hw/ide/core.c b/hw/ide/core.c > >> index b6643e8..603e537 100644 > >> --- a/hw/ide/core.c > >> +++ b/hw/ide/core.c > >> @@ -480,14 +480,17 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) > >> int is_read = (op & BM_STATUS_RETRY_READ); > >> BlockInterfaceErrorAction action = drive_get_on_error(s->bs, is_read); > >> > >> - if (action == BLOCK_ERR_IGNORE) > >> + if (action == BLOCK_ERR_IGNORE) { > >> + bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read); > >> return 0; > >> + } > >> > >> if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC) > >> || action == BLOCK_ERR_STOP_ANY) { > >> s->bus->bmdma->unit = s->unit; > >> s->bus->bmdma->status |= op; > >> vm_stop(0); > >> + bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); > > > > Why isn't the event directly sent from drive_get_on_error? Having > > to opencode this in every driver is a sure way to make sure it's > > going to be broken somewhere. > > Because drive_get_on_error isn't an event handler and shouldn't have any > side effects. It might be called anywhere. And it doesn't know the error > code, so it can't even decide if the VM has stopped or not. > > Maybe we could look at writing a generic handle_rw_error function for > all block devices. They look pretty much the same in every driver, even > without the monitor event. Any design in mind? I could try this later (preferably after the event series is merged). What if block devices register the following callbacks somewhere: - handle_rw_error_ignore() - handle_rw_error_stop() - handle_rw_error_report() But then they'd have to trigger their call by calling, say, block_handle_rw_error() from the error site.