Add a command that can replace bs in following BdrvChild structures:

 - qdev blk root child
 - block-export blk root child
 - any child of BlockDriverState selected by child-name

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 blockdev.c             | 56 +++++++++++++++++++++++++++
 qapi/block-core.json   | 88 ++++++++++++++++++++++++++++++++++++++++++
 stubs/blk-by-qdev-id.c | 13 +++++++
 stubs/meson.build      |  1 +
 4 files changed, 158 insertions(+)
 create mode 100644 stubs/blk-by-qdev-id.c

diff --git a/blockdev.c b/blockdev.c
index ba7e90b06e..2190467022 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3559,6 +3559,62 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
     bdrv_try_change_aio_context(bs, new_context, NULL, errp);
 }
 
+void qmp_blockdev_replace(BlockdevReplace *repl, Error **errp)
+{
+    BdrvChild *child = NULL;
+    BlockDriverState *new_child_bs;
+
+    if (repl->parent_type == BLOCK_PARENT_TYPE_DRIVER) {
+        BlockDriverState *parent_bs;
+
+        parent_bs = bdrv_find_node(repl->u.driver.node_name);
+        if (!parent_bs) {
+            error_setg(errp, "Block driver node with node-name '%s' not "
+                       "found", repl->u.driver.node_name);
+            return;
+        }
+
+        child = bdrv_find_child(parent_bs, repl->u.driver.child);
+        if (!child) {
+            error_setg(errp, "Block driver node '%s' doesn't have child "
+                       "named '%s'", repl->u.driver.node_name,
+                       repl->u.driver.child);
+            return;
+        }
+    } else {
+        /* Other types are similar, they work through blk */
+        BlockBackend *blk;
+        bool is_qdev = repl->parent_type == BLOCK_PARENT_TYPE_QDEV;
+        const char *id =
+            is_qdev ? repl->u.qdev.qdev_id : repl->u.export.export_id;
+
+        assert(is_qdev || repl->parent_type == BLOCK_PARENT_TYPE_EXPORT);
+
+        blk = is_qdev ? blk_by_qdev_id(id, errp) : blk_by_export_id(id, errp);
+        if (!blk) {
+            return;
+        }
+
+        child = blk_root(blk);
+        if (!child) {
+            error_setg(errp, "%s '%s' is empty, nothing to replace",
+                       is_qdev ? "Device" : "Export", id);
+            return;
+        }
+    }
+
+    assert(child);
+    assert(child->bs);
+
+    new_child_bs = bdrv_find_node(repl->new_child);
+    if (!new_child_bs) {
+        error_setg(errp, "Node '%s' not found", repl->new_child);
+        return;
+    }
+
+    bdrv_replace_child_bs(child, new_child_bs, errp);
+}
+
 QemuOptsList qemu_common_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
diff --git a/qapi/block-core.json b/qapi/block-core.json
index df5e07debd..0a6f08a6e0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6148,3 +6148,91 @@
 ##
 { 'struct': 'DummyBlockCoreForceArrays',
   'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
+
+##
+# @BlockParentType:
+#
+# @qdev: block device, such as created by device_add, and denoted by
+#     qdev-id
+#
+# @driver: block driver node, such as created by blockdev-add, and
+#     denoted by node-name
+#
+# @export: block export, such created by block-export-add, and
+#     denoted by export-id
+#
+# Since 9.1
+##
+{ 'enum': 'BlockParentType',
+  'data': ['qdev', 'driver', 'export'] }
+
+##
+# @BdrvChildRefQdev:
+#
+# @qdev-id: the device's ID or QOM path
+#
+# Since 9.1
+##
+{ 'struct': 'BdrvChildRefQdev',
+  'data': { 'qdev-id': 'str' } }
+
+##
+# @BdrvChildRefExport:
+#
+# @export-id: block export identifier
+#
+# Since 9.1
+##
+{ 'struct': 'BdrvChildRefExport',
+  'data': { 'export-id': 'str' } }
+
+##
+# @BdrvChildRefDriver:
+#
+# @node-name: the node name of the parent block node
+#
+# @child: name of the child to be replaced, like "file" or "backing"
+#
+# Since 9.1
+##
+{ 'struct': 'BdrvChildRefDriver',
+  'data': { 'node-name': 'str', 'child': 'str' } }
+
+##
+# @BlockdevReplace:
+#
+# @parent-type: type of the parent, which child is to be replaced
+#
+# @new-child: new child for replacement
+#
+# Since 9.1
+##
+{ 'union': 'BlockdevReplace',
+  'base': {
+      'parent-type': 'BlockParentType',
+      'new-child': 'str'
+  },
+  'discriminator': 'parent-type',
+  'data': {
+      'qdev': 'BdrvChildRefQdev',
+      'export': 'BdrvChildRefExport',
+      'driver': 'BdrvChildRefDriver'
+  } }
+
+##
+# @blockdev-replace:
+#
+# Replace a block-node associated with device (selected by
+# @qdev-id) or with block-export (selected by @export-id) or
+# any child of block-node (selected by @node-name and @child)
+# with @new-child block-node.
+#
+# Features:
+#
+# @unstable: This command is experimental.
+#
+# Since 9.1
+##
+{ 'command': 'blockdev-replace', 'boxed': true,
+  'features': [ 'unstable' ],
+  'data': 'BlockdevReplace' }
diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
new file mode 100644
index 0000000000..5ec9f755ee
--- /dev/null
+++ b/stubs/blk-by-qdev-id.c
@@ -0,0 +1,13 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/block-backend.h"
+
+BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
+{
+    /*
+     * We expect this when blockdev-change is called with parent-type=qdev,
+     * but qdev-monitor is not linked in. So no blk_ API is not available.
+     */
+    error_setg(errp, "Parameter 'parent-type' does not accept value 'qdev'");
+    return NULL;
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index 772a3e817d..068998c1a5 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -15,6 +15,7 @@ if have_block
   stub_ss.add(files('bdrv-next-monitor-owned.c'))
   stub_ss.add(files('blk-commit-all.c'))
   stub_ss.add(files('blk-exp-close-all.c'))
+  stub_ss.add(files('blk-by-qdev-id.c'))
   stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
   stub_ss.add(files('change-state-handler.c'))
   stub_ss.add(files('get-vm-name.c'))
-- 
2.34.1

On 26.06.24 14:53, Vladimir Sementsov-Ogievskiy wrote:
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index df5e07debd..0a6f08a6e0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6148,3 +6148,91 @@
>   ##
>   { 'struct': 'DummyBlockCoreForceArrays',
>     'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> +
> +##
> +# @BlockParentType:
> +#
> +# @qdev: block device, such as created by device_add, and denoted by
> +#     qdev-id
> +#
> +# @driver: block driver node, such as created by blockdev-add, and
> +#     denoted by node-name
> +#
> +# @export: block export, such created by block-export-add, and
> +#     denoted by export-id
> +#
> +# Since 9.1
> +##
> +{ 'enum': 'BlockParentType',
> +  'data': ['qdev', 'driver', 'export'] }
> +
> +##
> +# @BdrvChildRefQdev:
> +#
> +# @qdev-id: the device's ID or QOM path
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefQdev',
> +  'data': { 'qdev-id': 'str' } }
> +
> +##
> +# @BdrvChildRefExport:
> +#
> +# @export-id: block export identifier
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefExport',
> +  'data': { 'export-id': 'str' } }
> +
> +##
> +# @BdrvChildRefDriver:
> +#
> +# @node-name: the node name of the parent block node
> +#
> +# @child: name of the child to be replaced, like "file" or "backing"
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefDriver',
> +  'data': { 'node-name': 'str', 'child': 'str' } }
> +
> +##
> +# @BlockdevReplace:
> +#
> +# @parent-type: type of the parent, which child is to be replaced
> +#
> +# @new-child: new child for replacement
> +#
> +# Since 9.1
> +##
> +{ 'union': 'BlockdevReplace',
> +  'base': {
> +      'parent-type': 'BlockParentType',
> +      'new-child': 'str'
> +  },
> +  'discriminator': 'parent-type',
> +  'data': {
> +      'qdev': 'BdrvChildRefQdev',
> +      'export': 'BdrvChildRefExport',
> +      'driver': 'BdrvChildRefDriver'
> +  } }
> +
> +##
> +# @blockdev-replace:
> +#
> +# Replace a block-node associated with device (selected by
> +# @qdev-id) or with block-export (selected by @export-id) or
> +# any child of block-node (selected by @node-name and @child)
> +# with @new-child block-node.
> +#
> +# Features:
> +#
> +# @unstable: This command is experimental.
> +#
> +# Since 9.1
> +##
> +{ 'command': 'blockdev-replace', 'boxed': true,
> +  'features': [ 'unstable' ],
> +  'data': 'BlockdevReplace' }


