[PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

qom-set of the 'drive' property is allowed on a realized device and ends
up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
without telling the parent anything about it. Every other way the size of
a node changes goes through bdrv_co_truncate(), which does notify, so a
device model caching the size of its child has no way to learn that it
changed.

scsi-disk is the one really hurt by this. check_lba_range() validates
every guest request against SCSIDevice.max_lba, filled in by
scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
handlers. Point a scsi-hd at a larger node and every request past the end
of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
OUT OF RANGE for the whole life of the device, which Linux turns into
EREMOTEIO for reads as well as writes. It is silent on the host:
out-of-range requests are answered by scsi_check_condition() and never
reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
every request, so they end up with a stale idea of the size but never
refuse I/O the node underneath can serve.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Fam Zheng <fam@euphon.net>

Denis V. Lunev (4):
  block: notify the parent when a replaced child has a different size
  scsi-disk: refresh the cached capacity from the resize callback
  tests/qtest/virtio-scsi: cover a replacement that changes the size
  tests/qemu-iotests/qom-set-drive: replace with a differently sized
    node

 block.c                                    | 13 +++++
 hw/scsi/scsi-disk.c                        | 16 ++++--
 include/block/block_int-io.h               |  3 ++
 tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
 tests/qemu-iotests/tests/qom-set-drive.out |  2 +
 tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
 6 files changed, 104 insertions(+), 7 deletions(-)


base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
-- 
2.53.0

[PATCH 1/4] block: notify the parent when a replaced child has a different size

[Denis V. Lunev]

bdrv_replace_child_bs() swaps the node a BdrvChild points to without
telling the parent anything about it, so a parent which tracks the size of
its child never learns that it changed. Every other way the size of a node
changes goes through bdrv_co_truncate(), which does notify.

The device models which care all register the same BlockDevOps hook:

  virtio-blk  resize_cb = virtio_blk_resize(), ending up in
              virtio_notify_config()
  ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
              and the IDENTIFY data
  scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
              CAPACITY DATA HAS CHANGED
  xen-block   resize_cb = xen_block_resize_cb()

Only scsi-disk is really hurt by the missing notification, because it is
the one which validates guest requests against a cached size.
virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
blk_get_geometry() for every request, so a missing notification leaves
those guests with a stale idea of the size but never refuses I/O which the
node underneath can serve. check_lba_range() compares against
SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
re-read the capacity every request past the end of the old node is refused
for the whole life of the device.

This is reachable with qom-set of the 'drive' property, which is allowed
on a realized device, and it is silent on the host: out-of-range requests
are answered by scsi_check_condition() and never reach
scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
RANGE, which Linux turns into EREMOTEIO for reads as well as writes.

Emit the notification from bdrv_replace_child_bs(), so that every device
model gets it through the path it already implements rather than any of
them being special cased. Send it only when the size really changed, which
keeps the callback out of the common case of a replacement by an equally
sized node.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 block.c                      | 13 +++++++++++++
 include/block/block_int-io.h |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/block.c b/block.c
index f0a6042e61..1fe66caf8d 100644
--- a/block.c
+++ b/block.c
@@ -5524,9 +5524,12 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
     Transaction *tran = tran_new();
     g_autoptr(GSList) refresh_list = NULL;
     BlockDriverState *old_bs = child->bs;
+    int64_t old_len;
 
     GLOBAL_STATE_CODE();
 
+    old_len = bdrv_getlength(old_bs);
+
     bdrv_ref(old_bs);
     bdrv_drained_begin(old_bs);
     bdrv_drained_begin(new_bs);
@@ -5542,6 +5545,16 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
     tran_finalize(tran, ret);
 
     bdrv_graph_wrunlock();
+
+    /* A parent caching the child size has to learn that it changed */
+    if (ret == 0) {
+        int64_t new_len = bdrv_getlength(new_bs);
+
+        if (new_len >= 0 && new_len != old_len) {
+            bdrv_parent_cb_resize(new_bs);
+        }
+    }
+
     bdrv_drained_end(old_bs);
     bdrv_drained_end(new_bs);
     bdrv_unref(old_bs);
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index ed8b5657d6..95d0dd7a05 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -197,4 +197,7 @@ void bdrv_bsc_fill(BlockDriverState *bs, int64_t offset, int64_t bytes);
 void coroutine_fn GRAPH_RDLOCK
 bdrv_co_parent_cb_resize(BlockDriverState *bs);
 
+void co_wrapper_bdrv_rdlock
+bdrv_parent_cb_resize(BlockDriverState *bs);
+
 #endif /* BLOCK_INT_IO_H */
-- 
2.53.0

[PATCH 2/4] scsi-disk: refresh the cached capacity from the resize callback

[Denis V. Lunev]

scsi_disk_resize_cb() reports CAPACITY DATA HAS CHANGED but leaves
SCSIDevice.max_lba alone, and max_lba is what check_lba_range() compares
every request against. Until the guest reacts to the unit attention and
issues READ CAPACITY, requests beyond the old size keep being refused with
LOGICAL BLOCK ADDRESS OUT OF RANGE, and a guest which never re-reads the
capacity never recovers at all. A guest migrated onto a device whose size
changed underneath it is exactly such a guest.

ide_resize_cb() refreshes its cached size along with the IDENTIFY data it
reports to the guest. Do the same here, so that the notification is what
tells the guest about the new capacity rather than what unblocks the
device.

Factor the computation out of scsi_disk_reset(), which already had it.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 hw/scsi/scsi-disk.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 1b0cce128c..3d404a8651 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2386,13 +2386,10 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
     }
 }
 
-static void scsi_disk_reset(DeviceState *dev)
+static void scsi_disk_refresh_max_lba(SCSIDiskState *s)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
     uint64_t nb_sectors;
 
