This is necessary for efficient block-status export, for clients which
support it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 77 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 58 insertions(+), 19 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index ea5fe0eb33..a6d013aec4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1844,11 +1844,22 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
return ret;
}
-static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, NBDExtent *extent)
+/*
+ * Populate @extents from block status. Store in @bytes the byte length encoded
+ * (which may be smaller but (unlike bitmap_to_extents) _never_ larger than the
+ * original), and store in @nb_extents the number of extents used.
+ *
+ * Returns zero on success and -errno on bdrv_block_status_above failure.
+ */
+static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
+ uint64_t *bytes, NBDExtent *extents,
+ unsigned int *nb_extents)
{
- uint64_t remaining_bytes = bytes;
+ uint64_t remaining_bytes = *bytes;
+ NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
+ bool first_extent = true;
+ assert(*nb_extents);
while (remaining_bytes) {
uint32_t flags;
int64_t num;
@@ -1860,21 +1871,40 @@ static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
flags = (ret & BDRV_BLOCK_ALLOCATED ? 0 : NBD_STATE_HOLE) |
(ret & BDRV_BLOCK_ZERO ? NBD_STATE_ZERO : 0);
+ offset += num;
+ remaining_bytes -= num;
- if (remaining_bytes == bytes) {
+ if (first_extent) {
extent->flags = flags;
+ extent->length = num;
+ first_extent = false;
+ continue;
}
- if (flags != extent->flags) {
- break;
+ if (flags == extent->flags) {
+ /* extend current extent */
+ extent->length += num;
+ } else {
+ if (extent + 1 == extents_end) {
+ break;
+ }
+
+ /* start new extent */
+ extent++;
+ extent->flags = flags;
+ extent->length = num;
}
+ }
- offset += num;
- remaining_bytes -= num;
+ extents_end = extent + 1;
+
+ for (extent = extents; extent < extents_end; extent++) {
+ cpu_to_be32s(&extent->flags);
+ cpu_to_be32s(&extent->length);
}
- cpu_to_be32s(&extent->flags);
- extent->length = cpu_to_be32(bytes - remaining_bytes);
+ *bytes -= remaining_bytes;
+ *nb_extents = extents_end - extents;
return 0;
}
@@ -1910,21 +1940,29 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
/* Get block status from the exported device and send it to the client */
static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
BlockDriverState *bs, uint64_t offset,
- uint32_t length, bool last,
- uint32_t context_id, Error **errp)
+ uint32_t length, bool dont_fragment,
+ bool last, uint32_t context_id,
+ Error **errp)
{
int ret;
- NBDExtent extent;
+ unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
+ NBDExtent *extents = g_new(NBDExtent, nb_extents);
+ uint64_t final_length = length;
- ret = blockstatus_to_extent_be(bs, offset, length, &extent);
+ ret = blockstatus_to_extents(bs, offset, &final_length, extents,
+ &nb_extents);
if (ret < 0) {
+ g_free(extents);
return nbd_co_send_structured_error(
client, handle, -ret, "can't get block status", errp);
}
- return nbd_co_send_extents(client, handle, &extent, 1,
- be32_to_cpu(extent.length), last,
- context_id, errp);
+ ret = nbd_co_send_extents(client, handle, extents, nb_extents,
+ final_length, last, context_id, errp);
+
+ g_free(extents);
+
+ return ret;
}
/*
@@ -2228,10 +2266,11 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
(client->export_meta.base_allocation ||
client->export_meta.bitmap))
{
+ bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
if (client->export_meta.base_allocation) {
ret = nbd_co_send_block_status(client, request->handle,
blk_bs(exp->blk), request->from,
- request->len,
+ request->len, dont_fragment,
!client->export_meta.bitmap,
NBD_META_ID_BASE_ALLOCATION,
errp);
@@ -2244,7 +2283,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
ret = nbd_co_send_bitmap(client, request->handle,
client->exp->export_bitmap,
request->from, request->len,
- request->flags & NBD_CMD_FLAG_REQ_ONE,
+ dont_fragment,
true, NBD_META_ID_DIRTY_BITMAP, errp);
if (ret < 0) {
return ret;
--
2.11.1
On 07/04/2018 06:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> This is necessary for efficient block-status export, for clients which
> support it.
qemu as client doesn't currently process additional information when
querying block status. Should it?
Are there other clients which DO make use of the additional information?
This is a feature, so it is indeed 3.1 material.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 77 +++++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 58 insertions(+), 19 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index ea5fe0eb33..a6d013aec4 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1844,11 +1844,22 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
> return ret;
> }
>
> -static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, NBDExtent *extent)
> +/*
> + * Populate @extents from block status. Store in @bytes the byte length encoded
> + * (which may be smaller but (unlike bitmap_to_extents) _never_ larger than the
> + * original), and store in @nb_extents the number of extents used.
I don't think the comparison to bitmap_to_extents in a nested
parenthetical buys us anything; the shorter:
(which may be smaller than the original)
would do just fine for the function contract.
> + *
> + * Returns zero on success and -errno on bdrv_block_status_above failure.
Is it worth returning a positive value for one less pointer parameter
used for output?
> + */
> +static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
> + uint64_t *bytes, NBDExtent *extents,
> + unsigned int *nb_extents)
> {
> - uint64_t remaining_bytes = bytes;
> + uint64_t remaining_bytes = *bytes;
> + NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
So both bytes and nb_extents are in-out.
> @@ -1910,21 +1940,29 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
> /* Get block status from the exported device and send it to the client */
> static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
> BlockDriverState *bs, uint64_t offset,
> - uint32_t length, bool last,
> - uint32_t context_id, Error **errp)
> + uint32_t length, bool dont_fragment,
> + bool last, uint32_t context_id,
> + Error **errp)
> {
> int ret;
> - NBDExtent extent;
> + unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
> + NBDExtent *extents = g_new(NBDExtent, nb_extents);
> + uint64_t final_length = length;
>
> - ret = blockstatus_to_extent_be(bs, offset, length, &extent);
> + ret = blockstatus_to_extents(bs, offset, &final_length, extents,
> + &nb_extents);
> if (ret < 0) {
> + g_free(extents);
> return nbd_co_send_structured_error(
> client, handle, -ret, "can't get block status", errp);
> }
>
> - return nbd_co_send_extents(client, handle, &extent, 1,
> - be32_to_cpu(extent.length), last,
> - context_id, errp);
> + ret = nbd_co_send_extents(client, handle, extents, nb_extents,
> + final_length, last, context_id, errp);
> +
> + g_free(extents);
> +
At any rate, this conversion looks sane.
> + return ret;
> }
>
> /*
> @@ -2228,10 +2266,11 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
> (client->export_meta.base_allocation ||
> client->export_meta.bitmap))
> {
> + bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
> if (client->export_meta.base_allocation) {
Blank lines between declarations and statements can aid in legibility; I
can add when queuing.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
05.07.2018 18:59, Eric Blake wrote:
> On 07/04/2018 06:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> This is necessary for efficient block-status export, for clients which
>> support it.
>
> qemu as client doesn't currently process additional information when
> querying block status. Should it?
>
> Are there other clients which DO make use of the additional information?
Actually, we have one in development. block status export is used for
external full backups, like bitmap export - for external incremental
backups.
>
> This is a feature, so it is indeed 3.1 material.
>
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> nbd/server.c | 77
>> +++++++++++++++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 58 insertions(+), 19 deletions(-)
>>
>> diff --git a/nbd/server.c b/nbd/server.c
>> index ea5fe0eb33..a6d013aec4 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -1844,11 +1844,22 @@ static int coroutine_fn
>> nbd_co_send_sparse_read(NBDClient *client,
>> return ret;
>> }
>> -static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t
>> offset,
>> - uint64_t bytes, NBDExtent *extent)
>> +/*
>> + * Populate @extents from block status. Store in @bytes the byte
>> length encoded
>> + * (which may be smaller but (unlike bitmap_to_extents) _never_
>> larger than the
>> + * original), and store in @nb_extents the number of extents used.
>
> I don't think the comparison to bitmap_to_extents in a nested
> parenthetical buys us anything; the shorter:
>
> (which may be smaller than the original)
>
> would do just fine for the function contract.
>
>> + *
>> + * Returns zero on success and -errno on bdrv_block_status_above
>> failure.
>
> Is it worth returning a positive value for one less pointer parameter
> used for output?
I think, that if we have one such parameter anyway, let's use same path
for very similar thing, for consistency.
>
>> + */
>> +static int blockstatus_to_extents(BlockDriverState *bs, uint64_t
>> offset,
>> + uint64_t *bytes, NBDExtent *extents,
>> + unsigned int *nb_extents)
>> {
>> - uint64_t remaining_bytes = bytes;
>> + uint64_t remaining_bytes = *bytes;
>> + NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
>
> So both bytes and nb_extents are in-out.
>
>
>> @@ -1910,21 +1940,29 @@ static int nbd_co_send_extents(NBDClient
>> *client, uint64_t handle,
>> /* Get block status from the exported device and send it to the
>> client */
>> static int nbd_co_send_block_status(NBDClient *client, uint64_t
>> handle,
>> BlockDriverState *bs, uint64_t
>> offset,
>> - uint32_t length, bool last,
>> - uint32_t context_id, Error **errp)
>> + uint32_t length, bool
>> dont_fragment,
>> + bool last, uint32_t context_id,
>> + Error **errp)
>> {
>> int ret;
>> - NBDExtent extent;
>> + unsigned int nb_extents = dont_fragment ? 1 :
>> NBD_MAX_BITMAP_EXTENTS;
>> + NBDExtent *extents = g_new(NBDExtent, nb_extents);
>> + uint64_t final_length = length;
>> - ret = blockstatus_to_extent_be(bs, offset, length, &extent);
>> + ret = blockstatus_to_extents(bs, offset, &final_length, extents,
>> + &nb_extents);
>> if (ret < 0) {
>> + g_free(extents);
>> return nbd_co_send_structured_error(
>> client, handle, -ret, "can't get block status", errp);
>> }
>> - return nbd_co_send_extents(client, handle, &extent, 1,
>> - be32_to_cpu(extent.length), last,
>> - context_id, errp);
>> + ret = nbd_co_send_extents(client, handle, extents, nb_extents,
>> + final_length, last, context_id, errp);
>> +
>> + g_free(extents);
>> +
>
> At any rate, this conversion looks sane.
>
>> + return ret;
>> }
>> /*
>> @@ -2228,10 +2266,11 @@ static coroutine_fn int
>> nbd_handle_request(NBDClient *client,
>> (client->export_meta.base_allocation ||
>> client->export_meta.bitmap))
>> {
>> + bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
>> if (client->export_meta.base_allocation) {
>
> Blank lines between declarations and statements can aid in legibility;
> I can add when queuing.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Ok, thank you!
--
Best regards,
Vladimir
05.07.2018 19:18, Vladimir Sementsov-Ogievskiy wrote:
> 05.07.2018 18:59, Eric Blake wrote:
>> On 07/04/2018 06:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> This is necessary for efficient block-status export, for clients which
>>> support it.
>>
>> qemu as client doesn't currently process additional information when
>> querying block status. Should it?
>>
>> Are there other clients which DO make use of the additional information?
>
> Actually, we have one in development. block status export is used for
> external full backups, like bitmap export - for external incremental
> backups.
>
>>
>> This is a feature, so it is indeed 3.1 material.
>>
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> nbd/server.c | 77
>>> +++++++++++++++++++++++++++++++++++++++++++++---------------
>>> 1 file changed, 58 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/nbd/server.c b/nbd/server.c
>>> index ea5fe0eb33..a6d013aec4 100644
>>> --- a/nbd/server.c
>>> +++ b/nbd/server.c
>>> @@ -1844,11 +1844,22 @@ static int coroutine_fn
>>> nbd_co_send_sparse_read(NBDClient *client,
>>> return ret;
>>> }
>>> -static int blockstatus_to_extent_be(BlockDriverState *bs,
>>> uint64_t offset,
>>> - uint64_t bytes, NBDExtent *extent)
>>> +/*
>>> + * Populate @extents from block status. Store in @bytes the byte
>>> length encoded
>>> + * (which may be smaller but (unlike bitmap_to_extents) _never_
>>> larger than the
>>> + * original), and store in @nb_extents the number of extents used.
>>
>> I don't think the comparison to bitmap_to_extents in a nested
>> parenthetical buys us anything; the shorter:
>>
>> (which may be smaller than the original)
>>
>> would do just fine for the function contract.
>>
>>> + *
>>> + * Returns zero on success and -errno on bdrv_block_status_above
>>> failure.
>>
>> Is it worth returning a positive value for one less pointer parameter
>> used for output?
>
> I think, that if we have one such parameter anyway, let's use same
> path for very similar thing, for consistency.
>
>>
>>> + */
>>> +static int blockstatus_to_extents(BlockDriverState *bs, uint64_t
>>> offset,
>>> + uint64_t *bytes, NBDExtent *extents,
>>> + unsigned int *nb_extents)
>>> {
>>> - uint64_t remaining_bytes = bytes;
>>> + uint64_t remaining_bytes = *bytes;
>>> + NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
>>
>> So both bytes and nb_extents are in-out.
>>
>>
>>> @@ -1910,21 +1940,29 @@ static int nbd_co_send_extents(NBDClient
>>> *client, uint64_t handle,
>>> /* Get block status from the exported device and send it to the
>>> client */
>>> static int nbd_co_send_block_status(NBDClient *client, uint64_t
>>> handle,
>>> BlockDriverState *bs, uint64_t
>>> offset,
>>> - uint32_t length, bool last,
>>> - uint32_t context_id, Error **errp)
>>> + uint32_t length, bool
>>> dont_fragment,
>>> + bool last, uint32_t context_id,
>>> + Error **errp)
>>> {
>>> int ret;
>>> - NBDExtent extent;
>>> + unsigned int nb_extents = dont_fragment ? 1 :
>>> NBD_MAX_BITMAP_EXTENTS;
>>> + NBDExtent *extents = g_new(NBDExtent, nb_extents);
>>> + uint64_t final_length = length;
>>> - ret = blockstatus_to_extent_be(bs, offset, length, &extent);
>>> + ret = blockstatus_to_extents(bs, offset, &final_length, extents,
>>> + &nb_extents);
>>> if (ret < 0) {
>>> + g_free(extents);
>>> return nbd_co_send_structured_error(
>>> client, handle, -ret, "can't get block status",
>>> errp);
>>> }
>>> - return nbd_co_send_extents(client, handle, &extent, 1,
>>> - be32_to_cpu(extent.length), last,
>>> - context_id, errp);
>>> + ret = nbd_co_send_extents(client, handle, extents, nb_extents,
>>> + final_length, last, context_id, errp);
>>> +
>>> + g_free(extents);
>>> +
>>
>> At any rate, this conversion looks sane.
>>
>>> + return ret;
>>> }
>>> /*
>>> @@ -2228,10 +2266,11 @@ static coroutine_fn int
>>> nbd_handle_request(NBDClient *client,
>>> (client->export_meta.base_allocation ||
>>> client->export_meta.bitmap))
>>> {
>>> + bool dont_fragment = request->flags &
>>> NBD_CMD_FLAG_REQ_ONE;
>>> if (client->export_meta.base_allocation) {
>>
>> Blank lines between declarations and statements can aid in
>> legibility; I can add when queuing.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>
> Ok, thank you!
>
hmm, looks like lost patch :(
--
Best regards,
Vladimir
On 9/17/18 11:15 AM, Vladimir Sementsov-Ogievskiy wrote: > 05.07.2018 19:18, Vladimir Sementsov-Ogievskiy wrote: >> 05.07.2018 18:59, Eric Blake wrote: >>> On 07/04/2018 06:23 AM, Vladimir Sementsov-Ogievskiy wrote: >>>> This is necessary for efficient block-status export, for clients which >>>> support it. >>> >>> >>> This is a feature, so it is indeed 3.1 material. >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> >> >> Ok, thank you! >> > > hmm, looks like lost patch :( Not lost, just extremely slow getting queued. Now on my NBD queue. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
04.07.2018 14:23, Vladimir Sementsov-Ogievskiy wrote:
> This is necessary for efficient block-status export, for clients which
> support it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 77 +++++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 58 insertions(+), 19 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index ea5fe0eb33..a6d013aec4 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1844,11 +1844,22 @@ static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
> return ret;
> }
>
> -static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, NBDExtent *extent)
> +/*
> + * Populate @extents from block status. Store in @bytes the byte length encoded
> + * (which may be smaller but (unlike bitmap_to_extents) _never_ larger than the
> + * original), and store in @nb_extents the number of extents used.
> + *
> + * Returns zero on success and -errno on bdrv_block_status_above failure.
> + */
> +static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
> + uint64_t *bytes, NBDExtent *extents,
> + unsigned int *nb_extents)
> {
> - uint64_t remaining_bytes = bytes;
> + uint64_t remaining_bytes = *bytes;
> + NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
> + bool first_extent = true;
>
> + assert(*nb_extents);
> while (remaining_bytes) {
> uint32_t flags;
> int64_t num;
> @@ -1860,21 +1871,40 @@ static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
after bdrv_block_status_above, is there a guarantee that num > 0? Should
we add an assertion?
>
> flags = (ret & BDRV_BLOCK_ALLOCATED ? 0 : NBD_STATE_HOLE) |
> (ret & BDRV_BLOCK_ZERO ? NBD_STATE_ZERO : 0);
> + offset += num;
> + remaining_bytes -= num;
>
> - if (remaining_bytes == bytes) {
> + if (first_extent) {
> extent->flags = flags;
> + extent->length = num;
> + first_extent = false;
> + continue;
> }
>
> - if (flags != extent->flags) {
> - break;
> + if (flags == extent->flags) {
> + /* extend current extent */
> + extent->length += num;
> + } else {
> + if (extent + 1 == extents_end) {
> + break;
> + }
> +
> + /* start new extent */
> + extent++;
> + extent->flags = flags;
> + extent->length = num;
> }
> + }
>
> - offset += num;
> - remaining_bytes -= num;
> + extents_end = extent + 1;
> +
> + for (extent = extents; extent < extents_end; extent++) {
> + cpu_to_be32s(&extent->flags);
> + cpu_to_be32s(&extent->length);
> }
>
> - cpu_to_be32s(&extent->flags);
> - extent->length = cpu_to_be32(bytes - remaining_bytes);
> + *bytes -= remaining_bytes;
> + *nb_extents = extents_end - extents;
>
> return 0;
> }
> @@ -1910,21 +1940,29 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
> /* Get block status from the exported device and send it to the client */
> static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
> BlockDriverState *bs, uint64_t offset,
> - uint32_t length, bool last,
> - uint32_t context_id, Error **errp)
> + uint32_t length, bool dont_fragment,
> + bool last, uint32_t context_id,
> + Error **errp)
> {
> int ret;
> - NBDExtent extent;
> + unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
> + NBDExtent *extents = g_new(NBDExtent, nb_extents);
> + uint64_t final_length = length;
>
> - ret = blockstatus_to_extent_be(bs, offset, length, &extent);
> + ret = blockstatus_to_extents(bs, offset, &final_length, extents,
> + &nb_extents);
> if (ret < 0) {
> + g_free(extents);
> return nbd_co_send_structured_error(
> client, handle, -ret, "can't get block status", errp);
> }
>
> - return nbd_co_send_extents(client, handle, &extent, 1,
> - be32_to_cpu(extent.length), last,
> - context_id, errp);
> + ret = nbd_co_send_extents(client, handle, extents, nb_extents,
> + final_length, last, context_id, errp);
> +
> + g_free(extents);
> +
> + return ret;
> }
>
> /*
> @@ -2228,10 +2266,11 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
> (client->export_meta.base_allocation ||
> client->export_meta.bitmap))
> {
> + bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE;
> if (client->export_meta.base_allocation) {
> ret = nbd_co_send_block_status(client, request->handle,
> blk_bs(exp->blk), request->from,
> - request->len,
> + request->len, dont_fragment,
> !client->export_meta.bitmap,
> NBD_META_ID_BASE_ALLOCATION,
> errp);
> @@ -2244,7 +2283,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
> ret = nbd_co_send_bitmap(client, request->handle,
> client->exp->export_bitmap,
> request->from, request->len,
> - request->flags & NBD_CMD_FLAG_REQ_ONE,
> + dont_fragment,
> true, NBD_META_ID_DIRTY_BITMAP, errp);
> if (ret < 0) {
> return ret;
--
Best regards,
Vladimir
On 07/10/2018 09:33 AM, Vladimir Sementsov-Ogievskiy wrote:
> 04.07.2018 14:23, Vladimir Sementsov-Ogievskiy wrote:
>> This is necessary for efficient block-status export, for clients which
>> support it.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> +static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
>> + uint64_t *bytes, NBDExtent *extents,
>> + unsigned int *nb_extents)
>> {
>> - uint64_t remaining_bytes = bytes;
>> + uint64_t remaining_bytes = *bytes;
>> + NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
>> + bool first_extent = true;
>> + assert(*nb_extents);
>> while (remaining_bytes) {
>> uint32_t flags;
>> int64_t num;
>> @@ -1860,21 +1871,40 @@ static int
>> blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
>
> after bdrv_block_status_above, is there a guarantee that num > 0? Should
> we add an assertion?
bdrv_block_status_above() asserts that drivers set *pnum to non-zero on
success (that is, a driver's .bdrv_co_block_status must make progress or
fail); however, a caller can still get *pnum == 0 on the corner case of
requesting status at or beyond the end-of-file boundary (where
bdrv_block_status_above() does not call into a driver's
.bdrv_co_block_status callback). Since the NBD code already validated
that the client's code does not exceed EOF, and the block layer
guaranteed that an in-range request made progress on success, you should
be fine with an assertion that num > 0.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.