Re: [Qemu-devel] [PATCH v2 7/9] blkdebug: Add pass-through write_zero and discard support

[Max Reitz]

On 17.11.2016 21:14, Eric Blake wrote:
> In order to test the effects of artificial geometry constraints
> on operations like write zero or discard, we first need blkdebug
> to manage these actions.  Ideally, it would be nice to let these
> operations also react to injected errors like read/write/flush,
> but it is not trivial to turn bdrv_aio error injection (where
> we return BlockAIOCB*) into bdrv_co (where we return int), not
> to mention the fact that I don't want to conflict with Kevin's
> concurrent work on refactoring away from bdrv_aio.  So for now,
> the operations merely have a TODO comment for adding error
> injection.
> 
> However, one thing we CAN test is the contract promised by the
> block layer; namely, if a device has specified limits on
> alignment or maximum size, then those limits must be obeyed (for
> now, the blkdebug driver merely inherits limits from whatever it
> is wrapping, but the next patch will further enhance it to allow
> specific limit overrides).
> 
> Tested by setting up an NBD server with export 'foo', then invoking:
> $ ./qemu-io
> qemu-io> open -o driver=blkdebug blkdebug::nbd://localhost:10809/foo
> qemu-io> d 0 15M
> qemu-io> w -z 0 15M
> 
> Pre-patch, the server never sees the discard (it was silently
> eaten by the block layer); post-patch it is passed across the
> wire.  Likewise, pre-patch the write is always passed with
> NBD_WRITE (with 15M of zeroes on the wire), while post-patch
> it can utilize NBD_WRITE_ZEROES (for less traffic).
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>  block/blkdebug.c | 61 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 0a47977..d45826d 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c

[...]

> @@ -522,6 +528,59 @@ static BlockAIOCB *blkdebug_aio_flush(BlockDriverState 
> *bs,
>  }
> 
> 
> +static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
> +                                                  int64_t offset, int count,
> +                                                  BdrvRequestFlags flags)
> +{
> +    uint32_t align = MAX(bs->bl.request_alignment,
> +                         bs->bl.pwrite_zeroes_alignment);
> +
> +    /* Regardless of whether the lower layer has a finer granularity,
> +     * we want to treat any unaligned request as unsupported, and

Why?

Assuming there is a reason:

Reviewed-by: Max Reitz <address@hidden>

> +     * check that the block layer never hands us anything crossing an
> +     * alignment boundary.  */
> +    if (count < align) {
> +        return -ENOTSUP;
> +    }

[...]

signature.asc
Description: OpenPGP digital signature