Re: [Qemu-devel] [PATCH 1/6] block: Keep track of devices' I/O status

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Luiz Capitulino <address@hidden> writes:
>
>> This commit adds support to the BlockDriverState type to keep track
>> of devices' I/O status.
>>
>> There are three possible status: BDRV_IOS_OK (no error), BDRV_IOS_ENOSPC
>> (no space error) and BDRV_IOS_FAILED (any other error). The distinction
>> between no space and other errors is important because a management
>> application may want to watch for no space in order to extend the
>> space assigned to the VM and put it to run again.
>>
>> Qemu devices supporting the I/O status feature have to enable it
>> explicitly by calling bdrv_iostatus_enable() _and_ have to be
>> configured to stop the VM on errors (ie. werror=stop|enospc or
>> rerror=stop).
>>
>> In case of multiple errors being triggered in sequence only the first
>> one is stored. The I/O status is always reset to BDRV_IOS_OK when the
>> 'cont' command is issued.
>>
>> Next commits will add support to some devices and extend the
>> query-block/info block commands to return the I/O status information.
>>
>> Signed-off-by: Luiz Capitulino <address@hidden>
>> ---
>>  block.c     |   32 ++++++++++++++++++++++++++++++++
>>  block.h     |    9 +++++++++
>>  block_int.h |    1 +
>>  monitor.c   |    6 ++++++
>>  4 files changed, 48 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index e3fe97f..fbd90b4 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -221,6 +221,7 @@ BlockDriverState *bdrv_new(const char *device_name)
>>      if (device_name[0] != '\0') {
>>          QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
>>      }
>> +    bs->iostatus = BDRV_IOS_INVAL;
>>      return bs;
>>  }
>>  
>> @@ -3181,6 +3182,37 @@ int bdrv_in_use(BlockDriverState *bs)
>>      return bs->in_use;
>>  }
>>  
>> +void bdrv_iostatus_enable(BlockDriverState *bs)
>> +{
>> +    assert(bs->iostatus == BDRV_IOS_INVAL);
>> +    bs->iostatus = BDRV_IOS_OK;
>> +}
>
> bdrv_new() creates the BDS with I/O status tracking disabled.  Devices
> that do tracking declare that by calling this function during
> initialization.  Enables tracking if the BDS has suitable on_write_error
> and on_read_error.
>
> If a device gets hot unplugged, tracking remains enabled.  If the BDS
> then gets reused with a device that doesn't do tracking, I/O status
> becomes incorrect.  Can't happen right now, because we automatically
> delete the BDS on hot unplug, but it's a trap.

And if the BDS gets reused with a device that does tracking, the
assertion above fails.

> Suggest to disable tracking in bdrv_detach_dev() or bdrv_attach_dev().

[...]