convert net_client_init() to OptsVisitor

The net_client_init() prototype is kept intact.

Based on "is_netdev", the QemuOpts-rooted QemuOpt-list is parsed as a
Netdev or a NetLegacy. The original meat of net_client_init() is moved to
and simplified in net_client_init1():

Fields not common between -net and -netdev are clearly separated. Getting
the name for the init functions is cleaner: Netdev::id is mandatory, and
all init functions handle a NULL NetLegacy::name. NetLegacy::vlan
explicitly depends on -net (see below).

Verifying the "type=" option for -netdev can be turned into a switch.

Format validation with qemu_opts_validate() can be removed because the
visitor covers it. Relatedly, the "net_client_types" array is reduced to
an array of init functions that can be directly indexed by opts->kind.
(Help text is available in the schema JSON.)

The outermost negation in the condition around qemu_find_vlan() was
flattened, because it expresses the dependent code's requirements more
clearly.

VLAN lookup is avoided if there's no init function to pass the VLAN to.

Whenever the value of type=... is needed, we substitute
NetClientOptionsKind_lookup[kind].

The individual init functions are not converted yet, thus the original
QemuOpts instance is passed transparently.

v1->v2:
- NetLegacy::name is optional. Tracked it through all init functions: they
  all handle a NULL name. Updated commit message accordingly.

v2->v3:
- NetLegacy::id is allowed and takes precedence over NetLegacy::name.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

net.c

@@ -37,6 +37,9 @@
 #include "qmp-commands.h"
 #include "hw/qdev.h"
 #include "iov.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/qapi-dealloc-visitor.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -745,7 +748,8 @@ int net_handle_fd_param(Monitor *mon, const char *param)
     return fd;
 }
 
-static int net_init_nic(QemuOpts *opts, const char *name, VLANState *vlan)
+static int net_init_nic(QemuOpts *opts, const NetClientOptions *new_opts,
+                        const char *name, VLANState *vlan)
 {
     int idx;
     NICInfo *nd;
@@ -802,371 +806,130 @@ static int net_init_nic(QemuOpts *opts, const char *name, VLANState *vlan)
     return idx;
 }
 
-#define NET_COMMON_PARAMS_DESC                     \
-    {                                              \
-        .name = "type",                            \
-        .type = QEMU_OPT_STRING,                   \
-        .help = "net client type (nic, tap etc.)", \
-     }, {                                          \
-        .name = "vlan",                            \
-        .type = QEMU_OPT_NUMBER,                   \
-        .help = "vlan number",                     \
-     }, {                                          \
-        .name = "name",                            \
-        .type = QEMU_OPT_STRING,                   \
-        .help = "identifier for monitor commands", \
-     }
 
-typedef int (*net_client_init_func)(QemuOpts *opts,
+static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
-                                    const char *name,
+    QemuOpts *old_opts,
-                                    VLANState *vlan);
+    const NetClientOptions *new_opts,
-
+    const char *name,
-/* magic number, but compiler will warn if too small */
+    VLANState *vlan) = {
-#define NET_MAX_DESC 20
+        [NET_CLIENT_OPTIONS_KIND_NIC]    = net_init_nic,
-
-static const struct {
-    const char *type;
-    net_client_init_func init;
-    QemuOptDesc desc[NET_MAX_DESC];
-} net_client_types[NET_CLIENT_OPTIONS_KIND_MAX] = {
-    [NET_CLIENT_OPTIONS_KIND_NONE] = {
-        .type = "none",
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            { /* end of list */ }
-        },
-    },
-    [NET_CLIENT_OPTIONS_KIND_NIC] = {
-        .type = "nic",
-        .init = net_init_nic,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "netdev",
-                .type = QEMU_OPT_STRING,
-                .help = "id of -netdev to connect to",
-            },
-            {
-                .name = "macaddr",
-                .type = QEMU_OPT_STRING,
-                .help = "MAC address",
-            }, {
-                .name = "model",
-                .type = QEMU_OPT_STRING,
-                .help = "device model (e1000, rtl8139, virtio etc.)",
-            }, {
-                .name = "addr",
-                .type = QEMU_OPT_STRING,
-                .help = "PCI device address",
-            }, {
-                .name = "vectors",
-                .type = QEMU_OPT_NUMBER,
-                .help = "number of MSI-x vectors, 0 to disable MSI-X",
-            },
-            { /* end of list */ }
-        },
-    },
 #ifdef CONFIG_SLIRP
