qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in
@ 2015-05-27 16:16 Stefan Hajnoczi
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[] Stefan Hajnoczi
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

This series cleans up net_client_init1() and reports an error when the -net
type isn't compiled into the QEMU binary.

It is based on my net branch with Markus' error cleanups:
https://github.com/stefanha/qemu/tree/net

Stefan Hajnoczi (5):
  net: add missing "netmap" to host_net_devices[]
  net: replace net_client_init1() netdev whitelist with blacklist
  net: raise an error if -net type is invalid
  net: drop if expression that is always true
  net: simplify net_client_init1()

 net/net.c | 91 ++++++++++++++++++++++++++-------------------------------------
 1 file changed, 37 insertions(+), 54 deletions(-)

-- 
2.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[]
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
@ 2015-05-27 16:16 ` Stefan Hajnoczi
  2015-05-27 17:39   ` Thomas Huth
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist Stefan Hajnoczi
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

Although hmp-commands.hx lists "netmap" as a valid host_net_add type,
the command rejects it because it's missing from the list.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/net.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/net.c b/net/net.c
index db6be12..c5349d2 100644
--- a/net/net.c
+++ b/net/net.c
@@ -58,6 +58,9 @@ const char *host_net_devices[] = {
 #ifdef CONFIG_NET_BRIDGE
     "bridge",
 #endif
+#ifdef CONFIG_NETMAP
+    "netmap",
+#endif
 #ifdef CONFIG_SLIRP
     "user",
 #endif
-- 
2.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[] Stefan Hajnoczi
@ 2015-05-27 16:16 ` Stefan Hajnoczi
  2015-05-27 17:42   ` Thomas Huth
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid Stefan Hajnoczi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

It's cumbersome to keep the whitelist up-to-date.  New netdev backends
should most likely be allowed so a blacklist makes more sense than a
whitelist.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/net.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/net/net.c b/net/net.c
index c5349d2..3352b2b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -905,31 +905,9 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         opts = u.netdev->opts;
         name = u.netdev->id;
 
-        switch (opts->kind) {
-#ifdef CONFIG_SLIRP
-        case NET_CLIENT_OPTIONS_KIND_USER:
-#endif
-        case NET_CLIENT_OPTIONS_KIND_TAP:
-        case NET_CLIENT_OPTIONS_KIND_SOCKET:
-#ifdef CONFIG_VDE
-        case NET_CLIENT_OPTIONS_KIND_VDE:
-#endif
-#ifdef CONFIG_NETMAP
-        case NET_CLIENT_OPTIONS_KIND_NETMAP:
-#endif
-#ifdef CONFIG_NET_BRIDGE
-        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
-#endif
-        case NET_CLIENT_OPTIONS_KIND_HUBPORT:
-#ifdef CONFIG_VHOST_NET_USED
-        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
-#endif
-#ifdef CONFIG_L2TPV3
-        case NET_CLIENT_OPTIONS_KIND_L2TPV3:
-#endif
-            break;
-
-        default:
+        if (opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP ||
+            opts->kind == NET_CLIENT_OPTIONS_KIND_NIC ||
+            !net_client_init_fun[opts->kind]) {
             error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
                       "a netdev backend type");
             return -1;
-- 
2.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[] Stefan Hajnoczi
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist Stefan Hajnoczi
@ 2015-05-27 16:16 ` Stefan Hajnoczi
  2015-05-27 19:13   ` Thomas Huth
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true Stefan Hajnoczi
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

When a -net type is used that was not compiled into the binary there
should be an error message.

Note the special case for -net none, which is a no-op.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/net.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/net.c b/net/net.c
index 3352b2b..85a9ddb 100644
--- a/net/net.c
+++ b/net/net.c
@@ -922,6 +922,17 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         }
         /* missing optional values have been initialized to "all bits zero" */
         name = u.net->has_id ? u.net->id : u.net->name;
+
+        if (opts->kind == NET_CLIENT_OPTIONS_KIND_NONE) {
+            return 0; /* nothing to do */
+        }
+
+        if (!net_client_init_fun[opts->kind]) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                      "a net backend type (maybe it is not compiled "
+                      "into this binary)");
+            return -1;
+        }
     }
 
     if (net_client_init_fun[opts->kind]) {
-- 
2.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid Stefan Hajnoczi
@ 2015-05-27 16:16 ` Stefan Hajnoczi
  2015-05-27 19:16   ` Thomas Huth
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1() Stefan Hajnoczi
  2015-06-18 13:28 ` [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

Both is_netdev and !is_netdev paths already check that
net_client_init_func[opts->kind] is non-NULL so there is no need for the
if statement.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/net.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/net.c b/net/net.c
index 85a9ddb..cc1793c 100644
--- a/net/net.c
+++ b/net/net.c
@@ -899,6 +899,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
     } u;
     const NetClientOptions *opts;
     const char *name;
+    NetClientState *peer = NULL;
 
     if (is_netdev) {
         u.netdev = object;
@@ -935,25 +936,21 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         }
     }
 
-    if (net_client_init_fun[opts->kind]) {
-        NetClientState *peer = NULL;
+    /* 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)) {
+        peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
+    }
 
-        /* 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)) {
-            peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
-        }
-
-        if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
-            /* FIXME drop when all init functions store an Error */
-            if (errp && !*errp) {
-                error_set(errp, QERR_DEVICE_INIT_FAILED,
-                          NetClientOptionsKind_lookup[opts->kind]);
-            }
-            return -1;
+    if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
+        /* FIXME drop when all init functions store an Error */
+        if (errp && !*errp) {
+            error_set(errp, QERR_DEVICE_INIT_FAILED,
+                      NetClientOptionsKind_lookup[opts->kind]);
         }
+        return -1;
     }
     return 0;
 }
-- 
2.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1()
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
                   ` (3 preceding siblings ...)
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true Stefan Hajnoczi
@ 2015-05-27 16:16 ` Stefan Hajnoczi
  2015-05-27 19:22   ` Thomas Huth
  2015-06-18 13:28 ` [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 16:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Stefan Hajnoczi

Drop the union and move the hubport creation into the !is_netdev case.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/net.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/net/net.c b/net/net.c
index cc1793c..e57a089 100644
--- a/net/net.c
+++ b/net/net.c
@@ -893,18 +893,14 @@ 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)
 {
-    union {
-        const Netdev    *netdev;
-        const NetLegacy *net;
-    } u;
     const NetClientOptions *opts;
     const char *name;
     NetClientState *peer = NULL;
 
     if (is_netdev) {
-        u.netdev = object;
-        opts = u.netdev->opts;
-        name = u.netdev->id;
+        const Netdev *netdev = object;
+        opts = netdev->opts;
+        name = netdev->id;
 
         if (opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP ||
             opts->kind == NET_CLIENT_OPTIONS_KIND_NIC ||
@@ -914,19 +910,19 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
             return -1;
         }
     } else {
-        u.net = object;
-        opts = u.net->opts;
-        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
-            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
-                      "a net type");
-            return -1;
-        }
+        const NetLegacy *net = object;
+        opts = net->opts;
         /* missing optional values have been initialized to "all bits zero" */
-        name = u.net->has_id ? u.net->id : u.net->name;
+        name = net->has_id ? net->id : net->name;
 
         if (opts->kind == NET_CLIENT_OPTIONS_KIND_NONE) {
             return 0; /* nothing to do */
         }
+        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                      "a net type");
+            return -1;
+        }
 
         if (!net_client_init_fun[opts->kind]) {
             error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
@@ -934,14 +930,12 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
                       "into this binary)");
             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)) {
-        peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
+        /* 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);
+        }
     }
 
     if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
-- 
2.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[]
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[] Stefan Hajnoczi
@ 2015-05-27 17:39   ` Thomas Huth
  2015-05-29 13:18     ` Stefan Hajnoczi
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Huth @ 2015-05-27 17:39 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

On Wed, 27 May 2015 17:16:48 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Although hmp-commands.hx lists "netmap" as a valid host_net_add type,
> the command rejects it because it's missing from the list.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/net.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index db6be12..c5349d2 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -58,6 +58,9 @@ const char *host_net_devices[] = {
>  #ifdef CONFIG_NET_BRIDGE
>      "bridge",
>  #endif
> +#ifdef CONFIG_NETMAP
> +    "netmap",
> +#endif
>  #ifdef CONFIG_SLIRP
>      "user",
>  #endif

Did you consider to remove it from the help text in hmp-commands.hx
instead?
That would force the users to use "netdev_add" for this instead - one
more reason to get away from the legacy "net" syntax ;-)

 Thomas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist Stefan Hajnoczi
@ 2015-05-27 17:42   ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2015-05-27 17:42 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

On Wed, 27 May 2015 17:16:49 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> It's cumbersome to keep the whitelist up-to-date.  New netdev backends
> should most likely be allowed so a blacklist makes more sense than a
> whitelist.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/net.c | 28 +++-------------------------
>  1 file changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index c5349d2..3352b2b 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -905,31 +905,9 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          opts = u.netdev->opts;
>          name = u.netdev->id;
>  
> -        switch (opts->kind) {
> -#ifdef CONFIG_SLIRP
> -        case NET_CLIENT_OPTIONS_KIND_USER:
> -#endif
> -        case NET_CLIENT_OPTIONS_KIND_TAP:
> -        case NET_CLIENT_OPTIONS_KIND_SOCKET:
> -#ifdef CONFIG_VDE
> -        case NET_CLIENT_OPTIONS_KIND_VDE:
> -#endif
> -#ifdef CONFIG_NETMAP
> -        case NET_CLIENT_OPTIONS_KIND_NETMAP:
> -#endif
> -#ifdef CONFIG_NET_BRIDGE
> -        case NET_CLIENT_OPTIONS_KIND_BRIDGE:
> -#endif
> -        case NET_CLIENT_OPTIONS_KIND_HUBPORT:
> -#ifdef CONFIG_VHOST_NET_USED
> -        case NET_CLIENT_OPTIONS_KIND_VHOST_USER:
> -#endif
> -#ifdef CONFIG_L2TPV3
> -        case NET_CLIENT_OPTIONS_KIND_L2TPV3:
> -#endif
> -            break;
> -
> -        default:
> +        if (opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP ||
> +            opts->kind == NET_CLIENT_OPTIONS_KIND_NIC ||
> +            !net_client_init_fun[opts->kind]) {
>              error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>                        "a netdev backend type");
>              return -1;

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid Stefan Hajnoczi
@ 2015-05-27 19:13   ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2015-05-27 19:13 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

On Wed, 27 May 2015 17:16:50 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> When a -net type is used that was not compiled into the binary there
> should be an error message.
> 
> Note the special case for -net none, which is a no-op.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/net.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index 3352b2b..85a9ddb 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -922,6 +922,17 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          }
>          /* missing optional values have been initialized to "all bits zero" */
>          name = u.net->has_id ? u.net->id : u.net->name;
> +
> +        if (opts->kind == NET_CLIENT_OPTIONS_KIND_NONE) {
> +            return 0; /* nothing to do */
> +        }
> +
> +        if (!net_client_init_fun[opts->kind]) {
> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> +                      "a net backend type (maybe it is not compiled "
> +                      "into this binary)");
> +            return -1;
> +        }
>      }
>  
>      if (net_client_init_fun[opts->kind]) {

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true Stefan Hajnoczi
@ 2015-05-27 19:16   ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2015-05-27 19:16 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

On Wed, 27 May 2015 17:16:51 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Both is_netdev and !is_netdev paths already check that
> net_client_init_func[opts->kind] is non-NULL so there is no need for the
> if statement.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/net.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 85a9ddb..cc1793c 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -899,6 +899,7 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>      } u;
>      const NetClientOptions *opts;
>      const char *name;
> +    NetClientState *peer = NULL;
>  
>      if (is_netdev) {
>          u.netdev = object;
> @@ -935,25 +936,21 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>          }
>      }
>  
> -    if (net_client_init_fun[opts->kind]) {
> -        NetClientState *peer = NULL;
> +    /* 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)) {
> +        peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
> +    }
>  
> -        /* 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)) {
> -            peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
> -        }
> -
> -        if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
> -            /* FIXME drop when all init functions store an Error */
> -            if (errp && !*errp) {
> -                error_set(errp, QERR_DEVICE_INIT_FAILED,
> -                          NetClientOptionsKind_lookup[opts->kind]);
> -            }
> -            return -1;
> +    if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {
> +        /* FIXME drop when all init functions store an Error */
> +        if (errp && !*errp) {
> +            error_set(errp, QERR_DEVICE_INIT_FAILED,
> +                      NetClientOptionsKind_lookup[opts->kind]);
>          }
> +        return -1;
>      }
>      return 0;
>  }

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1()
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1() Stefan Hajnoczi
@ 2015-05-27 19:22   ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2015-05-27 19:22 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

On Wed, 27 May 2015 17:16:52 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Drop the union and move the hubport creation into the !is_netdev case.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  net/net.c | 38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index cc1793c..e57a089 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -893,18 +893,14 @@ 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)
>  {
> -    union {
> -        const Netdev    *netdev;
> -        const NetLegacy *net;
> -    } u;
>      const NetClientOptions *opts;
>      const char *name;
>      NetClientState *peer = NULL;
>  
>      if (is_netdev) {
> -        u.netdev = object;
> -        opts = u.netdev->opts;
> -        name = u.netdev->id;
> +        const Netdev *netdev = object;
> +        opts = netdev->opts;
> +        name = netdev->id;
>  
>          if (opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP ||
>              opts->kind == NET_CLIENT_OPTIONS_KIND_NIC ||
> @@ -914,19 +910,19 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>              return -1;
>          }
>      } else {
> -        u.net = object;
> -        opts = u.net->opts;
> -        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
> -            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> -                      "a net type");
> -            return -1;
> -        }
> +        const NetLegacy *net = object;
> +        opts = net->opts;
>          /* missing optional values have been initialized to "all bits zero" */
> -        name = u.net->has_id ? u.net->id : u.net->name;
> +        name = net->has_id ? net->id : net->name;
>  
>          if (opts->kind == NET_CLIENT_OPTIONS_KIND_NONE) {
>              return 0; /* nothing to do */
>          }
> +        if (opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> +                      "a net type");
> +            return -1;
> +        }
>  
>          if (!net_client_init_fun[opts->kind]) {
>              error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> @@ -934,14 +930,12 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>                        "into this binary)");
>              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)) {
> -        peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
> +        /* 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);
> +        }
>      }
>  
>      if (net_client_init_fun[opts->kind](opts, name, peer, errp) < 0) {

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[]
  2015-05-27 17:39   ` Thomas Huth
@ 2015-05-29 13:18     ` Stefan Hajnoczi
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-05-29 13:18 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

On Wed, May 27, 2015 at 07:39:17PM +0200, Thomas Huth wrote:
> On Wed, 27 May 2015 17:16:48 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > Although hmp-commands.hx lists "netmap" as a valid host_net_add type,
> > the command rejects it because it's missing from the list.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  net/net.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/net.c b/net/net.c
> > index db6be12..c5349d2 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -58,6 +58,9 @@ const char *host_net_devices[] = {
> >  #ifdef CONFIG_NET_BRIDGE
> >      "bridge",
> >  #endif
> > +#ifdef CONFIG_NETMAP
> > +    "netmap",
> > +#endif
> >  #ifdef CONFIG_SLIRP
> >      "user",
> >  #endif
> 
> Did you consider to remove it from the help text in hmp-commands.hx
> instead?
> That would force the users to use "netdev_add" for this instead - one
> more reason to get away from the legacy "net" syntax ;-)

There's no reason to artifically limit the command and the intention of
the documentation was clear.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in
  2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
                   ` (4 preceding siblings ...)
  2015-05-27 16:16 ` [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1() Stefan Hajnoczi
@ 2015-06-18 13:28 ` Stefan Hajnoczi
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2015-06-18 13:28 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Paolo Bonzini, qemu-devel, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Wed, May 27, 2015 at 05:16:47PM +0100, Stefan Hajnoczi wrote:
> This series cleans up net_client_init1() and reports an error when the -net
> type isn't compiled into the QEMU binary.
> 
> It is based on my net branch with Markus' error cleanups:
> https://github.com/stefanha/qemu/tree/net
> 
> Stefan Hajnoczi (5):
>   net: add missing "netmap" to host_net_devices[]
>   net: replace net_client_init1() netdev whitelist with blacklist
>   net: raise an error if -net type is invalid
>   net: drop if expression that is always true
>   net: simplify net_client_init1()
> 
>  net/net.c | 91 ++++++++++++++++++++++++++-------------------------------------
>  1 file changed, 37 insertions(+), 54 deletions(-)
> 
> -- 
> 2.4.1
> 
> 

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-06-18 13:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-27 16:16 [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi
2015-05-27 16:16 ` [Qemu-devel] [PATCH 1/5] net: add missing "netmap" to host_net_devices[] Stefan Hajnoczi
2015-05-27 17:39   ` Thomas Huth
2015-05-29 13:18     ` Stefan Hajnoczi
2015-05-27 16:16 ` [Qemu-devel] [PATCH 2/5] net: replace net_client_init1() netdev whitelist with blacklist Stefan Hajnoczi
2015-05-27 17:42   ` Thomas Huth
2015-05-27 16:16 ` [Qemu-devel] [PATCH 3/5] net: raise an error if -net type is invalid Stefan Hajnoczi
2015-05-27 19:13   ` Thomas Huth
2015-05-27 16:16 ` [Qemu-devel] [PATCH 4/5] net: drop if expression that is always true Stefan Hajnoczi
2015-05-27 19:16   ` Thomas Huth
2015-05-27 16:16 ` [Qemu-devel] [PATCH 5/5] net: simplify net_client_init1() Stefan Hajnoczi
2015-05-27 19:22   ` Thomas Huth
2015-06-18 13:28 ` [Qemu-devel] [PATCH 0/5] net: error when -net type isn't compiled in Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).