[PATCH hci-dev 2/6] block: notify the parent when a replaced child has a different size #VSTOR-130961

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

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.

[ Ported to hci-dev: the downstream bdrv_replace_child_bs() only adds
  its steps to the caller's Transaction, and the callers hold the graph
  write lock while it runs, so neither the length query nor the
  notification can be done there. Both happen in a .commit action
  instead: tran_finalize() runs after the lock is dropped, and both
  nodes stay referenced and drained by bdrv_drained() until .clean. ]

Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-ID: <20260812121500.1034178-2-den@openvz.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 8b267376df877681d0ac9fbe24035d06bf6ed47f)
---
 block.c                      | 27 +++++++++++++++++++++++++++
 include/block/block_int-io.h |  3 +++
 2 files changed, 30 insertions(+)

diff --git a/block.c b/block.c
index 7b27cb4edf3..1dc7e47431c 100644
--- a/block.c
+++ b/block.c
@@ -5605,6 +5605,28 @@ static void bdrv_drained(BlockDriverState *bs, Transaction *tran)
     tran_add(tran, &bdrv_drained_drv, bs);
 }
 
+typedef struct BdrvReplaceResizeState {
+    BlockDriverState *old_bs;
+    BlockDriverState *new_bs;
+} BdrvReplaceResizeState;
+
+static void bdrv_replace_resize_commit(void *opaque)
+{
+    BdrvReplaceResizeState *s = opaque;
+    int64_t old_len = bdrv_getlength(s->old_bs);
+    int64_t new_len = bdrv_getlength(s->new_bs);
+
+    /* A parent caching the child size has to learn that it changed */
+    if (old_len >= 0 && new_len >= 0 && new_len != old_len) {
+        bdrv_parent_cb_resize(s->new_bs);
+    }
+}
+
+static TransactionActionDrv bdrv_replace_resize_drv = {
+    .commit = bdrv_replace_resize_commit,
+    .clean = g_free,
+};
+
 /* Not for empty child */
 int TSA_NO_TSA
 bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
@@ -5612,9 +5634,14 @@ bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
 {
     g_autoptr(GSList) refresh_list = NULL;
     BlockDriverState *old_bs = child->bs;
+    BdrvReplaceResizeState *s;
 
     GLOBAL_STATE_CODE();
 
+    s = g_new(BdrvReplaceResizeState, 1);
+    *s = (BdrvReplaceResizeState) { .old_bs = old_bs, .new_bs = new_bs };
+    tran_add(tran, &bdrv_replace_resize_drv, s);
+
     bdrv_drained(old_bs, tran);
     bdrv_drained(new_bs, tran);
 
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index e462b33b515..6f0c278bbd4 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -200,4 +200,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