-    [NET_CLIENT_OPTIONS_KIND_USER] = {
+        [NET_CLIENT_OPTIONS_KIND_USER]   = net_init_slirp,
-        .type = "user",
-        .init = net_init_slirp,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "hostname",
-                .type = QEMU_OPT_STRING,
-                .help = "client hostname reported by the builtin DHCP server",
-            }, {
-                .name = "restrict",
-                .type = QEMU_OPT_STRING,
-                .help = "isolate the guest from the host (y|yes|n|no)",
-            }, {
-                .name = "ip",
-                .type = QEMU_OPT_STRING,
-                .help = "legacy parameter, use net= instead",
-            }, {
-                .name = "net",
-                .type = QEMU_OPT_STRING,
-                .help = "IP address and optional netmask",
-            }, {
-                .name = "host",
-                .type = QEMU_OPT_STRING,
-                .help = "guest-visible address of the host",
-            }, {
-                .name = "tftp",
-                .type = QEMU_OPT_STRING,
-                .help = "root directory of the built-in TFTP server",
-            }, {
-                .name = "bootfile",
-                .type = QEMU_OPT_STRING,
-                .help = "BOOTP filename, for use with tftp=",
-            }, {
-                .name = "dhcpstart",
-                .type = QEMU_OPT_STRING,
-                .help = "the first of the 16 IPs the built-in DHCP server can assign",
-            }, {
-                .name = "dns",
-                .type = QEMU_OPT_STRING,
-                .help = "guest-visible address of the virtual nameserver",
-            }, {
-                .name = "smb",
-                .type = QEMU_OPT_STRING,
-                .help = "root directory of the built-in SMB server",
-            }, {
-                .name = "smbserver",
-                .type = QEMU_OPT_STRING,
-                .help = "IP address of the built-in SMB server",
-            }, {
-                .name = "hostfwd",
-                .type = QEMU_OPT_STRING,
-                .help = "guest port number to forward incoming TCP or UDP connections",
-            }, {
-                .name = "guestfwd",
-                .type = QEMU_OPT_STRING,
-                .help = "IP address and port to forward guest TCP connections",
-            },
-            { /* end of list */ }
-        },
-    },
 #endif
-    [NET_CLIENT_OPTIONS_KIND_TAP] = {
+        [NET_CLIENT_OPTIONS_KIND_TAP]    = net_init_tap,
-        .type = "tap",
+        [NET_CLIENT_OPTIONS_KIND_SOCKET] = net_init_socket,
-        .init = net_init_tap,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "ifname",
-                .type = QEMU_OPT_STRING,
-                .help = "interface name",
-            },
-#ifndef _WIN32
-            {
-                .name = "fd",
-                .type = QEMU_OPT_STRING,
-                .help = "file descriptor of an already opened tap",
-            }, {
-                .name = "script",
-                .type = QEMU_OPT_STRING,
-                .help = "script to initialize the interface",
-            }, {
-                .name = "downscript",
-                .type = QEMU_OPT_STRING,
-                .help = "script to shut down the interface",
-            }, {
-#ifdef CONFIG_NET_BRIDGE
-                .name = "helper",
-                .type = QEMU_OPT_STRING,
-                .help = "command to execute to configure bridge",
-            }, {
-#endif
-                .name = "sndbuf",
-                .type = QEMU_OPT_SIZE,
-                .help = "send buffer limit"
-            }, {
-                .name = "vnet_hdr",
-                .type = QEMU_OPT_BOOL,
-                .help = "enable the IFF_VNET_HDR flag on the tap interface"
-            }, {
-                .name = "vhost",
-                .type = QEMU_OPT_BOOL,
-                .help = "enable vhost-net network accelerator",
-            }, {
-                .name = "vhostfd",
-                .type = QEMU_OPT_STRING,
-                .help = "file descriptor of an already opened vhost net device",
-            }, {
-                .name = "vhostforce",
-                .type = QEMU_OPT_BOOL,
-                .help = "force vhost on for non-MSIX virtio guests",
-        },
-#endif /* _WIN32 */
-            { /* end of list */ }
-        },
-    },
-    [NET_CLIENT_OPTIONS_KIND_SOCKET] = {
-        .type = "socket",
-        .init = net_init_socket,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "fd",
-                .type = QEMU_OPT_STRING,
-                .help = "file descriptor of an already opened socket",
-            }, {
-                .name = "listen",
-                .type = QEMU_OPT_STRING,
-                .help = "port number, and optional hostname, to listen on",
-            }, {
-                .name = "connect",
-                .type = QEMU_OPT_STRING,
-                .help = "port number, and optional hostname, to connect to",
-            }, {
-                .name = "mcast",
-                .type = QEMU_OPT_STRING,
-                .help = "UDP multicast address and port number",
-            }, {
-                .name = "localaddr",
-                .type = QEMU_OPT_STRING,
-                .help = "source address and port for multicast and udp packets",
-            }, {
-                .name = "udp",
-                .type = QEMU_OPT_STRING,
-                .help = "UDP unicast address and port number",
-            },
-            { /* end of list */ }
-        },
-    },
 #ifdef CONFIG_VDE
