diff mbox

[v2,04/49] net: remove NetLegacy struct

Message ID e1c1133eeb8f97bd08dead17860621e9df4aa258.1440171025.git.DirtY.iCE.hu@gmail.com
State New
Headers show

Commit Message

=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Aug. 21, 2015, 3:37 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. 4, 2015, 9:25 p.m. UTC | #1
On 08/21/2015 09:37 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(-)
> 
> 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)

I'm really hoping to fix QMP netdev_add to use a flat union type from
the get-go rather than its current type-unsafe gunk.  It may be safer
from the point of view of this patch to wait for that to land first
(among others, we'll want:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02093.html)

So I'm reluctant to review this right now, if it will just add churn
into getting that part done correctly.

Don't throw this away just yet, because I'm not sure if QMP netdev_add
needs to interact with command-line NetdevLegacy.  But be aware that it
is still an area under active debate/development.
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 999faa3..8253d0a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2499,21 +2499,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',
@@ -2521,22 +2525,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.