On Wed, 03 Feb 2010 10:10:59 +0100
Kevin Wolf <kw...@redhat.com> 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 <lcapitul...@redhat.com>
> >> ---
> >> 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.