Looking back at this all, I now have another idea: instead of trying to unite different types of parents, maybe just publish concept of BdrvChild to QAPI? So that it will have unique id. Like for block-nodes, it could be auto-generated or specified by user.

Then we'll add parameters for commands:

device-add
    root-child-slot-id: ID

block-export-add
    block-child-slot-id: ID

and for block-drivers we already have BlockdevRef structure, it only lacks an id.

{ 'alternate': 'BlockdevRef',
   'data': { 'definition': 'BlockdevOptions',
             'reference': 'str' } }

hmm.. Could it be as simple as


{ 'alternate': 'BlockdevRef',
   'base': { '*child-slot-id': 'str' },
   'data': { 'definition': 'BlockdevOptions',
             'reference': 'str' } }

?

Unfortunately, no: "../qapi/block-core.json:4781: alternate has unknown key 'base'"

Anyway, I believe, some solution should exist, probably by improving QAPI generator. Or, add "child-slot-id" to base of BlockdevOptions, and add virtual "reference" BlockdevDriver, to handle case with reference.


----

And finally, the new command becomes as simple as:

{ 'command': 'blockdev-replace',
   'data': {'child-slot': 'str', 'new-child': 'str' } }

-- 
Best regards,
Vladimir

On 02.10.24 17:41, Vladimir Sementsov-Ogievskiy wrote:
> On 26.06.24 14:53, Vladimir Sementsov-Ogievskiy wrote:
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index df5e07debd..0a6f08a6e0 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -6148,3 +6148,91 @@
>>   ##
>>   { 'struct': 'DummyBlockCoreForceArrays',
>>     'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
>> +
>> +##
>> +# @BlockParentType:
>> +#
>> +# @qdev: block device, such as created by device_add, and denoted by
>> +#     qdev-id
>> +#
>> +# @driver: block driver node, such as created by blockdev-add, and
>> +#     denoted by node-name
>> +#
>> +# @export: block export, such created by block-export-add, and
>> +#     denoted by export-id
>> +#
>> +# Since 9.1
>> +##
>> +{ 'enum': 'BlockParentType',
>> +  'data': ['qdev', 'driver', 'export'] }
>> +
>> +##
>> +# @BdrvChildRefQdev:
>> +#
>> +# @qdev-id: the device's ID or QOM path
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefQdev',
>> +  'data': { 'qdev-id': 'str' } }
>> +
>> +##
>> +# @BdrvChildRefExport:
>> +#
>> +# @export-id: block export identifier
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefExport',
>> +  'data': { 'export-id': 'str' } }
>> +
>> +##
>> +# @BdrvChildRefDriver:
>> +#
>> +# @node-name: the node name of the parent block node
>> +#
>> +# @child: name of the child to be replaced, like "file" or "backing"
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefDriver',
>> +  'data': { 'node-name': 'str', 'child': 'str' } }
>> +
>> +##
>> +# @BlockdevReplace:
>> +#
>> +# @parent-type: type of the parent, which child is to be replaced
>> +#
>> +# @new-child: new child for replacement
>> +#
>> +# Since 9.1
>> +##
>> +{ 'union': 'BlockdevReplace',
>> +  'base': {
>> +      'parent-type': 'BlockParentType',
>> +      'new-child': 'str'
>> +  },
>> +  'discriminator': 'parent-type',
>> +  'data': {
>> +      'qdev': 'BdrvChildRefQdev',
>> +      'export': 'BdrvChildRefExport',
>> +      'driver': 'BdrvChildRefDriver'
>> +  } }
>> +
>> +##
>> +# @blockdev-replace:
>> +#
>> +# Replace a block-node associated with device (selected by
>> +# @qdev-id) or with block-export (selected by @export-id) or
>> +# any child of block-node (selected by @node-name and @child)
>> +# with @new-child block-node.
>> +#
>> +# Features:
>> +#
>> +# @unstable: This command is experimental.
>> +#
>> +# Since 9.1
>> +##
>> +{ 'command': 'blockdev-replace', 'boxed': true,
>> +  'features': [ 'unstable' ],
>> +  'data': 'BlockdevReplace' }
> 
> 
> Looking back at this all, I now have another idea: instead of trying to unite different types of parents, maybe just publish concept of BdrvChild to QAPI? So that it will have unique id. Like for block-nodes, it could be auto-generated or specified by user.
> 
> Then we'll add parameters for commands:
> 
> device-add
>     root-child-slot-id: ID
> 
> block-export-add
>     block-child-slot-id: ID
> 
> and for block-drivers we already have BlockdevRef structure, it only lacks an id.
> 
> { 'alternate': 'BlockdevRef',
>    'data': { 'definition': 'BlockdevOptions',
>              'reference': 'str' } }
> 
> hmm.. Could it be as simple as
> 
> 
> { 'alternate': 'BlockdevRef',
>    'base': { '*child-slot-id': 'str' },
>    'data': { 'definition': 'BlockdevOptions',
>              'reference': 'str' } }
> 
> ?

