| Message ID | 20231017184444.932733-5-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | blockdev-replace | expand |
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 89751d81f2..cf92e0df8b 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -6054,3 +6054,86 @@ > ## > { '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 8.2 > +## > +{ 'enum': 'BlockParentType', > + 'data': ['qdev', 'driver', 'export'] } > + > +## > +# @BdrvChildRefQdev: > +# > +# @qdev-id: the device's ID or QOM path > +# > +# Since 8.2 > +## > +{ 'struct': 'BdrvChildRefQdev', > + 'data': { 'qdev-id': 'str' } } > + > +## > +# @BdrvChildRefExport: > +# > +# @export-id: block export identifier > +# > +# Since 8.2 > +## > +{ '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 8.2 > +## > +{ '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 8.2 > +## > +{ 'union': 'BlockdevReplace', > + 'base': { > + 'parent-type': 'BlockParentType', > + 'new-child': 'str' > + }, > + 'discriminator': 'parent-type', > + 'data': { > + 'qdev': 'BdrvChildRefQdev', > + 'export': 'BdrvChildRefExport', > + 'driver': 'BdrvChildRefDriver' > + } } > + > +## > +# @x-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. > +# > +# Since 8.2 > +## > +{ 'command': 'x-blockdev-replace', 'boxed': true, > + 'data': 'BlockdevReplace' } > diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c > new file mode 100644 > index 0000000000..0e751ce4f7 > --- /dev/null > +++ b/stubs/blk-by-qdev-id.c > @@ -0,0 +1,9 @@ > +#include "qemu/osdep.h" > +#include "qapi/error.h" > +#include "sysemu/block-backend.h" > + > +BlockBackend *blk_by_qdev_id(const char *id, Error **errp) > +{ > + error_setg(errp, "blk '%s' not found", id); Is this expected to happen? > + return NULL; > +} [...] QAPI schema Acked-by: Markus Armbruster <armbru@redhat.com>
On 18.10.23 13:45, 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> [..] >> --- /dev/null >> +++ b/stubs/blk-by-qdev-id.c >> @@ -0,0 +1,9 @@ >> +#include "qemu/osdep.h" >> +#include "qapi/error.h" >> +#include "sysemu/block-backend.h" >> + >> +BlockBackend *blk_by_qdev_id(const char *id, Error **errp) >> +{ >> + error_setg(errp, "blk '%s' not found", id); > > Is this expected to happen? Yes, if call the command from qemu-storage-daemon, where qdev-monitor is not linked in. Maybe, better message would be "devices are not supported" Maybe, that possible to use some 'if': notation in qapi, to not include support for qdev into the new command, when it compiled into qemu-storage-daemon? Seems that would not be simple, as we also need to split compilation of the command somehow, now it compiled once both for qemu and qemu tools.. > >> + return NULL; >> +} > > [...] > > QAPI schema > Acked-by: Markus Armbruster <armbru@redhat.com> >
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > On 18.10.23 13:45, 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> > > [..] > >>> --- /dev/null >>> +++ b/stubs/blk-by-qdev-id.c >>> @@ -0,0 +1,9 @@ >>> +#include "qemu/osdep.h" >>> +#include "qapi/error.h" >>> +#include "sysemu/block-backend.h" >>> + >>> +BlockBackend *blk_by_qdev_id(const char *id, Error **errp) >>> +{ >>> + error_setg(errp, "blk '%s' not found", id); >> >> Is this expected to happen? > > Yes, if call the command from qemu-storage-daemon, where qdev-monitor is not linked in. It happens when you try to x-blockdev-replace with "parent-type": "qdev". Correct? > Maybe, better message would be > > "devices are not supported" Best to spell out which argument is the problem. Stupidest solution that could possibly work: "Parameter 'parent-type' does not accept value 'qdev'" This is exactly what we'd get if we compiled out the parts that don't make sense for qemu-storage-daemon. > Maybe, that possible to use some 'if': notation in qapi, to not include support for qdev into the new command, when it compiled into qemu-storage-daemon? Seems that would not be simple, as we also need to split compilation of the command somehow, now it compiled once both for qemu and qemu tools.. That's precisely the problem. Our reuse of parts of qemu-system-FOO's QAPI schema for qemu-storage-daemon isn't pretty, but has worked for us so far. >>> + return NULL; >>> +} >> [...] >> QAPI schema >> Acked-by: Markus Armbruster <armbru@redhat.com> >>
diff --git a/blockdev.c b/blockdev.c index 49d0aab356..255f4971ec 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3700,6 +3700,62 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread, aio_context_release(old_context); } +void qmp_x_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 89751d81f2..cf92e0df8b 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -6054,3 +6054,86 @@ ## { '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 8.2 +## +{ 'enum': 'BlockParentType', + 'data': ['qdev', 'driver', 'export'] } + +## +# @BdrvChildRefQdev: +# +# @qdev-id: the device's ID or QOM path +# +# Since 8.2 +## +{ 'struct': 'BdrvChildRefQdev', + 'data': { 'qdev-id': 'str' } } + +## +# @BdrvChildRefExport: +# +# @export-id: block export identifier +# +# Since 8.2 +## +{ '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 8.2 +## +{ '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 8.2 +## +{ 'union': 'BlockdevReplace', + 'base': { + 'parent-type': 'BlockParentType', + 'new-child': 'str' + }, + 'discriminator': 'parent-type', + 'data': { + 'qdev': 'BdrvChildRefQdev', + 'export': 'BdrvChildRefExport', + 'driver': 'BdrvChildRefDriver' + } } + +## +# @x-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. +# +# Since 8.2 +## +{ 'command': 'x-blockdev-replace', 'boxed': true, + 'data': 'BlockdevReplace' } diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c new file mode 100644 index 0000000000..0e751ce4f7 --- /dev/null +++ b/stubs/blk-by-qdev-id.c @@ -0,0 +1,9 @@ +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "sysemu/block-backend.h" + +BlockBackend *blk_by_qdev_id(const char *id, Error **errp) +{ + error_setg(errp, "blk '%s' not found", id); + return NULL; +} diff --git a/stubs/meson.build b/stubs/meson.build index cde44972bf..6ff8db71f9 100644 --- a/stubs/meson.build +++ b/stubs/meson.build @@ -1,6 +1,7 @@ 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('cmos.c'))
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 | 83 ++++++++++++++++++++++++++++++++++++++++++ stubs/blk-by-qdev-id.c | 9 +++++ stubs/meson.build | 1 + 4 files changed, 149 insertions(+) create mode 100644 stubs/blk-by-qdev-id.c