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. Second, QOMifying block nodes feels ambitious to me. Please discuss it with block maintainers Kevin and Hanna. > And Markus, could you please take a look at patch 13? > I will prepare the V6 after your comments. Done. It may need an update for changes made here, though.
