On Mon, Mar 30, 2026 at 9:32 PM Stefan Hajnoczi <[email protected]> wrote: > > On Mon, Mar 30, 2026 at 11:02:03AM +0200, Markus Armbruster wrote: > > 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? > > Yes. >
Thank you for the comments, it's clear for next version. By the way, It's strange that I didn't receive Markus's previous email. Anything I missed? Thanks Chen > Stefan > > > > > 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 > >
