[PATCH v4 2/4] block/nbd: define new max_write_zero_fast limit

Vladimir Sementsov-Ogievskiy posted 4 patches 5 years, 8 months ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>, Eric Blake <eblake@redhat.com>
[PATCH v4 2/4] block/nbd: define new max_write_zero_fast limit
Posted by Vladimir Sementsov-Ogievskiy 5 years, 8 months ago
The NBD spec was recently updated to clarify that max_block doesn't
relate to NBD_CMD_WRITE_ZEROES with NBD_CMD_FLAG_FAST_ZERO (which
mirrors Qemu flag BDRV_REQ_NO_FALLBACK).

bs->bl.max_write_zero_fast is zero by default which means using
max_pwrite_zeroes. Update nbd driver to allow larger requests with
BDRV_REQ_NO_FALLBACK.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 4ac23c8f62..b0584cf68d 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1956,6 +1956,7 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
 
     bs->bl.request_alignment = min;
     bs->bl.max_pdiscard = QEMU_ALIGN_DOWN(INT_MAX, min);
+    bs->bl.max_pwrite_zeroes_fast = bs->bl.max_pdiscard;
     bs->bl.max_pwrite_zeroes = max;
     bs->bl.max_transfer = max;
 
-- 
2.21.0


Re: [PATCH v4 2/4] block/nbd: define new max_write_zero_fast limit
Posted by Eric Blake 5 years, 6 months ago
On 6/11/20 11:26 AM, Vladimir Sementsov-Ogievskiy wrote:
> The NBD spec was recently updated to clarify that max_block doesn't
> relate to NBD_CMD_WRITE_ZEROES with NBD_CMD_FLAG_FAST_ZERO (which
> mirrors Qemu flag BDRV_REQ_NO_FALLBACK).
> 
> bs->bl.max_write_zero_fast is zero by default which means using
> max_pwrite_zeroes. Update nbd driver to allow larger requests with
> BDRV_REQ_NO_FALLBACK.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/nbd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 4ac23c8f62..b0584cf68d 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -1956,6 +1956,7 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
>   
>       bs->bl.request_alignment = min;
>       bs->bl.max_pdiscard = QEMU_ALIGN_DOWN(INT_MAX, min);
> +    bs->bl.max_pwrite_zeroes_fast = bs->bl.max_pdiscard;
>       bs->bl.max_pwrite_zeroes = max;

Do we even need max_pwrite_zeroes_fast?  Doesn't qemu behave correctly 
if we just blindly assign max_pdiscard and max_pwrite_zeroes to the same 
value near 2G?

>       bs->bl.max_transfer = max;
>   
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v4 2/4] block/nbd: define new max_write_zero_fast limit
Posted by Vladimir Sementsov-Ogievskiy 5 years, 5 months ago
23.07.2020 23:32, Eric Blake wrote:
> On 6/11/20 11:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>> The NBD spec was recently updated to clarify that max_block doesn't
>> relate to NBD_CMD_WRITE_ZEROES with NBD_CMD_FLAG_FAST_ZERO (which
>> mirrors Qemu flag BDRV_REQ_NO_FALLBACK).
>>
>> bs->bl.max_write_zero_fast is zero by default which means using
>> max_pwrite_zeroes. Update nbd driver to allow larger requests with
>> BDRV_REQ_NO_FALLBACK.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/nbd.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 4ac23c8f62..b0584cf68d 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -1956,6 +1956,7 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
>>       bs->bl.request_alignment = min;
>>       bs->bl.max_pdiscard = QEMU_ALIGN_DOWN(INT_MAX, min);
>> +    bs->bl.max_pwrite_zeroes_fast = bs->bl.max_pdiscard;
>>       bs->bl.max_pwrite_zeroes = max;
> 
> Do we even need max_pwrite_zeroes_fast?  Doesn't qemu behave correctly if we just blindly assign max_pdiscard and max_pwrite_zeroes to the same value near 2G?
> 

Without BDRV_REQ_NO_FALLBACK, max_block is an actual limit for WRITE_ZERO.. So, in my understanding, it doesn't


-- 
Best regards,
Vladimir