-    scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
-
     blk_get_geometry(s->qdev.conf.blk, &nb_sectors);
 
     nb_sectors /= s->qdev.blocksize / BDRV_SECTOR_SIZE;
@@ -2400,6 +2397,15 @@ static void scsi_disk_reset(DeviceState *dev)
         nb_sectors--;
     }
     s->qdev.max_lba = nb_sectors;
+}
+
+static void scsi_disk_reset(DeviceState *dev)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
+
+    scsi_device_purge_requests(&s->qdev, SENSE_CODE(RESET));
+
+    scsi_disk_refresh_max_lba(s);
     /* reset tray statuses */
     s->tray_locked = 0;
     s->tray_open = 0;
@@ -2425,6 +2431,8 @@ static void scsi_disk_resize_cb(void *opaque)
 {
     SCSIDiskState *s = opaque;
 
+    scsi_disk_refresh_max_lba(s);
+
     /* SPC lists this sense code as available only for
      * direct-access devices.
      */
-- 
2.53.0

[PATCH 3/4] tests/qtest/virtio-scsi: cover a replacement that changes the size

[Denis V. Lunev]

Realize a scsi-hd on a 1 MiB node, replace it with a 128 MiB one using
qom-set, and check that the device reports CAPACITY DATA HAS CHANGED and
then serves a read which only fits in the new node.

Both halves regress on their own. Without the notification from
bdrv_replace_child_bs() the first command after the replacement fails with
ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF RANGE instead of the unit
attention. Without the refresh in scsi_disk_resize_cb() the read after it
is refused the same way, since nothing here issues READ CAPACITY.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 tests/qtest/virtio-scsi-test.c | 63 ++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index e2350c52f6..f82b7e5de9 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -236,6 +236,53 @@ static void test_unmap_large_lba(void *obj, void *data,
     qvirtio_scsi_pci_free(vs);
 }
 
+/* Test replacing the backing node with one of a different size */
+static void test_replaced_node_size(void *obj, void *data,
+                                    QGuestAllocator *t_alloc)
+{
+    QVirtioSCSI *scsi = obj;
+    QVirtioSCSIQueues *vs;
+    uint8_t buf[512] = { 0 };
+    /* READ(10), LBA 0, transfer length 1 */
+    const uint8_t read_low_cdb[VIRTIO_SCSI_CDB_SIZE] = {
+        0x28, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00
+    };
+    /* READ(10), LBA 0x1000 (2 MiB), transfer length 1 */
+    const uint8_t read_high_cdb[VIRTIO_SCSI_CDB_SIZE] = {
+        0x28, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x01, 0x00
+    };
+    struct virtio_scsi_cmd_resp resp;
+
+    alloc = t_alloc;
+    vs = qvirtio_scsi_init(scsi->vdev);
+
+    /* LBA 0 is inside the 1 MiB node the device was realized with */
+    virtio_scsi_do_command(vs, read_low_cdb, buf, sizeof(buf), NULL, 0, &resp);
+    g_assert_cmphex(resp.response, ==, 0);
+    g_assert_cmphex(resp.status, ==, GOOD);
+
+    /* Swap in a 128 MiB node; the device is not reset by this */
+    qtest_qmp_assert_success(global_qtest,
+                             "{'execute': 'qom-set', 'arguments': {"
+                             "  'path': '/machine/peripheral/scsi-hd0',"
+                             "  'property': 'drive', 'value': 'big'}}");
+
+    /* The guest has to be told that the capacity it knows is stale */
+    virtio_scsi_do_command(vs, read_high_cdb, buf, sizeof(buf), NULL, 0, &resp);
+    g_assert_cmphex(resp.response, ==, 0);
+    g_assert_cmphex(resp.status, ==, CHECK_CONDITION);
+    g_assert_cmphex(resp.sense[2], ==, UNIT_ATTENTION);
+    g_assert_cmphex(resp.sense[12], ==, 0x2a);
+    g_assert_cmphex(resp.sense[13], ==, 0x09); /* CAPACITY DATA HAS CHANGED */
+
+    /* LBA 0x1000 is past the old node but well inside the new one */
+    virtio_scsi_do_command(vs, read_high_cdb, buf, sizeof(buf), NULL, 0, &resp);
+    g_assert_cmphex(resp.response, ==, 0);
+    g_assert_cmphex(resp.status, ==, GOOD);
+
+    qvirtio_scsi_pci_free(vs);
+}
+
 static void test_write_to_cdrom(void *obj, void *data,
                                 QGuestAllocator *t_alloc)
 {
@@ -365,6 +412,18 @@ static void *virtio_scsi_setup_4k(GString *cmd_line, void *arg)
     return arg;
 }
 
+static void *virtio_scsi_setup_replace(GString *cmd_line, void *arg)
+{
+    g_string_append(cmd_line,
+                    " -blockdev driver=null-co,read-zeroes=on,"
+                    "size=1048576,node-name=small"
+                    " -blockdev driver=null-co,read-zeroes=on,"
+                    "size=134217728,node-name=big"
+                    " -device scsi-hd,id=scsi-hd0,drive=small,"
+                    "lun=0,scsi-id=1");
+    return arg;
+}
+
 static void *virtio_scsi_setup_cd(GString *cmd_line, void *arg)
 {
     g_string_append(cmd_line,
@@ -399,6 +458,10 @@ static void register_virtio_scsi_test(void)
     qos_add_test("large-lba-unmap", "virtio-scsi",
                  test_unmap_large_lba, &opts);
 
+    opts.before = virtio_scsi_setup_replace;
+    qos_add_test("replaced-node-size", "virtio-scsi",
+                 test_replaced_node_size, &opts);
+
     opts.before = virtio_scsi_setup_cd;
     qos_add_test("write-to-cdrom", "virtio-scsi", test_write_to_cdrom, &opts);
 
-- 
2.53.0

[PATCH 4/4] tests/qemu-iotests/qom-set-drive: replace with a differently sized node

[Denis V. Lunev]

All images in this test have the same size, so the test never reaches
the size change path of bdrv_replace_child_bs(). Add a node of a
different size and point a device at it.

The notification itself is not visible over QMP, so this only covers the
block layer side. What the device does with it is checked by
tests/qtest/virtio-scsi-test.c.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 tests/qemu-iotests/tests/qom-set-drive     | 14 +++++++++++---
 tests/qemu-iotests/tests/qom-set-drive.out |  2 ++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/tests/qom-set-drive b/tests/qemu-iotests/tests/qom-set-drive
index ec8ddac4fd..52d7eb96da 100755
--- a/tests/qemu-iotests/tests/qom-set-drive
+++ b/tests/qemu-iotests/tests/qom-set-drive
@@ -24,12 +24,15 @@ import iotests
 from iotests import imgfmt, log, qemu_img_create, QMPTestCase
 
 image_size = 1 * 1024 * 1024
-images = [os.path.join(iotests.test_dir, f'{i}.img') for i in range(0, 4)]
+images = [os.path.join(iotests.test_dir, f'{i}.img') for i in range(0, 5)]
+
+# node4 is deliberately larger, to cover a replacement which changes the size
+image_sizes = [image_size] * 4 + [2 * image_size]
 
 class TestQOMSetDrive(QMPTestCase):
     def setUp(self) -> None:
-        for image in images:
-            qemu_img_create('-f', imgfmt, image, str(image_size))
+        for image, size in zip(images, image_sizes):
+            qemu_img_create('-f', imgfmt, image, str(size))
 
         self.vm = iotests.VM()
         for i, image in enumerate(images):
@@ -68,6 +71,11 @@ class TestQOMSetDrive(QMPTestCase):
         log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot',
                         property='drive'))
 
+        log(self.vm.qmp('qom-set', path='/machine/peripheral/no-iot',
+                        property='drive', value='node4'))
+        log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot',
+                        property='drive'))
+
 if __name__ == '__main__':
     iotests.activate_logging()
     # LUKS would require special key-secret handling in add_blockdevs()
