Re: [PATCH 4/6] block/gluster: Do not force-cap *pnum
On 19.06.21 12:36, 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
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/gluster.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index e8ee14c8e9..8ef7bb18d5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1461,7 +1461,8 @@ exit:
* the specified offset) that are known to be in the same
* allocated/unallocated state.
*
- * 'bytes' is the max value 'pnum' should be set to.
+ * 'bytes' is a soft cap for 'pnum'. If the information is free,
'pnum' may
+ * well exceed it.
*
* (Based on raw_co_block_status() from file-posix.c.)
*/
@@ -1500,12 +1501,12 @@ static int coroutine_fn
qemu_gluster_co_block_status(BlockDriverState *bs,
} else if (data == offset) {
/* On a data extent, compute bytes to the end of the extent,
* possibly including a partial sector at EOF. */
- *pnum = MIN(bytes, hole - offset);
+ *pnum = hole - offset;
ret = BDRV_BLOCK_DATA;
Interesting, isn't it a bug that we don't ROUND_UP *pnum to
request_alignment here like it is done in file-posix ?
Guess I forgot gluster in 9c3db310ff0 O:)
I don’t think I’ll be able to reproduce it for gluster, but I suppose
just doing the same thing for gluster should be fine...
Max
Re: [PATCH 4/6] block/gluster: Do not force-cap *pnum
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
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/gluster.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index e8ee14c8e9..8ef7bb18d5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1461,7 +1461,8 @@ exit:
* the specified offset) that are known to be in the same
* allocated/unallocated state.
*
- * 'bytes' is the max value 'pnum' should be set to.
+ * 'bytes' is a soft cap for 'pnum'. If the information is free, 'pnum' may
+ * well exceed it.
*
* (Based on raw_co_block_status() from file-posix.c.)
*/
@@ -1500,12 +1501,12 @@ static int coroutine_fn
qemu_gluster_co_block_status(BlockDriverState *bs,
} else if (data == offset) {
/* On a data extent, compute bytes to the end of the extent,
* possibly including a partial sector at EOF. */
-*pnum = MIN(bytes, hole - offset);
+*pnum = hole - offset;
ret = BDRV_BLOCK_DATA;
Interesting, isn't it a bug that we don't ROUND_UP *pnum to request_alignment
here like it is done in file-posix ?
} else {
/* On a hole, compute bytes to the beginning of the next extent. */
assert(hole == offset);
-*pnum = MIN(bytes, data - offset);
+*pnum = data - offset;
ret = BDRV_BLOCK_ZERO;
}
--
Best regards,
Vladimir
Re: [PATCH 4/6] block/gluster: Do not force-cap *pnum
On Thu, Jun 17, 2021 at 05:52:45PM +0200, 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 > --- > block/gluster.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
