Re: [PATCH v10 4/8] qapi: add blockdev-replace command

[Markus Armbruster <armbru@redhat.com>]

Block layer maintainers' advice sought:

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 17.02.26 16:10, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 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/20240626115350.405778-5-vsementsov@yandex-team.ru/
>>>
>>> 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?