On Tue, Apr 28, 2026 at 3:26 PM Markus Armbruster <[email protected]> wrote: > > Zhang Chen <[email protected]> writes: > > > On Thu, Apr 23, 2026 at 8:44 PM Markus Armbruster <[email protected]> wrote: > >> Oh. > >> > >> PATCH 02 defines QAPI type IoThreadHolder as a union with two branches > >> for the two kinds of holders: one for block node holder, and one for QOM > >> object holder. Both branches use a string to identify the holder. > >> > >> The C code in PATCH 02 uses just a string to identify the holder. This > >> silently assumes that the strings for block nodes are distinct from the > >> strings for QOM objects. True as long as we only use absolute QOM > >> paths: these start with '/', and block nodes can't. > >> > >> This patch shows me there's actually a third kind: monitor. You use the > >> QOM object branch for it. That's confusing; a monitor is not a QOM > >> object. Moreover, you make yet another silent assumption: absolute QOM > >> paths do not start with "/monitor/". > >> > >> This is too much for me. > >> > >> Please use a separate IoThreadHolderKind value and IoThreadHolder branch > >> for each kind of holder. The kinds I've seen so far are block-node, QOM > >> object, monitor. > >> > >> If Daniel Berrangé's "[PATCH RFC 00/17] monitor: turn QMP and HMP into > >> QOM objects" gets merged, kind monitor can go away. > >> > >> Use of just a string to identify the holder requires strings for > >> different kinds to be distinct. The argument why they are must be > >> written down, simple, and likely to remain true. > >> > >> But I'd prefer to use IoThreadHolder instead of string. No assumptions > >> necessary then. > >> > >> If this IoThreadHolder arguments make the calls ugly or overly verbose, > >> consider thin wrappers for each kind, i.e. > >> > >> new_ctx = iothread_get_aio_context_block(iothread, bs); > >> > >> instead of > >> > >> holder_name = bdrv_get_node_name(bs); > >> new_ctx = iothread_get_aio_context(iothread, holder_name); > >> > >> and so forth. > >> > >> Questions? > > > > Thank you for the detailed explanation. > > You are right, in this patch I silent assumption: absolute QOM paths > > do not start with "/monitor/". > > Because the Daniel Berrangé's RFC patch and the monitor implementation > > is very special, > > it creates the iothread for itself. The iothread lifecycle is same > > with the monitor. > > It is not a general case like QOM and block node. > > Maybe no need for this case add a third kind here? > > I don't know. Or maybe I don't understand the question. >
If no other comments I will keep the assumption in the next version. At the same time, I will try to keep looking at Daniel Berrangé's RFC patch about monitor QOM. Thanks Chen
