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

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] block: bdrv_replace_resize_commit() notifies all parents of new_bs
--

commit 1388b90801798d7d441e56e99ea5745089939495
Author: Denis V. Lunev <den@openvz.org>

block: notify the parent when a replaced child has a different size #VSTOR-130961

This commit makes bdrv_replace_child_bs() register a transaction action. When the transaction commits, the action compares the lengths of the old and new nodes. If they differ, it calls bdrv_parent_cb_resize() on the new node, so parents that cache the child size learn about the change.

> diff --git a/block.c b/block.c
> --- a/block.c
> +++ b/block.c

[ ... ]

> +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);
> +    }
> +}

[Severity: Low]
Is the notification broader than the commit message describes? The subject and body talk about notifying "the parent" of the replaced child. However, bdrv_replace_resize_commit() calls bdrv_parent_cb_resize(s->new_bs), which wraps bdrv_co_parent_cb_resize(). That function walks every entry in bs->parents:

block/io.c:bdrv_co_parent_cb_resize() {
    ...
    QLIST_FOREACH(c, &bs->parents, next_parent) {
        if (c->klass->resize) {
            c->klass->resize(c);
        }
    }
}

BdrvReplaceResizeState records only old_bs and new_bs, not the replaced BdrvChild. The old_len versus new_len check says nothing about the other parents of new_bs.

When the commit action runs, the replaced child is already a parent of new_bs. Any other BlockBackend, export or device model attached to new_bs also gets resize(). Their view of the size did not change.

Can this send redundant notifications to those parents? For example, virtio-blk would get an extra config-change interrupt. scsi-disk would report a spurious CAPACITY DATA HAS CHANGED unit attention.

Would it be better to notify only the parent that was replaced, or to describe the broader behavior in the commit message?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104134539.1519657-1-den@openvz.org?part=2