Eric Blake <[email protected]> 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, while > making no changes to the set of previously-valid QMP commands > that would work, and without breaking command line parsing.
Isn't it amazing that you pulled this off without a compatibility break? > > Signed-off-by: Eric Blake <[email protected]> > > --- > v7: no change > v6: don't lose qemu_opts handling > --- > qapi-schema.json | 15 +++------------ > include/net/net.h | 1 - > net/net.c | 6 +++--- > qmp-commands.hx | 2 +- > 4 files changed, 7 insertions(+), 17 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 4fee44b..fee4d07 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2301,26 +2301,17 @@ > # > # Add a network backend. > # > -# @type: the type of network backend. Current valid values are 'user', > 'tap', > -# 'vde', 'socket', 'dump' and 'bridge' > +# @type: the type of network backend; determines which additional arguments > +# are valid. See Netdev documentation for more details. > # > # @id: the name of the new network backend > # > -# Additional arguments depend on the type. > -# > -# TODO This command effectively bypasses QAPI completely due to its > -# "additional arguments" business. It shouldn't have been added to > -# the schema in this form. It should be qapified properly, or > -# replaced by a properly qapified command. > -# > # Since: 0.14.0 > # > # Returns: Nothing on success > # If @type is not a valid network backend, DeviceNotFound > ## > -{ 'command': 'netdev_add', > - 'data': {'type': 'str', 'id': 'str'}, > - 'gen': false } # so we can get the additional arguments > +{ 'command': 'netdev_add', 'data': 'Netdev', 'box': true } > > ## > # @netdev_del: > diff --git a/include/net/net.h b/include/net/net.h > index 74b32e0..8f9be98 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -193,7 +193,6 @@ void net_cleanup(void); > void hmp_host_net_add(Monitor *mon, const QDict *qdict); > void hmp_host_net_remove(Monitor *mon, const QDict *qdict); > void netdev_add(QemuOpts *opts, Error **errp); > -void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp); Only a dead QemuOpts is a good QemuOpts. > > int net_hub_id_for_client(NetClientState *nc, int *id); > NetClientState *net_hub_port_find(int hub_id); > diff --git a/net/net.c b/net/net.c > index e159506..b93ff00 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -1188,7 +1188,7 @@ void netdev_add(QemuOpts *opts, Error **errp) > net_client_init(opts, true, errp); > } > > -void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp) > +void qmp_netdev_add(Netdev *netdev, Error **errp) > { > Error *local_err = NULL; > QemuOptsList *opts_list; > @@ -1199,12 +1199,12 @@ void qmp_netdev_add(QDict *qdict, QObject **ret, > Error **errp) > goto out; > } > > - opts = qemu_opts_from_qdict(opts_list, qdict, &local_err); > + opts = qemu_opts_create(opts_list, netdev->id, 1, &local_err); We keep creating a QemuOpts for the network backend, but its opts->head remains empty. We need it for qmp_netdev_del(). Possibly more, but I doubt it. I think this is worth a mention in the commit message. > if (local_err) { > goto out; > } > > - netdev_add(opts, &local_err); > + net_client_init1(netdev, true, &local_err); > if (local_err) { > qemu_opts_del(opts); > goto out; Suggest to inline netdev_add() into its only remaining caller hmp_netdev_add(). > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 94847e5..3804030 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -949,7 +949,7 @@ EQMP > { > .name = "netdev_add", > .args_type = "netdev:O", > - .mhandler.cmd_new = qmp_netdev_add, > + .mhandler.cmd_new = qmp_marshal_netdev_add, > }, > > SQMP