-    [NET_CLIENT_OPTIONS_KIND_VDE] = {
+        [NET_CLIENT_OPTIONS_KIND_VDE]    = net_init_vde,
-        .type = "vde",
-        .init = net_init_vde,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "sock",
-                .type = QEMU_OPT_STRING,
-                .help = "socket path",
-            }, {
-                .name = "port",
-                .type = QEMU_OPT_NUMBER,
-                .help = "port number",
-            }, {
-                .name = "group",
-                .type = QEMU_OPT_STRING,
-                .help = "group owner of socket",
-            }, {
-                .name = "mode",
-                .type = QEMU_OPT_NUMBER,
-                .help = "permissions for socket",
-            },
-            { /* end of list */ }
-        },
-    },
 #endif
-    [NET_CLIENT_OPTIONS_KIND_DUMP] = {
+        [NET_CLIENT_OPTIONS_KIND_DUMP]   = net_init_dump,
-        .type = "dump",
-        .init = net_init_dump,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "len",
-                .type = QEMU_OPT_SIZE,
-                .help = "per-packet size limit (64k default)",
-            }, {
-                .name = "file",
-                .type = QEMU_OPT_STRING,
-                .help = "dump file path (default is qemu-vlan0.pcap)",
-            },
-            { /* end of list */ }
-        },
-    },
 #ifdef CONFIG_NET_BRIDGE
-    [NET_CLIENT_OPTIONS_KIND_BRIDGE] = {
+        [NET_CLIENT_OPTIONS_KIND_BRIDGE] = net_init_bridge,
-        .type = "bridge",
+#endif
-        .init = net_init_bridge,
-        .desc = {
-            NET_COMMON_PARAMS_DESC,
-            {
-                .name = "br",
-                .type = QEMU_OPT_STRING,
-                .help = "bridge name",
-            }, {
-                .name = "helper",
-                .type = QEMU_OPT_STRING,
-                .help = "command to execute to configure bridge",
-            },
-            { /* end of list */ }
-        },
-    },
-#endif /* CONFIG_NET_BRIDGE */
 };
 