Oops that was obviously impossible idea :) Then, I think the only way is to introduce virtual "reference" BlockdevDriver, with only one parameter { 'reference': 'str' }, this way user will be able to specify

file: {driver: reference, reference: NODE_NAME, child-slot-id: NEW_ID}

> 
> Unfortunately, no: "../qapi/block-core.json:4781: alternate has unknown key 'base'"
> 
> Anyway, I believe, some solution should exist, probably by improving QAPI generator. Or, add "child-slot-id" to base of BlockdevOptions, and add virtual "reference" BlockdevDriver, to handle case with reference.
> 
> 
> ----
> 
> And finally, the new command becomes as simple as:
> 
> { 'command': 'blockdev-replace',
>    'data': {'child-slot': 'str', 'new-child': 'str' } }
> 

-- 
Best regards,
Vladimir

Am 04.10.2024 um 19:01 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 02.10.24 17:41, Vladimir Sementsov-Ogievskiy wrote:
> > On 26.06.24 14:53, Vladimir Sementsov-Ogievskiy wrote:
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index df5e07debd..0a6f08a6e0 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -6148,3 +6148,91 @@
> > >   ##
> > >   { 'struct': 'DummyBlockCoreForceArrays',
> > >     'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> > > +
> > > +##
> > > +# @BlockParentType:
> > > +#
> > > +# @qdev: block device, such as created by device_add, and denoted by
> > > +#     qdev-id
> > > +#
> > > +# @driver: block driver node, such as created by blockdev-add, and
> > > +#     denoted by node-name
> > > +#
> > > +# @export: block export, such created by block-export-add, and
> > > +#     denoted by export-id
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'enum': 'BlockParentType',
> > > +  'data': ['qdev', 'driver', 'export'] }
> > > +
> > > +##
> > > +# @BdrvChildRefQdev:
> > > +#
> > > +# @qdev-id: the device's ID or QOM path
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'struct': 'BdrvChildRefQdev',
> > > +  'data': { 'qdev-id': 'str' } }
> > > +
> > > +##
> > > +# @BdrvChildRefExport:
> > > +#
> > > +# @export-id: block export identifier
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'struct': 'BdrvChildRefExport',
> > > +  'data': { 'export-id': 'str' } }
> > > +
> > > +##
> > > +# @BdrvChildRefDriver:
> > > +#
> > > +# @node-name: the node name of the parent block node
> > > +#
> > > +# @child: name of the child to be replaced, like "file" or "backing"
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'struct': 'BdrvChildRefDriver',
> > > +  'data': { 'node-name': 'str', 'child': 'str' } }
> > > +
> > > +##
> > > +# @BlockdevReplace:
> > > +#
> > > +# @parent-type: type of the parent, which child is to be replaced
> > > +#
> > > +# @new-child: new child for replacement
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'union': 'BlockdevReplace',
> > > +  'base': {
> > > +      'parent-type': 'BlockParentType',
> > > +      'new-child': 'str'
> > > +  },
> > > +  'discriminator': 'parent-type',
> > > +  'data': {
> > > +      'qdev': 'BdrvChildRefQdev',
> > > +      'export': 'BdrvChildRefExport',
> > > +      'driver': 'BdrvChildRefDriver'
> > > +  } }
> > > +
> > > +##
> > > +# @blockdev-replace:
> > > +#
> > > +# Replace a block-node associated with device (selected by
> > > +# @qdev-id) or with block-export (selected by @export-id) or
> > > +# any child of block-node (selected by @node-name and @child)
> > > +# with @new-child block-node.
> > > +#
> > > +# Features:
> > > +#
> > > +# @unstable: This command is experimental.
> > > +#
> > > +# Since 9.1
> > > +##
> > > +{ 'command': 'blockdev-replace', 'boxed': true,
> > > +  'features': [ 'unstable' ],
> > > +  'data': 'BlockdevReplace' }
> > 
> > 
> > Looking back at this all, I now have another idea: instead of trying
> > to unite different types of parents, maybe just publish concept of
> > BdrvChild to QAPI? So that it will have unique id. Like for
> > block-nodes, it could be auto-generated or specified by user.
> > 
> > Then we'll add parameters for commands:
> > 
> > device-add
> >     root-child-slot-id: ID
> > 
> > block-export-add
> >     block-child-slot-id: ID
> > 
> > and for block-drivers we already have BlockdevRef structure, it only
> > lacks an id.
> > 
> > { 'alternate': 'BlockdevRef',
> >    'data': { 'definition': 'BlockdevOptions',
> >              'reference': 'str' } }
> > 
> > hmm.. Could it be as simple as
> > 
> > 
> > { 'alternate': 'BlockdevRef',
> >    'base': { '*child-slot-id': 'str' },
> >    'data': { 'definition': 'BlockdevOptions',
> >              'reference': 'str' } }
> > 
> > ?
> 
> Oops that was obviously impossible idea :) Then, I think the only way
> is to introduce virtual "reference" BlockdevDriver, with only one
> parameter { 'reference': 'str' }, this way user will be able to
> specify
> 
> file: {driver: reference, reference: NODE_NAME, child-slot-id: NEW_ID}

