Re: [Qemu-devel] [PATCH 3/5] ide: Generate BLOCK_IO_ERROR QMP event

[Kevin Wolf]

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 <address@hidden>
>> ---
>>  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.

Kevin