Re: [Qemu-devel] [PATCH] block: extend BLOCK_IO_ERROR event with nospace indicator

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 29 Aug 2014 16:07:27 -0400
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> Management software, such as RHEV's vdsm, want to be able to allocate
> disk space on demand. The basic use case is to start a VM with a small
> disk and then the disk is enlarged when QEMU hits a ENOSPC condition.
> 
> To this end, the management software has to be notified when QEMU
> encounters ENOSPC. The solution implemented by this commit is simple:
> it extends the BLOCK_IO_ERROR with a 'nospace' key, which is true
> when QEMU is stopped due to ENOSPC.
> 
> Note that support for querying this event is already present in
> query-block by means of the 'io-status' key. Also, the new 'nospace'
> BLOCK_IO_ERROR field shares the same semantics with 'io-status',
> which basically means that werror= has to be set to either
> 'stop' or 'enospc' to enable 'nospace'.
> 
> Finally, this commit also updates the 'io-status' key doc in the
> schema with a list of supported device models.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

Kevin, are you going to take this via block layer tree?

> ---
> 
> Three important observations:
> 
>  1. We've talked with oVirt and OpenStack folks. oVirt folks say that
>     this implementation is enough for their use-case. OpenStack don't
>     need this feature
> 
>  2. While testing this with a raw image on a (smaller) ext2 file mounted
>     via the loopback device, I get half "Invalid argument" I/O errors and
>     half  "No space" errors". This means that half of the BLOCK_IO_ERROR
>     events that are emitted for this test-case will have nospace=false
>     and the other half nospace=true. I don't know why I'm getting those
>     "Invalid argument" errors, can anyone of the block layer comment
>     on this? I don't get that with a qcow2 image (I get nospace=true for
>     all events)
> 
>  3. I think this should go via block tree
> 
>  block.c              | 22 ++++++++++++++--------
>  qapi/block-core.json |  8 +++++++-
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 1df13ac..b334e35 100644
> --- a/block.c
> +++ b/block.c
> @@ -3632,6 +3632,18 @@ BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int e
>      }
>  }
>  
> +static void send_qmp_error_event(BlockDriverState *bs,
> +                                 BlockErrorAction action,
> +                                 bool is_read, int error)
> +{
> +    BlockErrorAction ac;
> +
> +    ac = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
> +    qapi_event_send_block_io_error(bdrv_get_device_name(bs), ac, action,
> +                                   bdrv_iostatus_is_enabled(bs),
> +                                   error == ENOSPC, &error_abort);
> +}
> +
>  /* This is done by device models because, while the block layer knows
>   * about the error, it does not know whether an operation comes from
>   * the device or the block layer (from a job, for example).
> @@ -3657,16 +3669,10 @@ void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
>           * also ensures that the STOP/RESUME pair of events is emitted.
>           */
>          qemu_system_vmstop_request_prepare();
> -        qapi_event_send_block_io_error(bdrv_get_device_name(bs),
> -                                       is_read ? IO_OPERATION_TYPE_READ :
> -                                       IO_OPERATION_TYPE_WRITE,
> -                                       action, &error_abort);
> +        send_qmp_error_event(bs, action, is_read, error);
>          qemu_system_vmstop_request(RUN_STATE_IO_ERROR);
>      } else {
> -        qapi_event_send_block_io_error(bdrv_get_device_name(bs),
> -                                       is_read ? IO_OPERATION_TYPE_READ :
> -                                       IO_OPERATION_TYPE_WRITE,
> -                                       action, &error_abort);
> +        send_qmp_error_event(bs, action, is_read, error);
>      }
>  }
>  
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index fb74c56..567e0a6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -336,6 +336,7 @@
>  #
>  # @io-status: #optional @BlockDeviceIoStatus. Only present if the device
>  #             supports it and the VM is configured to stop on errors
> +#             (supported device models: virtio-blk, ide, scsi-disk)
>  #
>  # @inserted: #optional @BlockDeviceInfo describing the device if media is
>  #            present
> @@ -1569,6 +1570,11 @@
>  #
>  # @action: action that has been taken
>  #
> +# @nospace: #optional true if I/O error was caused due to a no-space
> +#           condition. This key is only present if query-block's
> +#           io-status is present, please see query-block documentation
> +#           for more information (since: 2.2)
> +#
>  # Note: If action is "stop", a STOP event will eventually follow the
>  # BLOCK_IO_ERROR event
>  #
> @@ -1576,7 +1582,7 @@
>  ##
>  { 'event': 'BLOCK_IO_ERROR',
>    'data': { 'device': 'str', 'operation': 'IoOperationType',
> -            'action': 'BlockErrorAction' } }
> +            'action': 'BlockErrorAction', '*nospace': 'bool' } }
>  
>  ##
>  # @BLOCK_JOB_COMPLETED