Re: [PATCH 4/8] block: remove has_variable_length from BlockDriver

2023-04-07 Thread Eric Blake 
On Fri, Apr 07, 2023 at 05:32:59PM +0200, Paolo Bonzini wrote:
> Fill in the field in BlockLimits directly for host devices, and
> copy it from there for the raw format.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/file-posix.c   | 12 
>  block/file-win32.c   |  2 +-
>  block/io.c   |  2 --
>  block/raw-format.c   |  3 ++-
>  include/block/block_int-common.h |  2 --
>  5 files changed, 11 insertions(+), 10 deletions(-)

The change makes sense to me.  I'm having a slight doubt on whether it
might cause any regression in the bigger picture where a regular host
file exposed as a guest image grew outside of qemu's control but where
qemu used to see the new size automatically but now won't see it until
an explicit QMP action.  But I suspect that since we already do have a
QMP size for telling qemu to do resizes itself, or to at least refresh
its notion of size (for that very case of a third-party adding more
storage and telling qemu it is now safe to use that extra space), the
explicit QMP interaction is probably sufficient, and that any such
corner-case regression I'm worrying about is not a problem in reality.

Reviewed-by: Eric Blake 

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

[PATCH 4/8] block: remove has_variable_length from BlockDriver

2023-04-07 Thread Paolo Bonzini 
Fill in the field in BlockLimits directly for host devices, and
copy it from there for the raw format.

Signed-off-by: Paolo Bonzini 
---
 block/file-posix.c   | 12 
 block/file-win32.c   |  2 +-
 block/io.c   |  2 --
 block/raw-format.c   |  3 ++-
 include/block/block_int-common.h |  2 --
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 5b0f2e6d261b..c7b723368e53 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3732,6 +3732,12 @@ static void cdrom_parse_filename(const char *filename, 
QDict *options,
 {
 bdrv_parse_filename_strip_prefix(filename, "host_cdrom:", options);
 }
+
+static void cdrom_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+bs->bl.has_variable_length = true;
+raw_refresh_limits(bs, errp);
+}
 #endif
 
 #ifdef __linux__
@@ -3827,14 +3833,13 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_preadv = raw_co_preadv,
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
-.bdrv_refresh_limits = raw_refresh_limits,
+.bdrv_refresh_limits= cdrom_refresh_limits,
 .bdrv_co_io_plug= raw_co_io_plug,
 .bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
 .bdrv_co_getlength  = raw_co_getlength,
-.has_variable_length= true,
 .bdrv_co_get_allocated_file_size= raw_co_get_allocated_file_size,
 
 /* removable device support */
@@ -3956,14 +3961,13 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_preadv = raw_co_preadv,
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
-.bdrv_refresh_limits = raw_refresh_limits,
+.bdrv_refresh_limits= cdrom_refresh_limits,
 .bdrv_co_io_plug= raw_co_io_plug,
 .bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
 .bdrv_co_getlength  = raw_co_getlength,
-.has_variable_length= true,
 .bdrv_co_get_allocated_file_size= raw_co_get_allocated_file_size,
 
 /* removable device support */
diff --git a/block/file-win32.c b/block/file-win32.c
index 1a5f9e6a9b13..48b790d91739 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -836,6 +836,7 @@ static void hdev_refresh_limits(BlockDriverState *bs, Error 
**errp)
 {
 /* XXX Does Windows support AIO on less than 512-byte alignment? */
 bs->bl.request_alignment = 512;
+bs->bl.has_variable_length = true;
 }
 
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
@@ -931,7 +932,6 @@ static BlockDriver bdrv_host_device = {
 .bdrv_attach_aio_context = raw_attach_aio_context,
 
 .bdrv_co_getlength= raw_co_getlength,
-.has_variable_length  = true,
 .bdrv_co_get_allocated_file_size  = raw_co_get_allocated_file_size,
 };
 
diff --git a/block/io.c b/block/io.c
index c49917c74677..6fa199337472 100644
--- a/block/io.c
+++ b/block/io.c
@@ -182,8 +182,6 @@ void bdrv_refresh_limits(BlockDriverState *bs, Transaction 
*tran, Error **errp)
 drv->bdrv_aio_preadv ||
 drv->bdrv_co_preadv_part) ? 1 : 512;
 
-bs->bl.has_variable_length = drv->has_variable_length;
-
 /* Take some limits from the children as a default */
 have_limits = false;
 QLIST_FOREACH(c, >children, next) {
diff --git a/block/raw-format.c b/block/raw-format.c
index 66783ed8e77b..06b8030d9d40 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -377,6 +377,8 @@ raw_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
+bs->bl.has_variable_length = bs->file->bs->bl.has_variable_length;
+
 if (bs->probed) {
 /* To make it easier to protect the first sector, any probed
  * image is restricted to read-modify-write on sub-sector
@@ -623,7 +625,6 @@ BlockDriver bdrv_raw = {
 .bdrv_co_truncate = _co_truncate,
 .bdrv_co_getlength= _co_getlength,
 .is_format= true,
-.has_variable_length  = true,
 .bdrv_measure = _measure,
 .bdrv_co_get_info = _co_get_info,
 .bdrv_refresh_limits  = _refresh_limits,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 95c934589571..013d419444d9 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -158,8 +158,6 @@ struct BlockDriver {
  */
 bool supports_backing;
 
-bool has_variable_length;
-
 /*
  * Drivers setting this field must be able to work with just a plain
  * filename with ':' as a prefix,