[Qemu-devel] [PATCH v9 14/17] net: Complete qapi-fication of netdev_add

[Eric Blake]

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

A later patch will resolve the problem by 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).

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 <address@hidden>

---
v9: s/box/boxed/, tweak commit message
v8: improve commit message, call out a change in QMP behavior,
inline netdev_add()
v7: no change
v6: don't lose qemu_opts handling

If you like the approach of the later patches, we can rearrange
things in the series to avoid the temporary regression
---
 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 5658723..7ec01e1 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', 'boxed': 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