diff --git a/tests/qemu-iotests/tests/qom-set-drive.out b/tests/qemu-iotests/tests/qom-set-drive.out
index 7fc243dca6..6f6e5e2b31 100644
--- a/tests/qemu-iotests/tests/qom-set-drive.out
+++ b/tests/qemu-iotests/tests/qom-set-drive.out
@@ -4,6 +4,8 @@
 {"return": "node1"}
 {"return": {}}
 {"return": "node3"}
+{"return": {}}
+{"return": "node4"}
 .
 ----------------------------------------------------------------------
 Ran 1 tests
-- 
2.53.0

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping v2

Re: [PATCH 2/4] scsi-disk: refresh the cached capacity from the resize callback

[Philippe Mathieu-Daudé]

On 12/8/26 14:14, Denis V. Lunev wrote:
> scsi_disk_resize_cb() reports CAPACITY DATA HAS CHANGED but leaves
> SCSIDevice.max_lba alone, and max_lba is what check_lba_range() compares
> every request against. Until the guest reacts to the unit attention and
> issues READ CAPACITY, requests beyond the old size keep being refused with
> LOGICAL BLOCK ADDRESS OUT OF RANGE, and a guest which never re-reads the
> capacity never recovers at all. A guest migrated onto a device whose size
> changed underneath it is exactly such a guest.
> 
> ide_resize_cb() refreshes its cached size along with the IDENTIFY data it
> reports to the guest. Do the same here, so that the notification is what
> tells the guest about the new capacity rather than what unblocks the
> device.
> 
> Factor the computation out of scsi_disk_reset(), which already had it.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   hw/scsi/scsi-disk.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 3/4] tests/qtest/virtio-scsi: cover a replacement that changes the size

[Philippe Mathieu-Daudé]

On 12/8/26 14:14, Denis V. Lunev wrote:
> Realize a scsi-hd on a 1 MiB node, replace it with a 128 MiB one using
> qom-set, and check that the device reports CAPACITY DATA HAS CHANGED and
> then serves a read which only fits in the new node.
> 
> Both halves regress on their own. Without the notification from
> bdrv_replace_child_bs() the first command after the replacement fails with
> ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF RANGE instead of the unit
> attention. Without the refresh in scsi_disk_resize_cb() the read after it
> is refused the same way, since nothing here issues READ CAPACITY.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   tests/qtest/virtio-scsi-test.c | 63 ++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 4/4] tests/qemu-iotests/qom-set-drive: replace with a differently sized node

[Philippe Mathieu-Daudé]

On 12/8/26 14:15, Denis V. Lunev wrote:
> All images in this test have the same size, so the test never reaches
> the size change path of bdrv_replace_child_bs(). Add a node of a
> different size and point a device at it.
> 
> The notification itself is not visible over QMP, so this only covers the
> block layer side. What the device does with it is checked by
> tests/qtest/virtio-scsi-test.c.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   tests/qemu-iotests/tests/qom-set-drive     | 14 +++++++++++---
>   tests/qemu-iotests/tests/qom-set-drive.out |  2 ++
>   2 files changed, 13 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 1/4] block: notify the parent when a replaced child has a different size

[Philippe Mathieu-Daudé]