I don't think adding such a hack would make the interface any nicer
compared to what you have now with the union.

If we want to get rid of the union, I think the best course of action
would unifying the namespaces (so that nodes, exports and devices can't
share the same ID) and then we could just accept a universal 'id' along
with 'child'.

This would mean having 'child' even for devices, which feels
unnecessary at least in the common case, but it would at the same time
resolve Markus' concern if there could be any devices with multiple
BlockBackends.

I can only think of isa-fdc that used to be such a device, but that was
just incorrect modelling on the qdev level. Not sure if there are actual
legitimate use cases for having multiple BlockBackends.

Anyway, for the moment, I would suggest going ahead with the union.

Kevin

On 18.10.24 16:59, Kevin Wolf wrote:
> If we want to get rid of the union, I think the best course of action
> would unifying the namespaces (so that nodes, exports and devices can't
> share the same ID) and then we could just accept a universal 'id' along
> with 'child'.

Maybe we can go this way even without explicit restriction (which should
some how go through deprecation period, etc), but simply look for the id
among nodes, devices and exports and if found more than one parent - fail.

And we document, that id should not be ambiguous, should not match more
than one parent object. So, those who want to use new command will care
to make unique ids.

-- 
Best regards,
Vladimir

Hi Vladimir,

I remembered this series and wanted to check what the current status is,
because I seemed to remember that the next step was that you would send
a new version. But reading it again, you're probably waiting for more
input? Let's try to get this finished.

Am 02.04.2025 um 15:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 18.10.24 16:59, Kevin Wolf wrote:
> > If we want to get rid of the union, I think the best course of action
> > would unifying the namespaces (so that nodes, exports and devices can't
> > share the same ID) and then we could just accept a universal 'id' along
> > with 'child'.
> 
> Maybe we can go this way even without explicit restriction (which
> should some how go through deprecation period, etc), but simply look
> for the id among nodes, devices and exports and if found more than one
> parent - fail.
> 
> And we document, that id should not be ambiguous, should not match more
> than one parent object. So, those who want to use new command will care
> to make unique ids.

I don't think such a state is very pretty, but it would be okay for me
as an intermediate state while we go through a deprecation period to
restrict IDs accordingly.

So we could start with blockdev-replace returning an error on ambiguous
IDs and at the same time deprecate them, and only later we would make
creating nodes/devices/exports with the same ID an error.

Kevin

On 17.11.25 13:49, Kevin Wolf wrote:
> Hi Vladimir,
> 
> I remembered this series and wanted to check what the current status is,
> because I seemed to remember that the next step was that you would send
> a new version. But reading it again, you're probably waiting for more
> input? Let's try to get this finished.

I think yes, I was waiting, but then switched to other tasks.

> 
> Am 02.04.2025 um 15:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> On 18.10.24 16:59, Kevin Wolf wrote:
>>> If we want to get rid of the union, I think the best course of action
>>> would unifying the namespaces (so that nodes, exports and devices can't
>>> share the same ID) and then we could just accept a universal 'id' along
>>> with 'child'.
>>
>> Maybe we can go this way even without explicit restriction (which
>> should some how go through deprecation period, etc), but simply look
>> for the id among nodes, devices and exports and if found more than one
>> parent - fail.
>>
>> And we document, that id should not be ambiguous, should not match more
>> than one parent object. So, those who want to use new command will care
>> to make unique ids.
> 
> I don't think such a state is very pretty, but it would be okay for me
> as an intermediate state while we go through a deprecation period to
> restrict IDs accordingly.
> 
> So we could start with blockdev-replace returning an error on ambiguous
> IDs and at the same time deprecate them, and only later we would make
> creating nodes/devices/exports with the same ID an error.
> 

Hmm, the only question remains, is what/how to deprecate exactly?

We want to deprecate user's possibility to set intersecting
IDs for exports / devices / block-nodes? I think, we don't
have a QAPI-native way to deprecate such thing..

May be, add new "uuid" parameter, and deprecate its absence (I doubt that we
can do such deprecation too). And deprecate old IDs? But we can't deprecate
QOM path for this..

Hmm, or move to QOM paths for block-nodes and exports? And deprecate export names
and node names?

Or we can just deprecate intersecting IDs in documentation and start to print warning,
when user make intersecting IDs? But nobody reads warnings..

Is there a proper way to deprecate such things?

-- 
Best regards,
Vladimir

