[Qemu-devel] [PATCH v8 14/16] net: Complete qapi-fication of netdev_add

[Eric Blake <eblake@redhat.com>]

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"}}

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.

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.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v8: improve commit message, call out a change in QMP behavior,
inline netdev_add()
v7: no change
v6: don't lose qemu_opts handling
---
 qapi-schema.json  | 15 +++------------
 include/net/net.h |  2 --
 hmp.c             |  2 +-
 net/net.c         | 11 +++--------
 qmp-commands.hx   |  2 +-
 5 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index e8a015f..3a7632b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2362,26 +2362,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 e8d9e9e..820f880 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -210,8 +210,6 @@ void net_check_clients(void);
 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);

 int net_hub_id_for_client(NetClientState *nc, int *id);
 NetClientState *net_hub_port_find(int hub_id);
diff --git a/hmp.c b/hmp.c
index 4819abc..3502abf 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1672,7 +1672,7 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
         goto out;
     }

-    netdev_add(opts, &err);
+    net_client_init(opts, true, &err);
     if (err) {
         qemu_opts_del(opts);
     }
diff --git a/net/net.c b/net/net.c
index c124b11..1bd7936 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1181,12 +1181,7 @@ void hmp_host_net_remove(Monitor *mon, const QDict *qdict)
     qemu_del_net_client(nc);
 }

-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;
@@ -1197,12 +1192,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);
     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;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6937e83..cfc4086 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -976,7 +976,7 @@ EQMP
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .mhandler.cmd_new = qmp_netdev_add,
+        .mhandler.cmd_new = qmp_marshal_netdev_add,
     },

 SQMP
-- 
2.5.5