On 12/8/26 14:14, Denis V. Lunev wrote:
> bdrv_replace_child_bs() swaps the node a BdrvChild points to without
> telling the parent anything about it, so a parent which tracks the size of
> its child never learns that it changed. Every other way the size of a node
> changes goes through bdrv_co_truncate(), which does notify.
> 
> The device models which care all register the same BlockDevOps hook:
> 
>    virtio-blk  resize_cb = virtio_blk_resize(), ending up in
>                virtio_notify_config()
>    ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
>                and the IDENTIFY data
>    scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
>                CAPACITY DATA HAS CHANGED
>    xen-block   resize_cb = xen_block_resize_cb()
> 
> Only scsi-disk is really hurt by the missing notification, because it is
> the one which validates guest requests against a cached size.
> virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
> blk_get_geometry() for every request, so a missing notification leaves
> those guests with a stale idea of the size but never refuses I/O which the
> node underneath can serve. check_lba_range() compares against
> SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
> READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
> re-read the capacity every request past the end of the old node is refused
> for the whole life of the device.
> 
> This is reachable with qom-set of the 'drive' property, which is allowed
> on a realized device, and it is silent on the host: out-of-range requests
> are answered by scsi_check_condition() and never reach
> scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
> stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
> RANGE, which Linux turns into EREMOTEIO for reads as well as writes.
> 
> Emit the notification from bdrv_replace_child_bs(), so that every device
> model gets it through the path it already implements rather than any of
> them being special cased. Send it only when the size really changed, which
> keeps the callback out of the common case of a replacement by an equally
> sized node.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   block.c                      | 13 +++++++++++++
>   include/block/block_int-io.h |  3 +++
>   2 files changed, 16 insertions(+)

To the best of my block/ understanding:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
Kevin, Hanna,

have you had a chance to take a look?
I think block tree is correct one to go through.

Thank you in advance,
    Den

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 9/2/26 17:39, Denis V. Lunev wrote:
> On 8/12/26 14:14, Denis V. Lunev wrote:
>> This email originated from an IP that might not be authorized by the domain it was sent from.
>> Do not click links or open attachments unless it is an email you expected to receive.
>> qom-set of the 'drive' property is allowed on a realized device and ends
>> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
>> without telling the parent anything about it. Every other way the size of
>> a node changes goes through bdrv_co_truncate(), which does notify, so a
>> device model caching the size of its child has no way to learn that it
>> changed.
>>
>> scsi-disk is the one really hurt by this. check_lba_range() validates
>> every guest request against SCSIDevice.max_lba, filled in by
>> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
>> handlers. Point a scsi-hd at a larger node and every request past the end
>> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
>> OUT OF RANGE for the whole life of the device, which Linux turns into
>> EREMOTEIO for reads as well as writes. It is silent on the host:
>> out-of-range requests are answered by scsi_check_condition() and never
>> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
>> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
>> every request, so they end up with a stale idea of the size but never
>> refuse I/O the node underneath can serve.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Hanna Reitz <hreitz@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Fam Zheng <fam@euphon.net>
>>
>> Denis V. Lunev (4):
>>   block: notify the parent when a replaced child has a different size
>>   scsi-disk: refresh the cached capacity from the resize callback
>>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>>     node
>>
>>  block.c                                    | 13 +++++
>>  hw/scsi/scsi-disk.c                        | 16 ++++--
>>  include/block/block_int-io.h               |  3 ++
>>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>>  6 files changed, 104 insertions(+), 7 deletions(-)
>>
>>
>> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
> Kevin, Hanna,
>
> have you had a chance to take a look?
> I think block tree is correct one to go through.
>
> Thank you in advance,
>     Den
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 8/12/26 14:14, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> qom-set of the 'drive' property is allowed on a realized device and ends
> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> without telling the parent anything about it. Every other way the size of
> a node changes goes through bdrv_co_truncate(), which does notify, so a
> device model caching the size of its child has no way to learn that it
> changed.
>
> scsi-disk is the one really hurt by this. check_lba_range() validates
> every guest request against SCSIDevice.max_lba, filled in by
> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> handlers. Point a scsi-hd at a larger node and every request past the end
> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> OUT OF RANGE for the whole life of the device, which Linux turns into
> EREMOTEIO for reads as well as writes. It is silent on the host:
> out-of-range requests are answered by scsi_check_condition() and never
> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> every request, so they end up with a stale idea of the size but never
> refuse I/O the node underneath can serve.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
>
> Denis V. Lunev (4):
>   block: notify the parent when a replaced child has a different size
>   scsi-disk: refresh the cached capacity from the resize callback
>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>     node
>
>  block.c                                    | 13 +++++
>  hw/scsi/scsi-disk.c                        | 16 ++++--
>  include/block/block_int-io.h               |  3 ++
>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 7 deletions(-)
>
>
> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
ping

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Eric Blake]

On Fri, Oct 30, 2026 at 12:08:24PM +0100, Denis V. Lunev wrote:
> On 8/12/26 14:14, Denis V. Lunev wrote:
> > qom-set of the 'drive' property is allowed on a realized device and ends
> > up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
> > without telling the parent anything about it. Every other way the size of
> > a node changes goes through bdrv_co_truncate(), which does notify, so a
> > device model caching the size of its child has no way to learn that it
> > changed.
> >
> > scsi-disk is the one really hurt by this. check_lba_range() validates
> > every guest request against SCSIDevice.max_lba, filled in by
> > scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
> > handlers. Point a scsi-hd at a larger node and every request past the end
> > of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
> > OUT OF RANGE for the whole life of the device, which Linux turns into
> > EREMOTEIO for reads as well as writes. It is silent on the host:
> > out-of-range requests are answered by scsi_check_condition() and never
> > reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
> > io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
> > every request, so they end up with a stale idea of the size but never
> > refuse I/O the node underneath can serve.
> >
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Hanna Reitz <hreitz@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Fam Zheng <fam@euphon.net>
> >
> > Denis V. Lunev (4):
> >   block: notify the parent when a replaced child has a different size
> >   scsi-disk: refresh the cached capacity from the resize callback
> >   tests/qtest/virtio-scsi: cover a replacement that changes the size
> >   tests/qemu-iotests/qom-set-drive: replace with a differently sized
> >     node
> >
> >  block.c                                    | 13 +++++
> >  hw/scsi/scsi-disk.c                        | 16 ++++--
> >  include/block/block_int-io.h               |  3 ++
> >  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
> >  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
> >  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
> >  6 files changed, 104 insertions(+), 7 deletions(-)
> >
> >
> > base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
> ping