Am 18.11.2025 um 08:37 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 17.11.25 13:49, Kevin Wolf wrote:
> > Hi Vladimir,
> > 
> > I remembered this series and wanted to check what the current status is,
> > because I seemed to remember that the next step was that you would send
> > a new version. But reading it again, you're probably waiting for more
> > input? Let's try to get this finished.
> 
> I think yes, I was waiting, but then switched to other tasks.
> 
> > 
> > Am 02.04.2025 um 15:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > On 18.10.24 16:59, Kevin Wolf wrote:
> > > > If we want to get rid of the union, I think the best course of action
> > > > would unifying the namespaces (so that nodes, exports and devices can't
> > > > share the same ID) and then we could just accept a universal 'id' along
> > > > with 'child'.
> > > 
> > > Maybe we can go this way even without explicit restriction (which
> > > should some how go through deprecation period, etc), but simply look
> > > for the id among nodes, devices and exports and if found more than one
> > > parent - fail.
> > > 
> > > And we document, that id should not be ambiguous, should not match more
> > > than one parent object. So, those who want to use new command will care
> > > to make unique ids.
> > 
> > I don't think such a state is very pretty, but it would be okay for me
> > as an intermediate state while we go through a deprecation period to
> > restrict IDs accordingly.
> > 
> > So we could start with blockdev-replace returning an error on ambiguous
> > IDs and at the same time deprecate them, and only later we would make
> > creating nodes/devices/exports with the same ID an error.
> > 
> 
> Hmm, the only question remains, is what/how to deprecate exactly?
> 
> We want to deprecate user's possibility to set intersecting
> IDs for exports / devices / block-nodes? I think, we don't
> have a QAPI-native way to deprecate such thing..

We don't have to be able to express every deprecation in the schema. If
it can be expressed, that's nice, but docs/about/deprecated.rst is the
important part.

> May be, add new "uuid" parameter, and deprecate its absence (I doubt
> that we can do such deprecation too). And deprecate old IDs? But we
> can't deprecate QOM path for this..

I don't think renaming options is necessary.

> Hmm, or move to QOM paths for block-nodes and exports? And deprecate
> export names and node names?

That would only make sense if we converted the block layer to a QOM
class hierarchy, which would be a project of its own.

> Or we can just deprecate intersecting IDs in documentation and start
> to print warning, when user make intersecting IDs? But nobody reads
> warnings..
> 
> Is there a proper way to deprecate such things?

The latter is what I would suggest. docs/about/deprecated.rst and
printing warnings. I think libvirt already keeps all IDs distinct
anyway, so for a large part of users nothing will change.

Kevin

On 18.11.25 12:47, Kevin Wolf wrote:
> Am 18.11.2025 um 08:37 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> On 17.11.25 13:49, Kevin Wolf wrote:
>>> Hi Vladimir,
>>>
>>> I remembered this series and wanted to check what the current status is,
>>> because I seemed to remember that the next step was that you would send
>>> a new version. But reading it again, you're probably waiting for more
>>> input? Let's try to get this finished.
>>
>> I think yes, I was waiting, but then switched to other tasks.
>>
>>>
>>> Am 02.04.2025 um 15:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> On 18.10.24 16:59, Kevin Wolf wrote:
>>>>> If we want to get rid of the union, I think the best course of action
>>>>> would unifying the namespaces (so that nodes, exports and devices can't
>>>>> share the same ID) and then we could just accept a universal 'id' along
>>>>> with 'child'.
>>>>
>>>> Maybe we can go this way even without explicit restriction (which
>>>> should some how go through deprecation period, etc), but simply look
>>>> for the id among nodes, devices and exports and if found more than one
>>>> parent - fail.
>>>>
>>>> And we document, that id should not be ambiguous, should not match more
>>>> than one parent object. So, those who want to use new command will care
>>>> to make unique ids.
>>>
>>> I don't think such a state is very pretty, but it would be okay for me
>>> as an intermediate state while we go through a deprecation period to
>>> restrict IDs accordingly.
>>>
>>> So we could start with blockdev-replace returning an error on ambiguous
>>> IDs and at the same time deprecate them, and only later we would make
>>> creating nodes/devices/exports with the same ID an error.
>>>
>>
>> Hmm, the only question remains, is what/how to deprecate exactly?
>>
>> We want to deprecate user's possibility to set intersecting
>> IDs for exports / devices / block-nodes? I think, we don't
>> have a QAPI-native way to deprecate such thing..
> 
> We don't have to be able to express every deprecation in the schema. If
> it can be expressed, that's nice, but docs/about/deprecated.rst is the
> important part.
> 
>> May be, add new "uuid" parameter, and deprecate its absence (I doubt
>> that we can do such deprecation too). And deprecate old IDs? But we
>> can't deprecate QOM path for this..
> 
> I don't think renaming options is necessary.
> 
>> Hmm, or move to QOM paths for block-nodes and exports? And deprecate
>> export names and node names?
> 
> That would only make sense if we converted the block layer to a QOM
> class hierarchy, which would be a project of its own.
> 
>> Or we can just deprecate intersecting IDs in documentation and start
>> to print warning, when user make intersecting IDs? But nobody reads
>> warnings..
>>
>> Is there a proper way to deprecate such things?
> 
> The latter is what I would suggest. docs/about/deprecated.rst and
> printing warnings. I think libvirt already keeps all IDs distinct
> anyway, so for a large part of users nothing will change.
> 

OK. Now the ball is definitely on my side, next step is v10)