-int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
-{
-    const char *name;
-    const char *type;
-    int i;
 
-    type = qemu_opt_get(opts, "type");
+static int net_client_init1(const void *object, int is_netdev,
-    if (!type) {
+                            QemuOpts *old_opts, Error **errp)
-        error_set(errp, QERR_MISSING_PARAMETER, "type");
+{
-        return -1;
+    union {
-    }
+        const Netdev    *netdev;
+        const NetLegacy *net;
+    } u;
+    const NetClientOptions *opts;
+    const char *name;
 
     if (is_netdev) {
-        if (strcmp(type, "tap") != 0 &&
+        u.netdev = object;
-#ifdef CONFIG_NET_BRIDGE
+        opts = u.netdev->opts;
-            strcmp(type, "bridge") != 0 &&
+        name = u.netdev->id;
-#endif
+
+        switch (opts->kind) {
 #ifdef CONFIG_SLIRP
-            strcmp(type, "user") != 0 &&
+        case NET_CLIENT_OPTIONS_KIND_USER:
 #endif
+        case NET_CLIENT_OPTIONS_KIND_TAP:
+        case NET_CLIENT_OPTIONS_KIND_SOCKET:
 #ifdef CONFIG_VDE
-            strcmp(type, "vde") != 0 &&
+        case NET_CLIENT_OPTIONS_KIND_VDE:
 #endif
-            strcmp(type, "socket") != 0) {
+#ifdef CONFIG_NET_BRIDGE
+        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
+#endif
+            break;
+
+        default:
             error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
                       "a netdev backend type");
             return -1;
         }
+    } else {
+        u.net = object;
+        opts = u.net->opts;
+        /* missing optional values have been initialized to "all bits zero" */
+        name = u.net->has_id ? u.net->id : u.net->name;
+    }
 
-        if (qemu_opt_get(opts, "vlan")) {
+    if (net_client_init_fun[opts->kind]) {
-            error_set(errp, QERR_INVALID_PARAMETER, "vlan");
+        VLANState *vlan = NULL;
-            return -1;
+
+        /* Do not add to a vlan if it's a -netdev or a nic with a netdev=
+         * parameter. */
+        if (!is_netdev &&
+            (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
+             !opts->nic->has_netdev)) {
+            vlan = qemu_find_vlan(u.net->has_vlan ? u.net->vlan : 0, true);
         }
-        if (qemu_opt_get(opts, "name")) {
+
-            error_set(errp, QERR_INVALID_PARAMETER, "name");
+        if (net_client_init_fun[opts->kind](old_opts, opts, name, vlan) < 0) {
-            return -1;
+            /* TODO push error reporting into init() methods */
-        }
+            error_set(errp, QERR_DEVICE_INIT_FAILED,
-        if (!qemu_opts_id(opts)) {
+                      NetClientOptionsKind_lookup[opts->kind]);
-            error_set(errp, QERR_MISSING_PARAMETER, "id");
             return -1;
         }
     }
-
+    return 0;
-    name = qemu_opts_id(opts);
-    if (!name) {
-        name = qemu_opt_get(opts, "name");
-    }
-
-    for (i = 0; i < NET_CLIENT_OPTIONS_KIND_MAX; i++) {
-        if (net_client_types[i].type != NULL &&
-            !strcmp(net_client_types[i].type, type)) {
-            Error *local_err = NULL;
-            VLANState *vlan = NULL;
-            int ret;
-
-            qemu_opts_validate(opts, &net_client_types[i].desc[0], &local_err);
-            if (error_is_set(&local_err)) {
-                error_propagate(errp, local_err);
-                return -1;
-            }
-
-            /* Do not add to a vlan if it's a -netdev or a nic with a
-             * netdev= parameter. */
-            if (!(is_netdev ||
-                  (strcmp(type, "nic") == 0 && qemu_opt_get(opts, "netdev")))) {
-                vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
-            }
-
-            ret = 0;
-            if (net_client_types[i].init) {
-                ret = net_client_types[i].init(opts, name, vlan);
-                if (ret < 0) {
-                    /* TODO push error reporting into init() methods */
-                    error_set(errp, QERR_DEVICE_INIT_FAILED, type);
-                    return -1;
-                }
-            }
-            return ret;
-        }
-    }
-
-    error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
-              "a network client type");
-    return -1;
 }
 
+
+static void net_visit(Visitor *v, int is_netdev, void **object, Error **errp)
+{
+    if (is_netdev) {
+        visit_type_Netdev(v, (Netdev **)object, NULL, errp);
+    } else {
+        visit_type_NetLegacy(v, (NetLegacy **)object, NULL, errp);
+    }
+}
+
+
+int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
+{
+    void *object = NULL;
+    Error *err = NULL;
+    int ret = -1;
+
+    {
+        OptsVisitor *ov = opts_visitor_new(opts);
+
+        net_visit(opts_get_visitor(ov), is_netdev, &object, &err);
+        opts_visitor_cleanup(ov);
+    }
+
+    if (!err) {
+        ret = net_client_init1(object, is_netdev, opts, &err);
+    }
+
+    if (object) {
+        QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
+
+        net_visit(qapi_dealloc_get_visitor(dv), is_netdev, &object, NULL);
+        qapi_dealloc_visitor_cleanup(dv);
+    }
+
+    error_propagate(errp, err);
+    return ret;
+}
+
+
 static int net_host_check_device(const char *device)
 {
     int i;
@@ -1286,7 +1049,7 @@ void qmp_netdev_del(const char *id, Error **errp)
 static void print_net_client(Monitor *mon, VLANClientState *vc)
 {
     monitor_printf(mon, "%s: type=%s,%s\n", vc->name,
-                   net_client_types[vc->info->type].type, vc->info_str);
+                   NetClientOptionsKind_lookup[vc->info->type], vc->info_str);
 }
 
 void do_info_network(Monitor *mon)

net/dump.c

@@ -144,7 +144,8 @@ static int net_dump_init(VLANState *vlan, const char *device,
     return 0;
 }
 
-int net_init_dump(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_dump(QemuOpts *opts, const NetClientOptions *new_opts,
+                  const char *name, VLANState *vlan)
 {
     int len;
     const char *file;

net/dump.h

@@ -26,7 +26,9 @@
 
 #include "net.h"
 #include "qemu-common.h"
+#include "qapi-types.h"
 
-int net_init_dump(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_dump(QemuOpts *opts, const NetClientOptions *new_opts,
+                  const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_DUMP_H */

net/slirp.c

@@ -708,7 +708,8 @@ static int net_init_slirp_configs(const char *name, const char *value, void *opa
     return 0;
 }
 
-int net_init_slirp(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_slirp(QemuOpts *opts, const NetClientOptions *new_opts,
+                   const char *name, VLANState *vlan)
 {
     struct slirp_config_str *config;
     const char *vhost;

net/slirp.h

@@ -27,10 +27,12 @@
 #include "qemu-common.h"
 #include "qdict.h"
 #include "qemu-option.h"
+#include "qapi-types.h"
 
 #ifdef CONFIG_SLIRP
 
-int net_init_slirp(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_slirp(QemuOpts *opts, const NetClientOptions *new_opts,
+                   const char *name, VLANState *vlan);
 
 void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict);
 void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict);

net/socket.c

@@ -586,7 +586,8 @@ static int net_socket_udp_init(VLANState *vlan,
     return 0;
 }
 
-int net_init_socket(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_socket(QemuOpts *opts, const NetClientOptions *new_opts,
+                    const char *name, VLANState *vlan)
 {
     if (qemu_opt_get(opts, "fd")) {
         int fd;

net/socket.h

@@ -26,7 +26,9 @@
 
 #include "net.h"
 #include "qemu-common.h"
+#include "qapi-types.h"
 
-int net_init_socket(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_socket(QemuOpts *opts, const NetClientOptions *new_opts,
+                    const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_SOCKET_H */

net/tap-win32.c

@@ -699,7 +699,8 @@ static int tap_win32_init(VLANState *vlan, const char *model,
     return 0;
 }
 
-int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_tap(QemuOpts *opts, const NetClientOptions *new_opts,
+                 const char *name, VLANState *vlan)
 {
     const char *ifname;
 

net/tap.c

@@ -513,7 +513,8 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
     return -1;
 }
 
-int net_init_bridge(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_bridge(QemuOpts *opts, const NetClientOptions *new_opts,
+                    const char *name, VLANState *vlan)
 {
     TAPState *s;
     int fd, vnet_hdr;
@@ -583,7 +584,8 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr)
     return fd;
 }
 
-int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_tap(QemuOpts *opts, const NetClientOptions *new_opts,
+                 const char *name, VLANState *vlan)
 {
     TAPState *s;
     int fd, vnet_hdr = 0;

net/tap.h

@@ -28,11 +28,13 @@
 
 #include "qemu-common.h"
 #include "qemu-option.h"
+#include "qapi-types.h"
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
 
-int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_tap(QemuOpts *opts, const NetClientOptions *new_opts,
+                 const char *name, VLANState *vlan);
 
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required);
 
@@ -57,6 +59,7 @@ int tap_get_fd(VLANClientState *vc);
 struct vhost_net;
 struct vhost_net *tap_get_vhost_net(VLANClientState *vc);
 
-int net_init_bridge(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_bridge(QemuOpts *opts, const NetClientOptions *new_opts,
+                    const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_TAP_H */

net/vde.c

@@ -110,7 +110,8 @@ static int net_vde_init(VLANState *vlan, const char *model,
     return 0;
 }
 
-int net_init_vde(QemuOpts *opts, const char *name, VLANState *vlan)
+int net_init_vde(QemuOpts *opts, const NetClientOptions *new_opts,
+                 const char *name, VLANState *vlan)
 {
     const char *sock;
     const char *group;

net/vde.h

@@ -26,10 +26,12 @@
 
 #include "qemu-common.h"
 #include "qemu-option.h"
+#include "qapi-types.h"
 
 #ifdef CONFIG_VDE
 
-int net_init_vde(QemuOpts *opts, const char *name, VLANState *vlan);
+int net_init_vde(QemuOpts *opts, const NetClientOptions *new_opts,
+                 const char *name, VLANState *vlan);
 
 #endif /* CONFIG_VDE */