Looks like this one has sat for a while; while it is not directly
related to my usual NBD pulls, I'm in the middle of preparing a block
layer pull request today so I'll add this one in my queue if it passes
my testing.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 0/4] block, scsi-disk: honour a size change on child replacement

[Denis V. Lunev]

On 11/2/26 17:30, Eric Blake wrote:
> On Fri, Oct 30, 2026 at 12:08:24PM +0100, Denis V. Lunev wrote:
>> On 8/12/26 14:14, Denis V. Lunev wrote:
>>> qom-set of the 'drive' property is allowed on a realized device and ends
>>> up in bdrv_replace_child_bs(), which swaps the node a BdrvChild points to
>>> without telling the parent anything about it. Every other way the size of
>>> a node changes goes through bdrv_co_truncate(), which does notify, so a
>>> device model caching the size of its child has no way to learn that it
>>> changed.
>>>
>>> scsi-disk is the one really hurt by this. check_lba_range() validates
>>> every guest request against SCSIDevice.max_lba, filled in by
>>> scsi_disk_reset() and updated only by the READ CAPACITY(10) and (16)
>>> handlers. Point a scsi-hd at a larger node and every request past the end
>>> of the old one is refused with ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS
>>> OUT OF RANGE for the whole life of the device, which Linux turns into
>>> EREMOTEIO for reads as well as writes. It is silent on the host:
>>> out-of-range requests are answered by scsi_check_condition() and never
>>> reach scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and
>>> io-status stays 'ok'. virtio-blk and ide call blk_get_geometry() for
>>> every request, so they end up with a stale idea of the size but never
>>> refuse I/O the node underneath can serve.
>>>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>> Cc: Hanna Reitz <hreitz@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Fam Zheng <fam@euphon.net>
>>>
>>> Denis V. Lunev (4):
>>>   block: notify the parent when a replaced child has a different size
>>>   scsi-disk: refresh the cached capacity from the resize callback
>>>   tests/qtest/virtio-scsi: cover a replacement that changes the size
>>>   tests/qemu-iotests/qom-set-drive: replace with a differently sized
>>>     node
>>>
>>>  block.c                                    | 13 +++++
>>>  hw/scsi/scsi-disk.c                        | 16 ++++--
>>>  include/block/block_int-io.h               |  3 ++
>>>  tests/qemu-iotests/tests/qom-set-drive     | 14 +++--
>>>  tests/qemu-iotests/tests/qom-set-drive.out |  2 +
>>>  tests/qtest/virtio-scsi-test.c             | 63 ++++++++++++++++++++++
>>>  6 files changed, 104 insertions(+), 7 deletions(-)
>>>
>>>
>>> base-commit: 3e3ccab106f879b1512f8e0d51a827dd4de30e22
>> ping
> Looks like this one has sat for a while; while it is not directly
> related to my usual NBD pulls, I'm in the middle of preparing a block
> layer pull request today so I'll add this one in my queue if it passes
> my testing.
>
That would be very kind of you.

This one is circulating here for months.

Den

Re: [PATCH 1/4] block: notify the parent when a replaced child has a different size

[Kevin Wolf]

Am 12.08.2026 um 14:14 hat Denis V. Lunev geschrieben:
> bdrv_replace_child_bs() swaps the node a BdrvChild points to without
> telling the parent anything about it, so a parent which tracks the size of
> its child never learns that it changed. Every other way the size of a node
> changes goes through bdrv_co_truncate(), which does notify.
> 
> The device models which care all register the same BlockDevOps hook:
> 
>   virtio-blk  resize_cb = virtio_blk_resize(), ending up in
>               virtio_notify_config()
>   ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
>               and the IDENTIFY data
>   scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
>               CAPACITY DATA HAS CHANGED
>   xen-block   resize_cb = xen_block_resize_cb()
> 
> Only scsi-disk is really hurt by the missing notification, because it is
> the one which validates guest requests against a cached size.
> virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
> blk_get_geometry() for every request, so a missing notification leaves
> those guests with a stale idea of the size but never refuses I/O which the
> node underneath can serve. check_lba_range() compares against
> SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
> READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
> re-read the capacity every request past the end of the old node is refused
> for the whole life of the device.
> 
> This is reachable with qom-set of the 'drive' property, which is allowed
> on a realized device, and it is silent on the host: out-of-range requests
> are answered by scsi_check_condition() and never reach
> scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
> stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
> RANGE, which Linux turns into EREMOTEIO for reads as well as writes.
> 
> Emit the notification from bdrv_replace_child_bs(), so that every device
> model gets it through the path it already implements rather than any of
> them being special cased. Send it only when the size really changed, which
> keeps the callback out of the common case of a replacement by an equally
> sized node.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  block.c                      | 13 +++++++++++++
>  include/block/block_int-io.h |  3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/block.c b/block.c
> index f0a6042e61..1fe66caf8d 100644
> --- a/block.c
> +++ b/block.c
> @@ -5524,9 +5524,12 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>      Transaction *tran = tran_new();
>      g_autoptr(GSList) refresh_list = NULL;
>      BlockDriverState *old_bs = child->bs;
> +    int64_t old_len;
>  
>      GLOBAL_STATE_CODE();
>  
> +    old_len = bdrv_getlength(old_bs);
> +
>      bdrv_ref(old_bs);
>      bdrv_drained_begin(old_bs);
>      bdrv_drained_begin(new_bs);

