Zhang Chen <[email protected]> writes:
> On Wed, Mar 18, 2026 at 5:13 PM Stefan Hajnoczi <[email protected]> wrote:
>>
>> On Wed, Mar 18, 2026 at 2:20 PM Markus Armbruster <[email protected]> wrote:
>> >
>> > Zhang Chen <[email protected]> writes:
>> >
>> > > On Thu, Mar 12, 2026 at 5:16 PM Markus Armbruster <[email protected]>
>> > > wrote:
>> > >>
>> > >> 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.
>> > >>
>> > >
>> > > I agree Markus's suggestion.
>> > > Compare with standard QOM path:
>> > > /machine/peripheral/blk0/virtio-backend
>> > >
>> > > I will try to implement block nodes path like this:
>> > > /machine/blockdriverstate/node-name
>> >
>> > I gather you'd like to try creating QOM objects for block nodes.
>> >
>> > First, this should not go into /machine. We already have /chardevs and
>> > /audiodevs, which suggests something like /blockdevs or /block-nodes.
>>
>> It may be a lot of work. An alternative I suggested was a holder enum
>> instead of a string so that QOM paths and block node names can be
>> separated with no chance of collisions. That approach is more
>> straightforward to implement and the main drawback I see is that the
>> enum would become superfluous if block nodes become QOM objects in the
>> future. That's not terrible.
>>
>> Stefan
>
> Ping....
>
> Hi Markus and Stefan,
> As Markus's comments, I prefer the /blockdevs or /block-nodes is more simple,
> And use a holder enum instead of a string will make it more difficult
> for users to understand.
> Please let us make a decision.
I'm not sure what exactly you need from me.
I assume you need advice on how to represent a reference to a "holder"
in the QAPI schema, where the holder can either be a QOM object or a
block node.
If everthing was a QOM object, we could simply use a QOM path,
i.e. 'str'.
Sadly, block nodes are not QOM objects. Unless you want to solve the
(non-trivial) problem of making them QOM objects first, you need
something else.
My recommendation for something else would be a union:
{ 'union': 'IoThreadHolder',
'base': { 'type': 'IoThreadHolderKind' },
'discriminator': 'type',
'data': {
'block-node': ...,
'qom-object': ... } }
Stefan, do you agree?
Hope this helps!
> Hi Kevin and Hanna,
> Considering the possibility of blockdevs being qom in the future, we
> need your idea here, any comments for the series?
>
> Thanks
> Chen
