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

[Kevin Wolf <kwolf@redhat.com>]

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