bdrv_drained_begin() involves nested event loops. Can't old_len become
stale here? It would feel safer to query it after draining.

> @@ -5542,6 +5545,16 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>      tran_finalize(tran, ret);
>  
>      bdrv_graph_wrunlock();
> +
> +    /* A parent caching the child size has to learn that it changed */
> +    if (ret == 0) {
> +        int64_t new_len = bdrv_getlength(new_bs);
> +
> +        if (new_len >= 0 && new_len != old_len) {
> +            bdrv_parent_cb_resize(new_bs);

This notifies other parents of the new node as well even though nothing
changed for them.

Especially since you already query old_len/new_len to avoid unnecessary
notifications, we should probably call only child->klass->resize(child)
through a coroutine wrapper here to notify only the BdrvChild that we
actually changed.

> +        }
> +    }
> +
>      bdrv_drained_end(old_bs);
>      bdrv_drained_end(new_bs);
>      bdrv_unref(old_bs);

Kevin

Re: [PATCH 1/4] block: notify the parent when a replaced child has a different size

[Denis V. Lunev]

On 11/3/26 11:02, Kevin Wolf wrote:
> Am 12.08.2026 um 14:14 hat Denis V. Lunev geschrieben:
>> bdrv_replace_child_bs() swaps the node a BdrvChild points to without
>> telling the parent anything about it, so a parent which tracks the size of
>> its child never learns that it changed. Every other way the size of a node
>> changes goes through bdrv_co_truncate(), which does notify.
>>
>> The device models which care all register the same BlockDevOps hook:
>>
>>   virtio-blk  resize_cb = virtio_blk_resize(), ending up in
>>               virtio_notify_config()
>>   ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
>>               and the IDENTIFY data
>>   scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
>>               CAPACITY DATA HAS CHANGED
>>   xen-block   resize_cb = xen_block_resize_cb()
>>
>> Only scsi-disk is really hurt by the missing notification, because it is
>> the one which validates guest requests against a cached size.
>> virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
>> blk_get_geometry() for every request, so a missing notification leaves
>> those guests with a stale idea of the size but never refuses I/O which the
>> node underneath can serve. check_lba_range() compares against
>> SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
>> READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
>> re-read the capacity every request past the end of the old node is refused
>> for the whole life of the device.
>>
>> This is reachable with qom-set of the 'drive' property, which is allowed
>> on a realized device, and it is silent on the host: out-of-range requests
>> are answered by scsi_check_condition() and never reach
>> scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
>> stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
>> RANGE, which Linux turns into EREMOTEIO for reads as well as writes.
>>
>> Emit the notification from bdrv_replace_child_bs(), so that every device
>> model gets it through the path it already implements rather than any of
>> them being special cased. Send it only when the size really changed, which
>> keeps the callback out of the common case of a replacement by an equally
>> sized node.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> ---
>>  block.c                      | 13 +++++++++++++
>>  include/block/block_int-io.h |  3 +++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index f0a6042e61..1fe66caf8d 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -5524,9 +5524,12 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>>      Transaction *tran = tran_new();
>>      g_autoptr(GSList) refresh_list = NULL;
>>      BlockDriverState *old_bs = child->bs;
>> +    int64_t old_len;
>>  
>>      GLOBAL_STATE_CODE();
>>  
>> +    old_len = bdrv_getlength(old_bs);
>> +
>>      bdrv_ref(old_bs);
>>      bdrv_drained_begin(old_bs);
>>      bdrv_drained_begin(new_bs);
> bdrv_drained_begin() involves nested event loops. Can't old_len become
> stale here? It would feel safer to query it after draining.
I do not think it can. The only caller is blk_replace_bs(), so @child is
the root of a device and old_len is the virtual size the device checks
every request against. blk_check_byte_request() refuses writes past it,
and block_resize is a coroutine QMP command, so it cannot run while the
qom-set one does. Any size change that does happen goes through
bdrv_co_write_req_finish(), which notifies the parents of old_bs, @child
included, on its own.
  
If the size could really move in the middle of a replacement, comparing
it at this point would not make sense in the first place, no matter
whether it is read before or after the drain.

