Context... we're talking about this command:
##
# @x-blockdev-set-iothread:
#
# Move @node and its children into the @iothread. If @iothread is
# null then move @node and its children into the main loop.
#
# The node must not be attached to a BlockBackend.
#
# @node-name: the name of the block driver node
#
# @iothread: the name of the IOThread object or null for the main loop
#
# @force: true if the node and its children should be moved when a
# BlockBackend is already attached
#
# Features:
#
# @unstable: This command is experimental and intended for test cases
# that need control over IOThreads only.
#
# Since: 2.12
#
# .. qmp-example::
# :title: Move a node into an IOThread
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
# "iothread": "iothread0" } }
# <- { "return": {} }
#
# .. qmp-example::
# :title: Move a node into the main loop
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
# "iothread": null } }
# <- { "return": {} }
##
{ 'command': 'x-blockdev-set-iothread',
'data' : { 'node-name': 'str',
'iothread': 'StrOrNull',
'*force': 'bool' },
'features': [ 'unstable' ],
'allow-preconfig': true }
Stefan Hajnoczi <[email protected]> writes:
> On Tue, Mar 10, 2026 at 06:02:54PM +0800, Zhang Chen wrote:
>> On Mon, Mar 9, 2026 at 4:15 PM Stefan Hajnoczi <[email protected]> wrote:
>> >
>> > On Thu, Mar 05, 2026 at 10:24:50PM +0800, Zhang Chen wrote:
>> > > Update the usage of "iothread_get_aio_context()".
>> > >
>> > > Signed-off-by: Zhang Chen <[email protected]>
>> > > ---
>> > > blockdev.c | 9 ++++++++-
>> > > 1 file changed, 8 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/blockdev.c b/blockdev.c
>> > > index 6e86c6262f..01ccf64b3f 100644
>> > > --- a/blockdev.c
>> > > +++ b/blockdev.c
>> > > @@ -3683,7 +3683,14 @@ void qmp_x_blockdev_set_iothread(const char
>> > > *node_name, StrOrNull *iothread,
>> > > goto out;
>> > > }
>> > >
>> > > - new_context = iothread_get_aio_context(obj);
>> > > + char *path = object_get_canonical_path(OBJECT(bs));
>> >
>> > CCing Kevin and Markus in case they have an opinion on this.
>> >
>> > BlockDriverState is not a QOM Object so using OBJECT(bs) is undefined
>> > behavior and may crash.
Yes.
>> > node_name is unique across block driver graph nodes and could be used.
>> > Unfortunately it's not connected to the QOM Object hierarchy.
Correct.
>> > Maybe it's
>> > best to build a holder name that is an invalid QOM path so there can be
>> > no collisions between QOM paths and block driver graph nodes.
I guess you're talking about the values that go into IOThreadInfo member
holders. From PATCH 13:
# @holders: The parameter is an array of QOM paths indicating how many
# active devices are currently associated with this iothread
# (e.g. virtio-blk). In hotplug scenarios, users can
# pre-allocate multiple iothread objects to serve as a persistent
# thread pool. When a device is hot-unplugged, the corresponding
# IOThread is released but remains available, allowing subsequent
# hot-plugged devices to attach to and reuse the existing thread.
# Returns empty if no devices are attached. (since 11.0)
#
I further guess you need it to refer to both QOM objects and block
nodes, and you worry about ambiguity.
Ambiguity indeed exists: a block node name can be a valid QOM path.
We could restrict QOM paths to absolute paths. These start with '/'.
If I remember correctly, node names cannot contain '/'.
Note that canonical paths (returned object_get_canonical_path()) are
absolute.
We ran into a similar design issue in review of Vladimir's "[PATCH v10
4/8] qapi: add blockdev-replace command" not too long ago:
Subject: Re: [PATCH v10 4/8] qapi: add blockdev-replace command
Date: Wed, 04 Feb 2026 13:26:35 +0100
Message-ID: <[email protected]>
There, the new command needs to refer to QOM object, block node, or
block export.
>> > g_autofree char *holder = g_strdup_printf("BlockDriverState %s",
>> > node_name);
>> >
>> > (A cleaner long-term solution would be making BlockDriverStates QOM
>> > Objects so they have a proper path.)
Yes, but that's a beefy project, isn't it?
>> If no other comments, it's OK for me. This issue like I mentioned in
>> patch 7 and 9.
>
> A thought about the QAPI interface:
>
> QAPI expresses as much information in the schema as possible, so I think
> the right approach would be a {'union': 'IOThreadHolder',
> 'discriminator': 'type', ...} that supports at least "qom" and
> "block-node". That way there are proper types to encode QOM Object paths
> vs block node-names. Let's avoid having a single string value that takes
> on different meaning depending on the type of holder.
This shifts the complexity from semantics to syntax.
Semantics: the member can have multiple meanings, and you have to
examine its value to decide which one applies. The member's
documentation should specify how to decide. Say something like "if the
value starts with '/', it's an absolute QOM path, else it's a block node
name".
Syntax: meaning is syntactically obvious. For instance, union of QOM
path and block node name.
Complex semantics tend to require more complex documentation.
Which choice is better depends on the specific case. I generally lean
towards syntax.
