[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 13/15] net: Complete qapi-fication of netdev_
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 13/15] net: Complete qapi-fication of netdev_add |
Date: |
Thu, 16 Jun 2016 15:40:29 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> 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 <address@hidden>
>
> ---
> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v7 13/15] net: Complete qapi-fication of netdev_add,
Markus Armbruster <=