-- 
Best regards,
Vladimir

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Add a command that can replace bs in following BdrvChild structures:
>
>  - qdev blk root child
>  - block-export blk root child
>  - any child of BlockDriverState selected by child-name
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  blockdev.c             | 56 +++++++++++++++++++++++++++
>  qapi/block-core.json   | 88 ++++++++++++++++++++++++++++++++++++++++++
>  stubs/blk-by-qdev-id.c | 13 +++++++
>  stubs/meson.build      |  1 +
>  4 files changed, 158 insertions(+)
>  create mode 100644 stubs/blk-by-qdev-id.c
>
> diff --git a/blockdev.c b/blockdev.c
> index ba7e90b06e..2190467022 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3559,6 +3559,62 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
>      bdrv_try_change_aio_context(bs, new_context, NULL, errp);
>  }
>  
> +void qmp_blockdev_replace(BlockdevReplace *repl, Error **errp)
> +{
> +    BdrvChild *child = NULL;
> +    BlockDriverState *new_child_bs;
> +
> +    if (repl->parent_type == BLOCK_PARENT_TYPE_DRIVER) {
> +        BlockDriverState *parent_bs;
> +
> +        parent_bs = bdrv_find_node(repl->u.driver.node_name);
> +        if (!parent_bs) {
> +            error_setg(errp, "Block driver node with node-name '%s' not "
> +                       "found", repl->u.driver.node_name);
> +            return;
> +        }
> +
> +        child = bdrv_find_child(parent_bs, repl->u.driver.child);
> +        if (!child) {
> +            error_setg(errp, "Block driver node '%s' doesn't have child "
> +                       "named '%s'", repl->u.driver.node_name,
> +                       repl->u.driver.child);
> +            return;
> +        }
> +    } else {
> +        /* Other types are similar, they work through blk */
> +        BlockBackend *blk;
> +        bool is_qdev = repl->parent_type == BLOCK_PARENT_TYPE_QDEV;
> +        const char *id =
> +            is_qdev ? repl->u.qdev.qdev_id : repl->u.export.export_id;
> +
> +        assert(is_qdev || repl->parent_type == BLOCK_PARENT_TYPE_EXPORT);
> +
> +        blk = is_qdev ? blk_by_qdev_id(id, errp) : blk_by_export_id(id, errp);

blk_by_export_id() finds export @exp, and returns the associated block
backend exp->blk.  Fine.

blk_by_qdev_id() finds the device, and then searches @block_backends for
a blk with blk->dev == blk.  If a device has more than one block
backend, you get the one first in @block_backends.  I figure that's the
one created first.

Interface issue: when a device has multiple block backends, only one of
them can be replaced, and which one is kind of random.

Do such devices exist?

If no, could they exist?

If yes, what should we do about it now?

> +        if (!blk) {
> +            return;
> +        }
> +
> +        child = blk_root(blk);
> +        if (!child) {
> +            error_setg(errp, "%s '%s' is empty, nothing to replace",
> +                       is_qdev ? "Device" : "Export", id);
> +            return;
> +        }
> +    }
> +
> +    assert(child);
> +    assert(child->bs);
> +
> +    new_child_bs = bdrv_find_node(repl->new_child);
> +    if (!new_child_bs) {
> +        error_setg(errp, "Node '%s' not found", repl->new_child);
> +        return;
> +    }
> +
> +    bdrv_replace_child_bs(child, new_child_bs, errp);
> +}
> +
>  QemuOptsList qemu_common_drive_opts = {
>      .name = "drive",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index df5e07debd..0a6f08a6e0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6148,3 +6148,91 @@
>  ##
>  { 'struct': 'DummyBlockCoreForceArrays',
>    'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> +
> +##
> +# @BlockParentType:
> +#
> +# @qdev: block device, such as created by device_add, and denoted by
> +#     qdev-id
> +#
> +# @driver: block driver node, such as created by blockdev-add, and
> +#     denoted by node-name

node-name and child?

> +#
> +# @export: block export, such created by block-export-add, and
> +#     denoted by export-id
> +#
> +# Since 9.1
> +##

I'm kind of unhappy with this doc comment.  I feel makes sense only in
the context of its use.  Let me try to avoid that:

   # @driver: the parent is a block node, the child is one of its
   #     children.
   #
   # @export: the parent is a block export, the child is its block
   #     backend.
   #
   # @qdev: the parent is a device, the child is its block backend.

> +{ 'enum': 'BlockParentType',
> +  'data': ['qdev', 'driver', 'export'] }

If you take my comment, change the order here as well.

> +
> +##
> +# @BdrvChildRefQdev:
> +#
> +# @qdev-id: the device's ID or QOM path
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefQdev',
> +  'data': { 'qdev-id': 'str' } }
> +
> +##
> +# @BdrvChildRefExport:
> +#
> +# @export-id: block export identifier

block-export.json calls this "block export id" in some places, and
"block export identifier" in others.  *Sigh*

Nothing to see here, move on!

> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefExport',
> +  'data': { 'export-id': 'str' } }
> +
> +##
> +# @BdrvChildRefDriver:
> +#
> +# @node-name: the node name of the parent block node
> +#
> +# @child: name of the child to be replaced, like "file" or "backing"
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefDriver',
> +  'data': { 'node-name': 'str', 'child': 'str' } }
> +
> +##
> +# @BlockdevReplace:
> +#
> +# @parent-type: type of the parent, which child is to be replaced

Suggest to scratch ", which child ..."

> +#
> +# @new-child: new child for replacement

Suggest "the new child".

> +#
> +# Since 9.1
> +##
> +{ 'union': 'BlockdevReplace',
> +  'base': {
> +      'parent-type': 'BlockParentType',
> +      'new-child': 'str'
> +  },
> +  'discriminator': 'parent-type',
> +  'data': {
> +      'qdev': 'BdrvChildRefQdev',
> +      'export': 'BdrvChildRefExport',
> +      'driver': 'BdrvChildRefDriver'
> +  } }
> +
> +##
> +# @blockdev-replace:
> +#
> +# Replace a block-node associated with device (selected by
> +# @qdev-id) or with block-export (selected by @export-id) or
> +# any child of block-node (selected by @node-name and @child)
> +# with @new-child block-node.

s/block-node/block node/ for consistency with existing usage.

Likewise, s/block-export/block export/.

