On 22.01.26 17:01, Vladimir Sementsov-Ogievskiy wrote:
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/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;
+        }
+

Node names and block export IDs need to conform to id_wellformed() (or id_generate()), which does not allow slashes. So the paths should never intersect with node names and block export IDs by design.

With these checks removed (or not, if you think we need them, they don’t hurt either), and with the spelling in patch 4 fixed as per Markus’s suggestions, for the series:

Reviewed-by: Hanna Czenczek <[email protected]>

(I know the discussion around how unique IDs should be is ongoing, but FWIW, having unique IDs across the block layer is good enough for me as a first step. Well, I guess, including block job IDs might be nice, too.)

Reply via email to