diff mbox

[3/7] net: remove NetLegacy struct

Message ID 1308b58bb2b2d1a7f4d3f484ea4ed8d8697bb010.1441627176.git.DirtY.iCE.hu@gmail.com
State New
Headers show

Commit Message

=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Sept. 7, 2015, 12:08 p.m. UTC
NetLegacy is just Netdev with some extra fields (name, vlan) and an
optional id.  This patch merges the two structs, and net_client_init1
got some extra checks to make sure only accept valid -netdev command
lines.  This is some extra work, but allows us to uniformly manage both
legacy -net and non-legacy -netdev in code.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 net/net.c        | 42 +++++++++++++++++++++---------------------
 qapi-schema.json | 30 +++++++++---------------------
 2 files changed, 30 insertions(+), 42 deletions(-)

Comments

Eric Blake Sept. 15, 2015, 2:54 p.m. UTC | #1
On 09/07/2015 06:08 AM, Kővágó, Zoltán wrote:
> NetLegacy is just Netdev with some extra fields (name, vlan) and an
> optional id.  This patch merges the two structs, and net_client_init1
> got some extra checks to make sure only accept valid -netdev command
> lines.  This is some extra work, but allows us to uniformly manage both
> legacy -net and non-legacy -netdev in code.
> 
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  net/net.c        | 42 +++++++++++++++++++++---------------------
>  qapi-schema.json | 30 +++++++++---------------------
>  2 files changed, 30 insertions(+), 42 deletions(-)

I'm still not a fan of this one. See my counter-proposal that rewrites
patches 4/7 and 5/7 of this series to keep NetdevLegacy a separate type
for command line usage, while exposing only Netdev through QMP (in
particular, patch 2/29 and 27/29 are my rewrites, and 29/29 turns on
Netdev use in QMP)

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02580.html
diff mbox

Patch

diff --git a/net/net.c b/net/net.c
index 28a5597..10fbaca 100644
--- a/net/net.c
+++ b/net/net.c
@@ -911,17 +911,29 @@  static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
 };
 
 
-static int net_client_init1(const void *object, int is_netdev, Error **errp)
+static int net_client_init1(const Netdev *netdev, int is_netdev, Error **errp)
 {
-    const NetClientOptions *opts;
+    const NetClientOptions *opts = netdev->opts;
     const char *name;
     NetClientState *peer = NULL;
 
     if (is_netdev) {
-        const Netdev *netdev = object;
-        opts = netdev->opts;
         name = netdev->id;
 
+        /* validate -netdev option: has id, no vlan or name */
+        if (!netdev->has_id) {
+            error_setg(errp, QERR_MISSING_PARAMETER, "id");
+            return -1;
+        }
+        if (netdev->has_name) {
+            error_setg(errp, QERR_INVALID_PARAMETER, "name");
+            return -1;
+        }
+        if (netdev->has_vlan) {
+            error_setg(errp, QERR_INVALID_PARAMETER, "vlan");
+            return -1;
+        }
+
         if (opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP ||
             opts->kind == NET_CLIENT_OPTIONS_KIND_NIC ||
             !net_client_init_fun[opts->kind]) {
@@ -930,10 +942,8 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
             return -1;
         }
     } else {
-        const NetLegacy *net = object;
-        opts = net->opts;
         /* missing optional values have been initialized to "all bits zero" */
-        name = net->has_id ? net->id : net->name;
+        name = netdev->has_id ? netdev->id : netdev->name;
 
         if (opts->kind == NET_CLIENT_OPTIONS_KIND_NONE) {
             return 0; /* nothing to do */
@@ -954,7 +964,7 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
         /* Do not add to a vlan if it's a nic with a netdev= parameter. */
         if (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
             !opts->nic->has_netdev) {
-            peer = net_hub_add_port(net->has_vlan ? net->vlan : 0, NULL);
+            peer = net_hub_add_port(netdev->has_vlan ? netdev->vlan : 0, NULL);
         }
     }
 
@@ -970,26 +980,16 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
 }
 
 
-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;
+    Netdev *object = NULL;
     Error *err = NULL;
     int ret = -1;
 
     {
         OptsVisitor *ov = opts_visitor_new(opts);
 
-        net_visit(opts_get_visitor(ov), is_netdev, &object, &err);
+        visit_type_Netdev(opts_get_visitor(ov), &object, NULL, &err);
         opts_visitor_cleanup(ov);
     }
 
@@ -1000,7 +1000,7 @@  int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
     if (object) {
         QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
 
-        net_visit(qapi_dealloc_get_visitor(dv), is_netdev, &object, NULL);
+        visit_type_Netdev(qapi_dealloc_get_visitor(dv), &object, NULL, NULL);
         qapi_dealloc_visitor_cleanup(dv);
     }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 9e4fe5d..d85b55b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2513,21 +2513,25 @@ 
     'vhost-user': 'NetdevVhostUserOptions' } }
 
 ##
-# @NetLegacy
+# @Netdev
 #
-# Captures the configuration of a network device; legacy.
+# Captures the configuration of a network device.
 #
-# @vlan: #optional vlan number
+# @vlan: #optional vlan number (legacy, forbidden with -netdev)
 #
-# @id: #optional identifier for monitor commands
+# @id: #optional identifier for monitor commands (required with -netdev)
 #
 # @name: #optional identifier for monitor commands, ignored if @id is present
+#        (legacy, forbidden with -netdev)
 #
 # @opts: device type specific properties (legacy)
 #
 # Since 1.2
+#
+# @id #optional - since 2.5
+# @vlan, @name - since 2.5
 ##
-{ 'struct': 'NetLegacy',
+{ 'struct': 'Netdev',
   'data': {
     '*vlan': 'int32',
     '*id':   'str',
@@ -2535,22 +2539,6 @@ 
     'opts':  'NetClientOptions' } }
 
 ##
-# @Netdev
-#
-# Captures the configuration of a network device.
-#
-# @id: identifier for monitor commands.
-#
-# @opts: device type specific properties
-#
-# Since 1.2
-##
-{ 'struct': 'Netdev',
-  'data': {
-    'id':   'str',
-    'opts': 'NetClientOptions' } }
-
-##
 # @InetSocketAddress
 #
 # Captures a socket address or address range in the Internet namespace.