Re: QMP netdev_add multiple dnssearch values

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 11/27/19 9:49 AM, Eric Blake wrote:
>> On 11/27/19 7:30 AM, Markus Armbruster wrote:
>>
>>> "Good enough" was true back then.  It wasn't true when we reused it for
>>> netdev_add: hostfwd and guestfwd are list-valued.
>>>
>>> We did define a QAPI schema a few months later (14aa0c2de0 "qapi schema:
>>> add Netdev types").  net_client_init() uses it to convert from QemuOpts
>>> to QAPI type Netdev.  This took us to the crazy pipeline we still use
>>> today:
>>>
>>>                              CLI, HMP
>>>                          (key=value,...)
>>>                                 |
>>>                                 v
>>>      QMP (JSON) -> QDict -> QemuOpts -> Netdev
>>>
>>> We should instead use:
>>>
>>>                            CLI, HMP
>>>                        (key=value,...)
>>>                               |
>>>                               v
>>>                            QemuOpts
>>>                               |
>>>                               v
>>>      QMP (JSON) -> QDict -> Netdev
>>>
>>> Back in 2016, Eric (cc'ed) posted patches to get us to this pipeline.
>>> They got stuck on backward compatibility worries: the old code accepts
>>> all parameters as JSON strings in addition to their proper type, the new
>>> code doesn't.  Undocumented misfeature, but we chickened out anyway.
>>
>> That was before we had a deprecation process.  Now we do.  If we are
>> still worried about it, then we should start the deprecation clock
>> (squeezing it into 4.2-rc3 is risky, more likely is starting it in
>> 5.0, so that we get rid of string support in 5.2).  If we are not
>> worried about it, then we can just kill the misfeature in 5.0.
>
> I'm leaning towards just killing the misfeature (it's a lot of glue
> code to add to support the misfeature for 5.0 and 5.1, if we're just
> going to rip it back out for 5.2), especially since introspection is
> enough for any affected clients to learn about the stricter behavior.

Here's my practical argument for simply killing it without further
delay.

We have a bug we know to affect users: the one reported by Alex.

We have a misfeature users may or may not rely on: the one that made us
chicken out.

To fix the bug, we have to kill the misfeature.  Fixing it without
killing the misfeature feels impractical.  For 5.0, it's flat-out
impossible.

Deprecating the misfeature for a grace period before we kill it means
letting the bug continue to bite users for the whole grace period.

Since we lack the means to communicate "you're using a deprecate
misfeature, stop it!" to QMP clients, the deprecation grace period is
unlikely to help much.

>>> Let's reconsider.  Eric's patches break interface misuse that may or may
>>> not exist in the field.  They fix a correct use of interface people want
>>> to use (or Alex wouldn't have reported this bug), and they make QMP
>>> introspection work for netdev_add.
>>>
>>> Eric, what do you think?
>>
>> Yes, it's time to revive that work (I have no idea if my patches
>> from back then will still rebase nicely, though).
>
> Now posted:
> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg03842.html

Thanks, I'll review.