>> @@ -5542,6 +5545,16 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>>      tran_finalize(tran, ret);
>>  
>>      bdrv_graph_wrunlock();
>> +
>> +    /* A parent caching the child size has to learn that it changed */
>> +    if (ret == 0) {
>> +        int64_t new_len = bdrv_getlength(new_bs);
>> +
>> +        if (new_len >= 0 && new_len != old_len) {
>> +            bdrv_parent_cb_resize(new_bs);
> This notifies other parents of the new node as well even though nothing
> changed for them.
>
> Especially since you already query old_len/new_len to avoid unnecessary
> notifications, we should probably call only child->klass->resize(child)
> through a coroutine wrapper here to notify only the BdrvChild that we
> actually changed.
True, but the extra call costs nothing. The resize callback re-reads the
size from the node: virtio-blk sends a config interrupt, scsi-disk
raises CAPACITY DATA HAS CHANGED, and the guest finds the capacity it
already had. A second parent on the new node also needs share-rw on both
devices, so this is rare. A dedicated coroutine wrapper for one call
site does not look worth it to me. If you still prefer it, I will send
the patch. 

Thank you for a review,
    Den

Re: [PATCH 1/4] block: notify the parent when a replaced child has a different size

[Kevin Wolf]

Am 04.11.2026 um 15:50 hat Denis V. Lunev geschrieben:
> On 11/3/26 11:02, Kevin Wolf wrote:
> > Am 12.08.2026 um 14:14 hat Denis V. Lunev geschrieben:
> >> bdrv_replace_child_bs() swaps the node a BdrvChild points to without
> >> telling the parent anything about it, so a parent which tracks the size of
> >> its child never learns that it changed. Every other way the size of a node
> >> changes goes through bdrv_co_truncate(), which does notify.
> >>
> >> The device models which care all register the same BlockDevOps hook:
> >>
> >>   virtio-blk  resize_cb = virtio_blk_resize(), ending up in
> >>               virtio_notify_config()
> >>   ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
> >>               and the IDENTIFY data
> >>   scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
> >>               CAPACITY DATA HAS CHANGED
> >>   xen-block   resize_cb = xen_block_resize_cb()
> >>
> >> Only scsi-disk is really hurt by the missing notification, because it is
> >> the one which validates guest requests against a cached size.
> >> virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
> >> blk_get_geometry() for every request, so a missing notification leaves
> >> those guests with a stale idea of the size but never refuses I/O which the
> >> node underneath can serve. check_lba_range() compares against
> >> SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
> >> READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
> >> re-read the capacity every request past the end of the old node is refused
> >> for the whole life of the device.
> >>
> >> This is reachable with qom-set of the 'drive' property, which is allowed
> >> on a realized device, and it is silent on the host: out-of-range requests
> >> are answered by scsi_check_condition() and never reach
> >> scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
> >> stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
> >> RANGE, which Linux turns into EREMOTEIO for reads as well as writes.
> >>
> >> Emit the notification from bdrv_replace_child_bs(), so that every device
> >> model gets it through the path it already implements rather than any of
> >> them being special cased. Send it only when the size really changed, which
> >> keeps the callback out of the common case of a replacement by an equally
> >> sized node.
> >>
> >> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> ---
> >>  block.c                      | 13 +++++++++++++
> >>  include/block/block_int-io.h |  3 +++
> >>  2 files changed, 16 insertions(+)
> >>
> >> diff --git a/block.c b/block.c
> >> index f0a6042e61..1fe66caf8d 100644
> >> --- a/block.c
> >> +++ b/block.c
> >> @@ -5524,9 +5524,12 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
> >>      Transaction *tran = tran_new();
> >>      g_autoptr(GSList) refresh_list = NULL;
> >>      BlockDriverState *old_bs = child->bs;
> >> +    int64_t old_len;
> >>  
> >>      GLOBAL_STATE_CODE();
> >>  
> >> +    old_len = bdrv_getlength(old_bs);
> >> +
> >>      bdrv_ref(old_bs);
> >>      bdrv_drained_begin(old_bs);
> >>      bdrv_drained_begin(new_bs);
> > bdrv_drained_begin() involves nested event loops. Can't old_len become
> > stale here? It would feel safer to query it after draining.
> I do not think it can. The only caller is blk_replace_bs(), so @child is
> the root of a device and old_len is the virtual size the device checks
> every request against. blk_check_byte_request() refuses writes past it,
> and block_resize is a coroutine QMP command, so it cannot run while the
> qom-set one does. Any size change that does happen goes through
> bdrv_co_write_req_finish(), which notifies the parents of old_bs, @child
> included, on its own.

We're operating at the node level here, not BlockBackend level, so the
node could have multiple parents. For example, in the past, surprising
changes were sometimes caused by a block job completing at the wrong
time. But you could just as well receive a truncate from an export that
is run during the drain.

> If the size could really move in the middle of a replacement, comparing
> it at this point would not make sense in the first place, no matter
> whether it is read before or after the drain.

If you perform all of the querying, the comparison and notification
within the same drained section, it should be consistent. Then you'd
either get the notification from the concurrent size change (if the size
change is run first) or from the node change (if that is first).

> >> @@ -5542,6 +5545,16 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
> >>      tran_finalize(tran, ret);
> >>  
> >>      bdrv_graph_wrunlock();
> >> +
> >> +    /* A parent caching the child size has to learn that it changed */
> >> +    if (ret == 0) {
> >> +        int64_t new_len = bdrv_getlength(new_bs);
> >> +
> >> +        if (new_len >= 0 && new_len != old_len) {
> >> +            bdrv_parent_cb_resize(new_bs);
> > This notifies other parents of the new node as well even though nothing
> > changed for them.
> >
> > Especially since you already query old_len/new_len to avoid unnecessary
> > notifications, we should probably call only child->klass->resize(child)
> > through a coroutine wrapper here to notify only the BdrvChild that we
> > actually changed.
> True, but the extra call costs nothing. The resize callback re-reads the
> size from the node: virtio-blk sends a config interrupt, scsi-disk
> raises CAPACITY DATA HAS CHANGED, and the guest finds the capacity it
> already had. A second parent on the new node also needs share-rw on both
> devices, so this is rare.

Yes, I think you can argue that additional notifications don't hurt. In
this case, we should be consistent and not compare sizes for the parent
getting its child changed either.

For a node with an existing parent, notifying this parent if, and only
if, a newly added parent experienced a size change is really strange
and will make everybody think that this code wasn't thought through.

> A dedicated coroutine wrapper for one call site does not look worth it
> to me. If you still prefer it, I will send the patch. 

You're already adding a coroutine wrapper for bdrv_co_parent_cb_resize()
in this patch, so I don't think it would be very different.

Kevin

Re: [PATCH 1/4] block: notify the parent when a replaced child has a different size

[Denis V. Lunev]

On 11/5/26 16:41, Kevin Wolf wrote:
> Am 04.11.2026 um 15:50 hat Denis V. Lunev geschrieben:
>> On 11/3/26 11:02, Kevin Wolf wrote:
>>> Am 12.08.2026 um 14:14 hat Denis V. Lunev geschrieben:
>>>> bdrv_replace_child_bs() swaps the node a BdrvChild points to without
>>>> telling the parent anything about it, so a parent which tracks the size of
>>>> its child never learns that it changed. Every other way the size of a node
>>>> changes goes through bdrv_co_truncate(), which does notify.
>>>>
>>>> The device models which care all register the same BlockDevOps hook:
>>>>
>>>>   virtio-blk  resize_cb = virtio_blk_resize(), ending up in
>>>>               virtio_notify_config()
>>>>   ide/ahci    resize_cb = ide_resize_cb(), refreshing the cached size
>>>>               and the IDENTIFY data
>>>>   scsi-disk   resize_cb = scsi_disk_resize_cb(), reporting
>>>>               CAPACITY DATA HAS CHANGED
>>>>   xen-block   resize_cb = xen_block_resize_cb()
>>>>
>>>> Only scsi-disk is really hurt by the missing notification, because it is
>>>> the one which validates guest requests against a cached size.
>>>> virtio_blk_sect_range_ok() and ide_sect_range_ok() both call
>>>> blk_get_geometry() for every request, so a missing notification leaves
>>>> those guests with a stale idea of the size but never refuses I/O which the
>>>> node underneath can serve. check_lba_range() compares against
>>>> SCSIDevice.max_lba, filled in by scsi_disk_reset() and updated only by the
>>>> READ CAPACITY(10) and (16) handlers, so with nothing to make the guest
>>>> re-read the capacity every request past the end of the old node is refused
>>>> for the whole life of the device.
>>>>
>>>> This is reachable with qom-set of the 'drive' property, which is allowed
>>>> on a realized device, and it is silent on the host: out-of-range requests
>>>> are answered by scsi_check_condition() and never reach
>>>> scsi_handle_rw_error(), so there is no BLOCK_IO_ERROR event and io-status
>>>> stays 'ok'. The guest sees ILLEGAL REQUEST / LOGICAL BLOCK ADDRESS OUT OF
>>>> RANGE, which Linux turns into EREMOTEIO for reads as well as writes.
>>>>
>>>> Emit the notification from bdrv_replace_child_bs(), so that every device
>>>> model gets it through the path it already implements rather than any of
>>>> them being special cased. Send it only when the size really changed, which
>>>> keeps the callback out of the common case of a replacement by an equally
>>>> sized node.
>>>>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> ---
>>>>  block.c                      | 13 +++++++++++++
>>>>  include/block/block_int-io.h |  3 +++
>>>>  2 files changed, 16 insertions(+)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index f0a6042e61..1fe66caf8d 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -5524,9 +5524,12 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>>>>      Transaction *tran = tran_new();
>>>>      g_autoptr(GSList) refresh_list = NULL;
>>>>      BlockDriverState *old_bs = child->bs;
>>>> +    int64_t old_len;
>>>>  
>>>>      GLOBAL_STATE_CODE();
>>>>  
>>>> +    old_len = bdrv_getlength(old_bs);
>>>> +
>>>>      bdrv_ref(old_bs);
>>>>      bdrv_drained_begin(old_bs);
>>>>      bdrv_drained_begin(new_bs);
>>> bdrv_drained_begin() involves nested event loops. Can't old_len become
>>> stale here? It would feel safer to query it after draining.
>> I do not think it can. The only caller is blk_replace_bs(), so @child is
>> the root of a device and old_len is the virtual size the device checks
>> every request against. blk_check_byte_request() refuses writes past it,
>> and block_resize is a coroutine QMP command, so it cannot run while the
>> qom-set one does. Any size change that does happen goes through
>> bdrv_co_write_req_finish(), which notifies the parents of old_bs, @child
>> included, on its own.
> We're operating at the node level here, not BlockBackend level, so the
> node could have multiple parents. For example, in the past, surprising
> changes were sometimes caused by a block job completing at the wrong
> time. But you could just as well receive a truncate from an export that
> is run during the drain.
>
>> If the size could really move in the middle of a replacement, comparing
>> it at this point would not make sense in the first place, no matter
>> whether it is read before or after the drain.
> If you perform all of the querying, the comparison and notification
> within the same drained section, it should be consistent. Then you'd
> either get the notification from the concurrent size change (if the size
> change is run first) or from the node change (if that is first).
>
>>>> @@ -5542,6 +5545,16 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
>>>>      tran_finalize(tran, ret);
>>>>  
>>>>      bdrv_graph_wrunlock();
>>>> +
>>>> +    /* A parent caching the child size has to learn that it changed */
>>>> +    if (ret == 0) {
>>>> +        int64_t new_len = bdrv_getlength(new_bs);
>>>> +
>>>> +        if (new_len >= 0 && new_len != old_len) {
>>>> +            bdrv_parent_cb_resize(new_bs);
>>> This notifies other parents of the new node as well even though nothing
>>> changed for them.
>>>
>>> Especially since you already query old_len/new_len to avoid unnecessary
>>> notifications, we should probably call only child->klass->resize(child)
>>> through a coroutine wrapper here to notify only the BdrvChild that we
>>> actually changed.
>> True, but the extra call costs nothing. The resize callback re-reads the
>> size from the node: virtio-blk sends a config interrupt, scsi-disk
>> raises CAPACITY DATA HAS CHANGED, and the guest finds the capacity it
>> already had. A second parent on the new node also needs share-rw on both
>> devices, so this is rare.
> Yes, I think you can argue that additional notifications don't hurt. In
> this case, we should be consistent and not compare sizes for the parent
> getting its child changed either.
>
> For a node with an existing parent, notifying this parent if, and only
> if, a newly added parent experienced a size change is really strange
> and will make everybody think that this code wasn't thought through.
>
>> A dedicated coroutine wrapper for one call site does not look worth it
>> to me. If you still prefer it, I will send the patch. 
> You're already adding a coroutine wrapper for bdrv_co_parent_cb_resize()
> in this patch, so I don't think it would be very different.
>
> Kevin
no prob at all. Will do.

Thanks,
    Den