On 11/17/2016 04:47 PM, Max Reitz wrote:
> 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 <ebl...@redhat.com>
>> ---
>> 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?Hmm, at the moment, I'm having a hard time coming up with a strong reason why I did that. I'll retest without it, and see if it still picks up the regression fixed by 3/5; if so I'll drop it as part of the respin (since I still have the iotest to fix); if not, I'll have a good reason why and include it in the commit message. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
