* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.