> +#
> +# Features:
> +#
> +# @unstable: This command is experimental.
> +#
> +# Since 9.1
> +##
> +{ 'command': 'blockdev-replace', 'boxed': true,
> +  'features': [ 'unstable' ],
> +  'data': 'BlockdevReplace' }
> diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
> new file mode 100644
> index 0000000000..5ec9f755ee
> --- /dev/null
> +++ b/stubs/blk-by-qdev-id.c
> @@ -0,0 +1,13 @@
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "sysemu/block-backend.h"
> +
> +BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
> +{
> +    /*
> +     * We expect this when blockdev-change is called with parent-type=qdev,
> +     * but qdev-monitor is not linked in. So no blk_ API is not available.
> +     */

The last sentence is confusing.

> +    error_setg(errp, "Parameter 'parent-type' does not accept value 'qdev'");

I suggested this message.  I must have suffered from tunnel vision then.

The error message is good when the caller is qmp_blockdev_replace().
Then parameter @parent-type exists, and parameter value "qdev" cannot
work for any value of parameter "qdev-id" (which is @id here).

There are several more callers.  They don't use the stub now (or else
they wouldn't link before this patch).  But future callers may well use
it, and then the error message will likely be misleading.

v8 had

       error_setg(errp, "blk '%s' not found", id);

instead.  No good, because @id is not a "blk" (whatever that may be),
it's a qdev ID.  You offered "devices are not supported".  Less than
ideal, since it doesn't point to the argument that's causing the error,
but I figure it's the best we can do without refactoring.  Maybe
"can't select block backend by device ID".  Up to you.

> +    return NULL;
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 772a3e817d..068998c1a5 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -15,6 +15,7 @@ if have_block
>    stub_ss.add(files('bdrv-next-monitor-owned.c'))
>    stub_ss.add(files('blk-commit-all.c'))
>    stub_ss.add(files('blk-exp-close-all.c'))
> +  stub_ss.add(files('blk-by-qdev-id.c'))
>    stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
>    stub_ss.add(files('change-state-handler.c'))
>    stub_ss.add(files('get-vm-name.c'))

On 18.07.24 15:00, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> Add a command that can replace bs in following BdrvChild structures:
>>
>>   - qdev blk root child
>>   - block-export blk root child
>>   - any child of BlockDriverState selected by child-name
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   blockdev.c             | 56 +++++++++++++++++++++++++++
>>   qapi/block-core.json   | 88 ++++++++++++++++++++++++++++++++++++++++++
>>   stubs/blk-by-qdev-id.c | 13 +++++++
>>   stubs/meson.build      |  1 +
>>   4 files changed, 158 insertions(+)
>>   create mode 100644 stubs/blk-by-qdev-id.c
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index ba7e90b06e..2190467022 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3559,6 +3559,62 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
>>       bdrv_try_change_aio_context(bs, new_context, NULL, errp);
>>   }
>>   
>> +void qmp_blockdev_replace(BlockdevReplace *repl, Error **errp)
>> +{
>> +    BdrvChild *child = NULL;
>> +    BlockDriverState *new_child_bs;
>> +
>> +    if (repl->parent_type == BLOCK_PARENT_TYPE_DRIVER) {
>> +        BlockDriverState *parent_bs;
>> +
>> +        parent_bs = bdrv_find_node(repl->u.driver.node_name);
>> +        if (!parent_bs) {
>> +            error_setg(errp, "Block driver node with node-name '%s' not "
>> +                       "found", repl->u.driver.node_name);
>> +            return;
>> +        }
>> +
>> +        child = bdrv_find_child(parent_bs, repl->u.driver.child);
>> +        if (!child) {
>> +            error_setg(errp, "Block driver node '%s' doesn't have child "
>> +                       "named '%s'", repl->u.driver.node_name,
>> +                       repl->u.driver.child);
>> +            return;
>> +        }
>> +    } else {
>> +        /* Other types are similar, they work through blk */
>> +        BlockBackend *blk;
>> +        bool is_qdev = repl->parent_type == BLOCK_PARENT_TYPE_QDEV;
>> +        const char *id =
>> +            is_qdev ? repl->u.qdev.qdev_id : repl->u.export.export_id;
>> +
>> +        assert(is_qdev || repl->parent_type == BLOCK_PARENT_TYPE_EXPORT);
>> +
>> +        blk = is_qdev ? blk_by_qdev_id(id, errp) : blk_by_export_id(id, errp);
> 
> blk_by_export_id() finds export @exp, and returns the associated block
> backend exp->blk.  Fine.
> 
> blk_by_qdev_id() finds the device, and then searches @block_backends for
> a blk with blk->dev == blk.  If a device has more than one block
> backend, you get the one first in @block_backends.  I figure that's the
> one created first.
> 
> Interface issue: when a device has multiple block backends, only one of
> them can be replaced, and which one is kind of random.
> 
> Do such devices exist?

Hmm.. I expect, they don't. If there is a problem, it's pre-existing, all callers of qmp_get_blk are affected. But honestly, I don't know.

> 
> If no, could they exist?
> 
> If yes, what should we do about it now?

Maybe, continue the loop in blk_by_dev() up the the end, and if two blk found for the device, return an error? Or even abort?

And add check to blk_attach_dev(), that we don't attach same device to several blks.

> 
>> +        if (!blk) {
>> +            return;
>> +        }
>> +
>> +        child = blk_root(blk);
>> +        if (!child) {
>> +            error_setg(errp, "%s '%s' is empty, nothing to replace",
>> +                       is_qdev ? "Device" : "Export", id);
>> +            return;
>> +        }
>> +    }
>> +
>> +    assert(child);
>> +    assert(child->bs);
>> +
>> +    new_child_bs = bdrv_find_node(repl->new_child);
>> +    if (!new_child_bs) {
>> +        error_setg(errp, "Node '%s' not found", repl->new_child);
>> +        return;
>> +    }
>> +
>> +    bdrv_replace_child_bs(child, new_child_bs, errp);
>> +}
>> +
>>   QemuOptsList qemu_common_drive_opts = {
>>       .name = "drive",
>>       .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index df5e07debd..0a6f08a6e0 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -6148,3 +6148,91 @@
>>   ##
>>   { 'struct': 'DummyBlockCoreForceArrays',
>>     'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
>> +
>> +##
>> +# @BlockParentType:
>> +#
>> +# @qdev: block device, such as created by device_add, and denoted by
>> +#     qdev-id
>> +#
>> +# @driver: block driver node, such as created by blockdev-add, and
>> +#     denoted by node-name
> 
> node-name and child?

Hmm. I'd say that parent is denoted only by node-name, as parent is node itself (the fact that we have separate BdrvChild structure as a parent I'd keep as internal realization). But parent may have several children, and concrete child is denoted by @child.

> 
>> +#
>> +# @export: block export, such created by block-export-add, and
>> +#     denoted by export-id
>> +#
>> +# Since 9.1
>> +##
> 
> I'm kind of unhappy with this doc comment.  I feel makes sense only in
> the context of its use.  Let me try to avoid that:
> 
>     # @driver: the parent is a block node, the child is one of its
>     #     children.
>     #
>     # @export: the parent is a block export, the child is its block
>     #     backend.
>     #
>     # @qdev: the parent is a device, the child is its block backend.

Sounds good to me and leaves no questions)

