Kevin Wolf <[email protected]> writes: > Am 18.02.2026 um 07:25 hat Markus Armbruster geschrieben: >> Block layer maintainers' advice sought: >> >> Vladimir Sementsov-Ogievskiy <[email protected]> writes: >> >> > On 17.02.26 16:10, Markus Armbruster wrote: >> >> Vladimir Sementsov-Ogievskiy <[email protected]> writes: >> >> >> >>> On 14.02.26 11:04, Markus Armbruster wrote: >> >> [...] >> >> >>>> This interface sort of overloads @parent and @child, and overloading the >> >>>> former creates ambiguity that can make the command unusable. >> >>> >> >>> Is it a real problem if we do deprecate the thing, that leads to this >> >>> ambiguity? >> >>> >> >>> If it is, we may mark the command "unstable" during the deprecation >> >>> period. >> >> >> >> By itself, the proposed command is a trap for the unwary. >> >> >> >> Deprecating the usage that enables the trap makes the trap temporary. >> >> >> >> Warning on (deprecated) usage that enables the trap helps users avoid >> >> it. Human users, mostly. >> >> >> >> Marking the command "unstable" should ensure management application >> >> avoid the command, and thus the trap. >> >> >> >> All of the above together feels acceptable to me. Can we do better? >> >> Maybe, and that's discussed below. >> >> >> >>> Or even postpone its addition up to the end of this period. >> >> >> >> No trap, no problem :) >> >> >> >>>> @parent's set of valid values is the union of QOM path, block node name, >> >>>> block export ID with values occuring in more than one of them dropped. >> >>>> >> >>>> @child's set of valid values depends on @parent: child name when it's a >> >>>> QOM path, else "root". >> >>>> >> >>>> The obvious non-overloaded interface is a union of three branches: QOM, >> >>>> block node, block export. The QOM branch has variant members @qom-path >> >>>> and @child. The block node branch has variant member @node-name. The >> >>>> block export branch has variant member @id. >> >>>> >> >>>> This is a bit more verbose: you have to specify the union tag[*]. >> >>>> >> >>>> If we ever need to replace block nodes associated with additional >> >>>> things, we simply more branches. These branches can have arbitrary >> >>>> members. In your interface, we'd have to make do with @child, or maybe >> >>>> add optional arguments. >> >>>> >> >>>> Thoughts? Mind, this isn't a demand! I'm exploring the design space. >> >>> >> >>> Union-based solution was v9: >> >>> https://lore.kernel.org/qemu-devel/[email protected]/ >> >>> >> >>> And there was a discussion, that we may try a way without a union. And >> >>> that's >> >>> this v10. >> >> >> >> I see Kevin suggested to explore this approach. I certainly respect his >> >> judgement. >> >> >> >>> Hmm, but that had different field names for parents: qdev-id, export-id >> >>> and node-name. >> >>> >> >>> What you suggest is to keep one filed for parent - "parent", but also >> >>> add a "parent-type" >> >>> field. This way, we may deprecate and remove "parent-type" in future. >> >>> Or, we may >> >>> add it as deprecated now (together with deprecating non-unique IDs) >> >> >> >> We can't deprecate the union tag right away: it is *required*. >> >> >> >>>> [*] We could add a "may omit union tag when the tag value can be derived >> >>>> from present variant members" convenience feature to QAPI, but I'm not >> >>>> sure it's worth the complexity. >> >>>> >> >>> >> >>> It also may be not a union, but a simple structure with optional fields: >> >>> >> >>> { >> >>> parent: str, required, qom-path, or node-name, or export name >> >>> child: str, optional, but required when parent is node-name, and >> >>> must be 'root' >> >>> if present for other parent types >> >>> parent-type: str, optional, "qom-path" | "block-node" | >> >>> "block-export", helps to >> >>> overcome parent ambiguity, deprecated >> >>> new-child: str, required node-name >> >>> } >> >> >> >> PRO union: which members are valid together is syntactically obvious. >> >> With a bunch of optional members, that information is in the member >> >> descriptions. >> >> >> >> CON union: need to specify the union tag. The verbosity is a non-issue >> >> for management applications, and mildly bothersome for humans. >> >> >> >> How important is keeping things terse for humans here? >> >> >> > >> > It's not very important. Mostly, I just want to make "nice" interface, and >> > possibility to >> > reference any supported object by one ID seemed very tempting to me. >> > >> > But to be honest, introducing such a precedent makes sense, if we have in >> > mind more >> > cases where such approach may be reused. Personally, I don't have them. If >> > blockdev-replace >> > command remains the only API for the ages, which allows to mix in one >> > field different kinds >> > of IDs, it becomes just an extra exclusion from common practices. >> > >> > Finally, I don't care too much, which interface we chose. I'm not sure now >> > that unifying >> > ids is better than union. So I'm OK to go back to union-based solution >> > from v9, update and >> > resend it as v11. >> >> Hanna, Kevin, got a preference? > > My stance is unchanged from v9. When Vladimir asked if we could somehow > get rid of the union, I tried to help him find such a solution that > might be acceptable, but also recommended to keep the union for now, > simply because it raises a lot less questions and avoids the temporary > trap, and to move to a unified ID only later. > > Still, as I said, "I don't think such a state is very pretty, but it > would be okay for me". So I wouldn't block the approach taken here in > v10 if Vladimir prefers it (at least as far as I can say from the > discussion; I didn't actually review it yet). > > If you say that you really don't want to have the command fail on > ambiguous IDs, we probably shouldn't do it. But as far as I am > concerned, it seemed tolerable, so if you don't object, it remains > Vladimir's decision.
I don't object, I advise. Overall, the union approach seems cleaner, and more in line with existing interface practice, albeit a bit more verbose. It avoids the ambiguous ID issue. It's also a more flexible base for extensions: add a branch with whatever variant members are needed. Whether we'll ever need the flexibility is uncertain. The non-union approach looks workable to me. As long as ambiguous IDs remain possible, users can get their VMs into states where the command doesn't work. Documentation must spell this out, which is a bit of a bother. Experience has taught us that "don't do that" documentation reduces the likelihood of users doing it, but not nearly as much as we'd like. Warnings are more effective, and I strongly recommend to add them. More bother. We should also consider to mark the interface unstable. This would delay adoption, which may well be a reason not to mark. Independently, I find truly unique IDs desirable.
