On 19.06.21 12:53, Vladimir Sementsov-Ogievskiy wrote:
17.06.2021 18:52, Max Reitz wrote:
bdrv_co_block_status() does it for us, we do not need to do it here.
The advantage of not capping *pnum is that bdrv_co_block_status() can
cache larger data regions than requested by its caller.
Signed-off-by: Max Reitz <mre...@redhat.com>
---
block/nbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/nbd.c b/block/nbd.c
index 616f9ae6c4..930bd234de 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1702,7 +1702,7 @@ static int coroutine_fn
nbd_client_co_block_status(
.type = NBD_CMD_BLOCK_STATUS,
.from = offset,
.len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
- MIN(bytes, s->info.size - offset)),
+ s->info.size - offset),
.flags = NBD_CMD_FLAG_REQ_ONE,
};
Hmm..
I don't that this change is correct. In contrast with file-posix you
don't get extra information for free, you just make a larger request.
This means that server will have to do more work.
Oh, oops. Seems I was blind in my rage to replace this MIN() pattern.
You’re absolutely right. So this patch should be dropped.
Max
(look at blockstatus_to_extents, it calls bdrv_block_status_above in a
loop).
For example, assume that nbd export is a qcow2 image with all clusters
allocated. With this change, nbd server will loop through the whole
qcow2 image, load all L2 tables to return big allocated extent.
So, only server can decide, could it add some extra free information
to request or not. But unfortunately NBD_CMD_FLAG_REQ_ONE doesn't
allow it.