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             | 68 ++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json   | 24 +++++++++++++++
 stubs/blk-by-qdev-id.c | 13 ++++++++
 stubs/meson.build      |  1 +
 4 files changed, 106 insertions(+)
 create mode 100644 stubs/blk-by-qdev-id.c

diff --git a/blockdev.c b/blockdev.c
index 2540f991b3..3082a5763c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3681,6 +3681,74 @@ out:
     bdrv_drain_all_end();
 }
 
+void qmp_blockdev_replace(const char *parent, const char *child,
+                          const char *new_child, Error **errp)
+{
+    BdrvChild *child_to_replace = NULL;
+    BlockDriverState *new_child_bs;
+    Error *dev_err = NULL, *exp_err = NULL;
+    BlockDriverState *parent_bs = bdrv_find_node(parent);
+    BlockBackend *dev_blk = blk_by_qdev_id(parent, &dev_err);
+    BlockBackend *exp_blk = blk_by_export_id(parent, &exp_err);
+    unsigned found = !!parent_bs + !!dev_blk + !!exp_blk;
+
+    if (found == 0) {
+        error_setg(errp, "Neither device, nor export, nor block-node exist"
+                   " with name '%s'. block-node: not found,"
+                   " device block-backend: %s, export block-backend: %s",
+                   parent, error_get_pretty(dev_err),
+                   error_get_pretty(exp_err));
+    }
+    error_free(dev_err);
+    error_free(exp_err);
+
+    if (found == 0) {
+        return;
+    }
+
+    if (found > 1) {
+        error_setg(errp, "Parent name '%s' is ambigous: block-node %s,"
+                   " device block-backend %s, export block-backend %s",
+                   parent, parent_bs ? "found" : "not found",
+                   dev_blk ? "found" : "not found",
+                   exp_blk ? "found" : "not found");
+        return;
+    }
+
+    if (parent_bs) {
+        child_to_replace = bdrv_find_child(parent_bs, child);
+        if (!child_to_replace) {
+            error_setg(errp, "Block driver node '%s' doesn't have child "
+                       "named '%s'", parent, child);
+            return;
+        }
+    } else {
+        BlockBackend *blk = dev_blk ?: exp_blk;
+
+        if (strcmp(child, "root")) {
+            error_setg(errp, "Devices and exports may have only 'root' child");
+        }
+
+        child_to_replace = blk_root(blk);
+        if (!child_to_replace) {
+            error_setg(errp, "%s '%s' is empty, nothing to replace",
+                       dev_blk ? "Device" : "Export", parent);
+            return;
+        }
+    }
+
+    assert(child_to_replace);
+    assert(child_to_replace->bs);
+
+    new_child_bs = bdrv_find_node(new_child);
+    if (!new_child_bs) {
+        error_setg(errp, "Node '%s' not found", new_child);
+        return;
+    }
+
+    bdrv_replace_child_bs(child_to_replace, 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 b82af74256..9cc7c3d140 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6334,3 +6334,27 @@
 ##
 { 'struct': 'DummyBlockCoreForceArrays',
   'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
+
+##
+# @blockdev-replace:
+#
+# Replace a block-node associated with device (@parent should be
+# QOM path, and @child should be "root") or with block-export (@parent
+# should be export name, and @child should be "root") or any child
+# of block-node (@parent should be node-name, and @child should be any
+# if its children names) with @new-child block-node.
+#
+# @parent: QOM path or block-export or node-name, which @child should
+#     be repalced.  If several matching parents exist, the command
+#     will fail
+#
+# @child: child to replace.  Must be "root" when parent is QOM path or
+#     block-export
+#
+# @new-child: node-name of the block-node, which should become a
+#    replacement for @child's current block-node
+#
+# Since 11.0
+##
+{ 'command': 'blockdev-replace',
+  'data': { 'parent': 'str', 'child': 'str', 'new-child': 'str' } }
diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
new file mode 100644
index 0000000000..66ead77f4d
--- /dev/null
+++ b/stubs/blk-by-qdev-id.c
@@ -0,0 +1,13 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "system/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 d3b551f4de..93a52d273e 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.52.0

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>

[...]

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b82af74256..9cc7c3d140 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6334,3 +6334,27 @@
>  ##
>  { 'struct': 'DummyBlockCoreForceArrays',
>    'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> +
> +##
> +# @blockdev-replace:
> +#
> +# Replace a block-node associated with device (@parent should be
> +# QOM path, and @child should be "root") or with block-export (@parent
> +# should be export name, and @child should be "root") or any child
> +# of block-node (@parent should be node-name, and @child should be any
> +# if its children names) with @new-child block-node.

of its

> +#
> +# @parent: QOM path or block-export or node-name, which @child should
> +#     be repalced.  If several matching parents exist, the command

replaced

> +#     will fail

End the sentence with a period, please.

> +#
> +# @child: child to replace.  Must be "root" when parent is QOM path or
> +#     block-export

Likewise.

> +#
> +# @new-child: node-name of the block-node, which should become a
> +#    replacement for @child's current block-node

Likewise.

Indent one more, please.

> +#
> +# Since 11.0
> +##

Let's see whether I understand...

@parent determines which of the three cases mentioned in the commit
message it ids:

* If @parent is a QOM path, case 1.

* If @parent is a block export name (@id in BlockExportOptions and
  BlockExportInfo), case 2.

* If @parent is a block node name (@node-name in BlockdevOptions and
  BlockDeviceInfo), case 3.

Correct?

Problem: this is ambiguous.  A @parent "foo" could in fact be any of the
three cases.

3. If a block node named "foo" exists, it's case 3.

2. If a block export named "foo" exists, it's also case 2.

1. If exactly one QOM object named "foo" exists, it's also case 1.  Why?
   "foo" is a syntactically valid partial QOM path.  Partial QOM paths
   are a convenience feature that is virtually unknown (and possibly
   ill-advised): you can omit leading path components as long as there's
   no ambiguity.

Peeking ahead in the series...  PATCH 8 appears to deprecate the
ambiguity between case 2. and 3.

I think we need to do better.

More questions...

In case 1 and 2, @child "should be root".  What happens when it's
something else?

In case 3, @child "should be any if its children names".  I figure the
command fails when it isn't.

> +{ 'command': 'blockdev-replace',
> +  'data': { 'parent': 'str', 'child': 'str', 'new-child': 'str' } }

[...]

On 04.02.26 15:26, 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>
> 
> [...]
> 
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index b82af74256..9cc7c3d140 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -6334,3 +6334,27 @@
>>   ##
>>   { 'struct': 'DummyBlockCoreForceArrays',
>>     'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
>> +
>> +##
>> +# @blockdev-replace:
>> +#
>> +# Replace a block-node associated with device (@parent should be
>> +# QOM path, and @child should be "root") or with block-export (@parent
>> +# should be export name, and @child should be "root") or any child
>> +# of block-node (@parent should be node-name, and @child should be any
>> +# if its children names) with @new-child block-node.
> 
> of its
> 
>> +#
>> +# @parent: QOM path or block-export or node-name, which @child should
>> +#     be repalced.  If several matching parents exist, the command
> 
> replaced
> 
>> +#     will fail
> 
> End the sentence with a period, please.
> 
>> +#
>> +# @child: child to replace.  Must be "root" when parent is QOM path or
>> +#     block-export
> 
> Likewise.
> 
>> +#
>> +# @new-child: node-name of the block-node, which should become a
>> +#    replacement for @child's current block-node
> 
> Likewise.
> 
> Indent one more, please.
> 
>> +#
>> +# Since 11.0
>> +##
> 
> Let's see whether I understand...
> 
> @parent determines which of the three cases mentioned in the commit
> message it ids:
> 
> * If @parent is a QOM path, case 1.
> 
> * If @parent is a block export name (@id in BlockExportOptions and
>    BlockExportInfo), case 2.
> 
> * If @parent is a block node name (@node-name in BlockdevOptions and
>    BlockDeviceInfo), case 3.
> 
> Correct?
> 
> Problem: this is ambiguous.  A @parent "foo" could in fact be any of the
> three cases.

Yes. And we return an error in case of any ambiguity.

> 
> 3. If a block node named "foo" exists, it's case 3.
> 
> 2. If a block export named "foo" exists, it's also case 2.
> 
> 1. If exactly one QOM object named "foo" exists, it's also case 1.  Why?
>     "foo" is a syntactically valid partial QOM path.  Partial QOM paths
>     are a convenience feature that is virtually unknown (and possibly
>     ill-advised): you can omit leading path components as long as there's
>     no ambiguity.
> 
> Peeking ahead in the series...  PATCH 8 appears to deprecate the
> ambiguity between case 2. and 3.
> 
> I think we need to do better.
> 
> More questions...
> 
> In case 1 and 2, @child "should be root".  What happens when it's
> something else?

Error returned. s/should/must/ ?

> 
> In case 3, @child "should be any if its children names".  I figure the
> command fails when it isn't.

And here.

> 
>> +{ 'command': 'blockdev-replace',
>> +  'data': { 'parent': 'str', 'child': 'str', 'new-child': 'str' } }
> 
> [...]
> 


-- 
Best regards,
Vladimir