Eric Blake <ebl...@redhat.com> writes: > We finally have all the required pieces for doing a type-safe > representation of netdev_add as a flat union, where the > discriminator 'type' now selects which additional members may > appear in the "arguments" JSON object sent over QMP, and exposes > those types through introspection, and without breaking command > line parsing. > > Inline the function netdev_add() into its lone remaining caller. > > There are a few places where the QMP 'netdev_add' command is now > more strict: anywhere that the QAPI lists an integer member, we > now require a strict JSON integer (previously, we allowed both > integers and strings, because the conversion from QMP to QemuOpts > back to QObject collapsed them into integers). For example, > pre-patch, both of these examples succeed, but post-patch, the > second example fails: > > {'execute':'netdev_add', > 'arguments':{'id':'net1', 'type':'hubport', 'hubid':1}} > {"return": {}} > {'execute':'netdev_add', > 'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}} > {"error": {"class": "GenericError", "desc": "Invalid parameter type for > 'hubid', expected: integer"}}
Needs to be covered in release notes. > If that turns out to be problematic, we could resolve the problem > by either adding a loose parsing mode to qmp-input-visitor (teaching > it to accept a string encoding of an integer in place of an actual > integer), or by using QAPI alternates. But that work should be > in followup patches, if at all. The sloppy typing is an accidental misfeature that is at odds with QMP's design. As far as I can tell, it's not documented anywhere. But lots of things aren't documented anywhere; the questions is whether they're used. If we find we must keep netdev_add misfeature-compatible, I want it deprecated in favor of a new command that adheres to the QMP design. Do we have to keep it misfeature-compatible proactively? How comfortable are we with "wait see whether anything breaks" here? The scenarios I want to avoid are "OMG, hold the release while we improvise a misfeature-compatibility hack", and "OMG, we need a stable release for misfeature compatibility a.s.a.p." I'm okay with breaking misfeature compatibility in this patch, and unbreak it separately if necessary. > In qmp_netdev_add(), we still have to create a QemuOpts object > so that qmp_netdev_del() will be able to remove a hotplugged > network device; but the opts->head remains empty since we now > manage all parsing through the QAPI object rather than QemuOpts. This use of QemuOpts as a network backend directory has always been somewhat questionable. More so now the QemuOpts is empty. But it's not this patch's business. > Signed-off-by: Eric Blake <ebl...@redhat.com>