[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] net: Complete qapi-fication of netdev_add

From: Eric Blake
Subject: [PATCH] net: Complete qapi-fication of netdev_add
Date: Thu, 12 Mar 2020 16:14:40 -0500

We've had all the required pieces for doing a type-safe representation
of netdev_add as a flat union for quite some time now (since
0e55c381f6 in v2.7.0, released in 2016), but did not make the final
switch to using it because of concern about whether a command-line
regression in accepting "1" in place of 1 for integer arguments would
be problematic.  Back then, we did not have the deprecation cycle to
allow us to make progress.  But now that we have waited so long, other
problems have crept in: for example, our desire to add
qemu-storage-daemon is hampered by the inability to express net
objects, and we are unable to introspect what we actually accept.
Additionally, our round-trip through QemuOpts silently eats any
argument that expands to an array, rendering dnssearch, hostfwd, and
guestfwd useless through QMP:

{"execute": "netdev_add", "arguments": { "id": "netdev0",
  "type": "user", "dnssearch": [
    { "str": "" }, { "str": "" }

So without further ado, let's turn on proper QAPI.

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 now fails:

  'arguments':{'id':'net1', 'type':'hubport', 'hubid':1}}
{"return": {}}
  'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}}
{"error": {"class": "GenericError", "desc": "Invalid parameter type for 
'hubid', expected: integer"}}

But this stricter QMP is desirable, and introspection is sufficient
for any affected applications to make sure they use it correctly.

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.

Reported-by: Alex Kirillov <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
 qapi/net.json     | 14 +-------------
 include/net/net.h |  1 -
 monitor/misc.c    |  2 --
 net/net.c         |  6 +++---
 4 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/qapi/net.json b/qapi/net.json
index 1cb9a7d782b4..cebb1b52e3b1 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -39,18 +39,8 @@
 # Add a network backend.
-# @type: the type of network backend. Possible values are listed in
-#        NetClientDriver (excluding 'none' and 'nic')
-# @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
@@ -64,9 +54,7 @@
 # <- { "return": {} }
-{ '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 e175ba9677dc..96e6eae8176e 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -203,7 +203,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);

 int net_hub_id_for_client(NetClientState *nc, int *id);
 NetClientState *net_hub_port_find(int hub_id);
diff --git a/monitor/misc.c b/monitor/misc.c
index c3bc34c099dd..41a86e7012a1 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -247,8 +247,6 @@ static void monitor_init_qmp_commands(void)
                          qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
     qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
-    qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
-                         QCO_NO_OPTIONS);
     qmp_register_command(&qmp_commands, "object-add", qmp_object_add,

diff --git a/net/net.c b/net/net.c
index 9e93c3f8a1e2..a2065aabede2 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1170,7 +1170,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;
@@ -1181,12 +1181,12 @@ void qmp_netdev_add(QDict *qdict, QObject **ret, Error 
         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) {
         goto out;

reply via email to

[Prev in Thread] Current Thread [Next in Thread]