> 
>> +{ 'enum': 'BlockParentType',
>> +  'data': ['qdev', 'driver', 'export'] }
> 
> If you take my comment, change the order here as well.
> 
>> +
>> +##
>> +# @BdrvChildRefQdev:
>> +#
>> +# @qdev-id: the device's ID or QOM path
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefQdev',
>> +  'data': { 'qdev-id': 'str' } }
>> +
>> +##
>> +# @BdrvChildRefExport:
>> +#
>> +# @export-id: block export identifier
> 
> block-export.json calls this "block export id" in some places, and
> "block export identifier" in others.  *Sigh*
> 
> Nothing to see here, move on!
> 
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefExport',
>> +  'data': { 'export-id': 'str' } }
>> +
>> +##
>> +# @BdrvChildRefDriver:
>> +#
>> +# @node-name: the node name of the parent block node
>> +#
>> +# @child: name of the child to be replaced, like "file" or "backing"
>> +#
>> +# Since 9.1
>> +##
>> +{ 'struct': 'BdrvChildRefDriver',
>> +  'data': { 'node-name': 'str', 'child': 'str' } }
>> +
>> +##
>> +# @BlockdevReplace:
>> +#
>> +# @parent-type: type of the parent, which child is to be replaced
> 
> Suggest to scratch ", which child ..."
> 
>> +#
>> +# @new-child: new child for replacement
> 
> Suggest "the new child".
> 
>> +#
>> +# Since 9.1
>> +##
>> +{ 'union': 'BlockdevReplace',
>> +  'base': {
>> +      'parent-type': 'BlockParentType',
>> +      'new-child': 'str'
>> +  },
>> +  'discriminator': 'parent-type',
>> +  'data': {
>> +      'qdev': 'BdrvChildRefQdev',
>> +      'export': 'BdrvChildRefExport',
>> +      'driver': 'BdrvChildRefDriver'
>> +  } }
>> +
>> +##
>> +# @blockdev-replace:
>> +#
>> +# Replace a block-node associated with device (selected by
>> +# @qdev-id) or with block-export (selected by @export-id) or
>> +# any child of block-node (selected by @node-name and @child)
>> +# with @new-child block-node.
> 
> s/block-node/block node/ for consistency with existing usage.
> 
> Likewise, s/block-export/block export/.
> 
>> +#
>> +# Features:
>> +#
>> +# @unstable: This command is experimental.
>> +#
>> +# Since 9.1
>> +##
>> +{ 'command': 'blockdev-replace', 'boxed': true,
>> +  'features': [ 'unstable' ],
>> +  'data': 'BlockdevReplace' }
>> diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
>> new file mode 100644
>> index 0000000000..5ec9f755ee
>> --- /dev/null
>> +++ b/stubs/blk-by-qdev-id.c
>> @@ -0,0 +1,13 @@
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "sysemu/block-backend.h"
>> +
>> +BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
>> +{
>> +    /*
>> +     * We expect this when blockdev-change is called with parent-type=qdev,
>> +     * but qdev-monitor is not linked in. So no blk_ API is not available.
>> +     */
> 
> The last sentence is confusing.
> 
>> +    error_setg(errp, "Parameter 'parent-type' does not accept value 'qdev'");
> 
> I suggested this message.  I must have suffered from tunnel vision then.
> 
> The error message is good when the caller is qmp_blockdev_replace().
> Then parameter @parent-type exists, and parameter value "qdev" cannot
> work for any value of parameter "qdev-id" (which is @id here).
> 
> There are several more callers.  They don't use the stub now (or else
> they wouldn't link before this patch).  But future callers may well use
> it, and then the error message will likely be misleading.
> 
> v8 had
> 
>         error_setg(errp, "blk '%s' not found", id);
> 
> instead.  No good, because @id is not a "blk" (whatever that may be),
> it's a qdev ID.  You offered "devices are not supported".  Less than
> ideal, since it doesn't point to the argument that's causing the error,
> but I figure it's the best we can do without refactoring.  Maybe
> "can't select block backend by device ID".  Up to you.

"selecting block backend by device ID is not supported" may be?


> 
>> +    return NULL;
>> +}
>> diff --git a/stubs/meson.build b/stubs/meson.build
>> index 772a3e817d..068998c1a5 100644
>> --- a/stubs/meson.build
>> +++ b/stubs/meson.build
>> @@ -15,6 +15,7 @@ if have_block
>>     stub_ss.add(files('bdrv-next-monitor-owned.c'))
>>     stub_ss.add(files('blk-commit-all.c'))
>>     stub_ss.add(files('blk-exp-close-all.c'))
>> +  stub_ss.add(files('blk-by-qdev-id.c'))
>>     stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
>>     stub_ss.add(files('change-state-handler.c'))
>>     stub_ss.add(files('get-vm-name.c'))
> 

-- 
Best regards,
Vladimir