Now we have blockdev-replace QMP command, which depend on a possibility to select any block parent (block node, block export, or qdev) by one unique name. The command fails, if name is ambiguous (i.e., match several parents of different types). In future we want to rid of this ambiguity.
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> --- v11: rework separate warn_s into one common function check_existing_parent_id(). block.c | 6 ++++++ block/export/export.c | 6 ++++++ blockdev.c | 21 +++++++++++++++++++++ docs/about/deprecated.rst | 10 ++++++++++ include/block/block-global-state.h | 2 ++ stubs/check-existing-parent-id.c | 6 ++++++ stubs/meson.build | 1 + system/qdev-monitor.c | 20 ++++++++++++++++++++ 8 files changed, 72 insertions(+) create mode 100644 stubs/check-existing-parent-id.c diff --git a/block.c b/block.c index 8254d57212..69674ad4ed 100644 --- a/block.c +++ b/block.c @@ -1618,6 +1618,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs, { char *gen_node_name = NULL; GLOBAL_STATE_CODE(); + Error *local_err; if (!node_name) { node_name = gen_node_name = id_generate(ID_BLOCK); @@ -1649,6 +1650,11 @@ static void bdrv_assign_node_name(BlockDriverState *bs, goto out; } + if (!check_existing_parent_id(node_name, &local_err)) { + error_report_err(local_err); + local_err = NULL; + } + /* copy node name into the bs and insert it into the graph list */ pstrcpy(bs->node_name, sizeof(bs->node_name), node_name); QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list); diff --git a/block/export/export.c b/block/export/export.c index 9169b43e13..174c86b4d2 100644 --- a/block/export/export.c +++ b/block/export/export.c @@ -96,6 +96,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) AioContext *ctx; uint64_t perm; int ret; + Error *local_err = NULL; GLOBAL_STATE_CODE(); @@ -108,6 +109,11 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) return NULL; } + if (!check_existing_parent_id(export->id, &local_err)) { + error_report_err(local_err); + local_err = NULL; + } + drv = blk_exp_find_driver(export->type); if (!drv) { error_setg(errp, "No driver found for the requested export type"); diff --git a/blockdev.c b/blockdev.c index 3082a5763c..643b2132c9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3681,6 +3681,27 @@ out: bdrv_drain_all_end(); } + +bool check_existing_parent_id(const char *id, Error **errp) +{ + if (bdrv_find_node(id)) { + error_setg(errp, "block node with id '%s' already exist", id); + return false; + } + + if (blk_by_qdev_id(id, NULL)) { + error_setg(errp, "block device with id '%s' already exist", id); + return false; + } + + if (blk_by_export_id(id, NULL)) { + error_setg(errp, "block export with id '%s' already exist", id); + return false; + } + + return true; +} + void qmp_blockdev_replace(const char *parent, const char *child, const char *new_child, Error **errp) { diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 88efa3aa80..18bb1eeafc 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -551,3 +551,13 @@ command documentation for details on the ``fdset`` usage. The ``zero-blocks`` capability was part of the block migration which doesn't exist anymore since it was removed in QEMU v9.1. + +Identifiers +----------- + +Possibility to intersect qdev ids/paths, block node names, and block +export names namespaces is deprecated. In future that would be +abandoned and all block exports, block nodes and devices will have +unique names. Now, reusing the name for another type of object (for +example, creating block-node with node-name equal to existing qdev +id) produce a warning. diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index ed89999f0f..5014324241 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -318,4 +318,6 @@ void bdrv_unregister_buf(BlockDriverState *bs, void *host, size_t size); void bdrv_cancel_in_flight(BlockDriverState *bs); +bool check_existing_parent_id(const char *id, Error **errp); + #endif /* BLOCK_GLOBAL_STATE_H */ diff --git a/stubs/check-existing-parent-id.c b/stubs/check-existing-parent-id.c new file mode 100644 index 0000000000..ef5ea3f26d --- /dev/null +++ b/stubs/check-existing-parent-id.c @@ -0,0 +1,6 @@ +#include "qemu/osdep.h" +#include "block/block-global-state.h" + +bool check_existing_parent_id(const char *id, Error **errp) { + return true; +} diff --git a/stubs/meson.build b/stubs/meson.build index 30536ec8fa..3bd8649bd1 100644 --- a/stubs/meson.build +++ b/stubs/meson.build @@ -16,6 +16,7 @@ if have_block 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('check-existing-parent-id.c')) stub_ss.add(files('blk-exp-find-by-blk.c')) stub_ss.add(files('blockdev-close-all-bdrv-states.c')) stub_ss.add(files('change-state-handler.c')) diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index be18902bb2..ca14135191 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -593,6 +593,7 @@ static BusState *qbus_find(const char *path, Error **errp) const char *qdev_set_id(DeviceState *dev, char *id, Error **errp) { ObjectProperty *prop; + Error *local_err = NULL; assert(!dev->id && !dev->realized); @@ -601,6 +602,18 @@ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp) * has no parent */ if (id) { + g_autofree char *fullpath = g_strdup_printf("/peripheral/%s", id); + + if (!check_existing_parent_id(id, &local_err)) { + error_report_err(local_err); + local_err = NULL; + } + + if (!check_existing_parent_id(fullpath, &local_err)) { + error_report_err(local_err); + local_err = NULL; + } + prop = object_property_try_add_child(qdev_get_peripheral(), id, OBJECT(dev), NULL); if (prop) { @@ -613,6 +626,13 @@ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp) } else { static int anon_count; gchar *name = g_strdup_printf("device[%d]", anon_count++); + g_autofree char *fullpath = g_strdup_printf("/peripheral-anon/%s", id); + + if (!check_existing_parent_id(fullpath, &local_err)) { + error_report_err(local_err); + local_err = NULL; + } + prop = object_property_add_child(qdev_get_peripheral_anon(), name, OBJECT(dev)); g_free(name); -- 2.52.0
