Re: [PATCH v2 1/3] block: add bdrv_open_backing_chain_until

[Kevin Wolf <kwolf@redhat.com>]

Am 15.09.2026 um 16:04 hat Jean-Louis Dupond geschrieben:
> Add a new function called bdrv_open_backing_chain_until.
> This is used to open a block up until a specified backing chain.
> 
> Modified bdrv_open_inherit to accept a new arg
> 'open_backing_recursively' to force to not open the next backing.
> Adjusting all references to keep the existing behaviour.
> 
> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

Do you actually have a use case where limiting the backing chain is a
requirement? It looks a bit like premature optimisation, because for
running a VM, you'll open the whole chain anyway, so it can't be
unworkably long.

> diff --git a/block.c b/block.c
> index f0a6042e61..9763137d8c 100644
> --- a/block.c
> +++ b/block.c
> @@ -83,6 +83,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
>  static BlockDriverState *bdrv_open_inherit(const char *filename,
>                                             const char *reference,
>                                             QDict *options, int flags,
> +                                           bool open_backing_recursively,
>                                             BlockDriverState *parent,
>                                             const BdrvChildClass *child_class,
>                                             BdrvChildRole child_role,

bdrv_open_inherit() already has too many parameters. Can't you just pass
BDRV_O_NO_BACKING instead of adding a new one?

> @@ -3733,6 +3735,54 @@ free_exit:
>      return ret;
>  }
>  
> +int
> +bdrv_open_backing_chain_until(BlockDriverState *top_bs,
> +                              const char *base_filename,
> +                              Error **errp)
> +{
> +    BlockDriverState *curr = top_bs;
> +    int ret;
> +
> +    GLOBAL_STATE_CODE();
> +
> +    if (!base_filename) {
> +        return 0;
> +    }
> +
> +    if (bdrv_find_backing_image(top_bs, base_filename)) {
> +        return 0;
> +    }
> +
> +    while (true) {
> +        QDict *options = qdict_clone_shallow(curr->options);
> +
> +        ret = bdrv_open_backing_file(curr, options, false, "backing", errp);
> +        qobject_unref(options);

Does this actually do anything? I don't think curr->options can contain
"backing" options when you opened it without backing files. It would
already fail during the initial open because bdrv_open_backing_file()
doesn't absorb the options, so they are considered unknown:

$ ./qemu-img info --image-opts driver=qcow2,file.filename=/tmp/overlay.qcow2,backing=foo
qemu-img: Could not open 'driver=qcow2,file.filename=/tmp/overlay.qcow2,backing=foo': Block format 'qcow2' does not support the option 'backing'

(Using 'qemu-img info' as an example that uses BDRV_O_NO_BACKING.)

In your specific 'qemu-img measure' implementation, it seems even
impossible to specify backing file options in the BDRV_O_NO_BACKING case
in the first place because --image-opts disables the partial loading.
Which might actually be another reason not to do the partial opening at
all because it's inconsistent behaviour.

> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        bdrv_graph_rdlock_main_loop();
> +        if (!curr->backing) {
> +            bdrv_graph_rdunlock_main_loop();
> +            error_setg(errp,
> +                       "Did not find '%s' in the backing chain of '%s'",
> +                       base_filename, top_bs->filename);
> +            return -ENOENT;
> +        }
> +        bdrv_graph_rdunlock_main_loop();
> +
> +        if (bdrv_find_backing_image(curr, base_filename)) {
> +            return 0;
> +        }
> +
> +        /* Switch to the next layer */
> +        bdrv_graph_rdlock_main_loop();
> +        curr = curr->backing->bs;
> +        bdrv_graph_rdunlock_main_loop();
> +    }
> +}

Kevin