From 6a8b4a5be21ad4941c8a6a5db1d355a522aea2fb Mon Sep 17 00:00:00 2001
From: Thomas Huth <thuth@redhat.com>
Date: Fri, 15 May 2015 16:58:24 +0200
Subject: [PATCH 01/17] net: Change help text to list -netdev instead of -net
by default
Looking at the output of "qemu-system-xxx -help", you easily get
the impression that "-net" is the preferred way instead of "-netdev"
to specify host network interface, since the "-net" option is
omnipresent but the "-netdev" option is only listed as a one-liner
at the end. This is ugly since "-net" is considered as legacy and
even might be removed one day. Thus, this patch switches the output
to explain the host network interfaces with the "-netdev" option
instead, moving the old "-net" option into some few lines at
the end.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1431701904-12230-1-git-send-email-thuth@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
qemu-options.hx | 77 +++++++++++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 32 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index ec356f65c1..88d7661256 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1449,25 +1449,25 @@ DEF("smb", HAS_ARG, QEMU_OPTION_smb, "", QEMU_ARCH_ALL)
#endif
#endif
-DEF("net", HAS_ARG, QEMU_OPTION_net,
- "-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
- " create a new Network Interface Card and connect it to VLAN 'n'\n"
+DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
#ifdef CONFIG_SLIRP
- "-net user[,vlan=n][,name=str][,net=addr[/mask]][,host=addr][,restrict=on|off]\n"
+ "-netdev user,id=str[,net=addr[/mask]][,host=addr][,restrict=on|off]\n"
" [,hostname=host][,dhcpstart=addr][,dns=addr][,dnssearch=domain][,tftp=dir]\n"
" [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
#ifndef _WIN32
"[,smb=dir[,smbserver=addr]]\n"
#endif
- " connect the user mode network stack to VLAN 'n', configure its\n"
- " DHCP server and enabled optional services\n"
+ " configure a user mode network backend with ID 'str',\n"
+ " its DHCP server and optional services\n"
#endif
#ifdef _WIN32
- "-net tap[,vlan=n][,name=str],ifname=name\n"
- " connect the host TAP network interface to VLAN 'n'\n"
+ "-netdev tap,id=str,ifname=name\n"
+ " configure a host TAP network backend with ID 'str'\n"
#else
- "-net tap[,vlan=n][,name=str][,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
- " connect the host TAP network interface to VLAN 'n'\n"
+ "-netdev tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
+ " [,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
+ " [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
+ " configure a host TAP network backend with ID 'str'\n"
" use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
" to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
" to deconfigure it\n"
@@ -1486,14 +1486,18 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
" use 'vhostfd=h' to connect to an already opened vhost net device\n"
" use 'vhostfds=x:y:...:z to connect to multiple already opened vhost net devices\n"
" use 'queues=n' to specify the number of queues to be created for multiqueue TAP\n"
- "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
- " connects a host TAP network interface to a host bridge device 'br'\n"
- " (default=" DEFAULT_BRIDGE_INTERFACE ") using the program 'helper'\n"
- " (default=" DEFAULT_BRIDGE_HELPER ")\n"
+ "-netdev bridge,id=str[,br=bridge][,helper=helper]\n"
+ " configure a host TAP network backend with ID 'str' that is\n"
+ " connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
+ " using the program 'helper (default=" DEFAULT_BRIDGE_HELPER ")\n"
#endif
#ifdef __linux__
- "-net l2tpv3[,vlan=n][,name=str],src=srcaddr,dst=dstaddr[,srcport=srcport][,dstport=dstport],txsession=txsession[,rxsession=rxsession][,ipv6=on/off][,udp=on/off][,cookie64=on/off][,counter][,pincounter][,txcookie=txcookie][,rxcookie=rxcookie][,offset=offset]\n"
- " connect the VLAN to an Ethernet over L2TPv3 pseudowire\n"
+ "-netdev l2tpv3,id=str,src=srcaddr,dst=dstaddr[,srcport=srcport][,dstport=dstport]\n"
+ " [,rxsession=rxsession],txsession=txsession[,ipv6=on/off][,udp=on/off]\n"
+ " [,cookie64=on/off][,counter][,pincounter][,txcookie=txcookie]\n"
+ " [,rxcookie=rxcookie][,offset=offset]\n"
+ " configure a network backend with ID 'str' connected to\n"
+ " an Ethernet over L2TPv3 pseudowire.\n"
" Linux kernel 3.3+ as well as most routers can talk\n"
" L2TPv3. This transport allows connecting a VM to a VM,\n"
" VM to a router and even VM to Host. It is a nearly-universal\n"
@@ -1514,32 +1518,41 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
" use 'pincounter=on' to work around broken counter handling in peer\n"
" use 'offset=X' to add an extra offset between header and data\n"
#endif
- "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n"
- " connect the vlan 'n' to another VLAN using a socket connection\n"
- "-net socket[,vlan=n][,name=str][,fd=h][,mcast=maddr:port[,localaddr=addr]]\n"
- " connect the vlan 'n' to multicast maddr and port\n"
+ "-netdev socket,id=str[,fd=h][,listen=[host]:port][,connect=host:port]\n"
+ " configure a network backend to connect to another network\n"
+ " using a socket connection\n"
+ "-netdev socket,id=str[,fd=h][,mcast=maddr:port[,localaddr=addr]]\n"
+ " configure a network backend to connect to a multicast maddr and port\n"
" use 'localaddr=addr' to specify the host address to send packets from\n"
- "-net socket[,vlan=n][,name=str][,fd=h][,udp=host:port][,localaddr=host:port]\n"
- " connect the vlan 'n' to another VLAN using an UDP tunnel\n"
+ "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n"
+ " configure a network backend to connect to another network\n"
+ " using an UDP tunnel\n"
#ifdef CONFIG_VDE
- "-net vde[,vlan=n][,name=str][,sock=socketpath][,port=n][,group=groupname][,mode=octalmode]\n"
- " connect the vlan 'n' to port 'n' of a vde switch running\n"
- " on host and listening for incoming connections on 'socketpath'.\n"
+ "-netdev vde,id=str[,sock=socketpath][,port=n][,group=groupname][,mode=octalmode]\n"
+ " configure a network backend to connect to port 'n' of a vde switch\n"
+ " running on host and listening for incoming connections on 'socketpath'.\n"
" Use group 'groupname' and mode 'octalmode' to change default\n"
" ownership and permissions for communication port.\n"
#endif
#ifdef CONFIG_NETMAP
- "-net netmap,ifname=name[,devname=nmname]\n"
+ "-netdev netmap,id=str,ifname=name[,devname=nmname]\n"
" attach to the existing netmap-enabled network interface 'name', or to a\n"
" VALE port (created on the fly) called 'name' ('nmname' is name of the \n"
" netmap device, defaults to '/dev/netmap')\n"
#endif
+ "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
+ " configure a vhost-user network, backed by a chardev 'dev'\n"
+ "-netdev hubport,id=str,hubid=n\n"
+ " configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
+DEF("net", HAS_ARG, QEMU_OPTION_net,
+ "-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
+ " old way to create a new NIC and connect it to VLAN 'n'\n"
+ " (use the '-device devtype,netdev=str' option if possible instead)\n"
"-net dump[,vlan=n][,file=f][,len=n]\n"
" dump traffic on vlan 'n' to file 'f' (max n bytes per packet)\n"
"-net none use it alone to have zero network devices. If no -net option\n"
- " is provided, the default is '-net nic -net user'\n", QEMU_ARCH_ALL)
-DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
- "-netdev ["
+ " is provided, the default is '-net nic -net user'\n"
+ "-net ["
#ifdef CONFIG_SLIRP
"user|"
#endif
@@ -1551,9 +1564,9 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
#ifdef CONFIG_NETMAP
"netmap|"
#endif
- "vhost-user|"
- "socket|"
- "hubport],id=str[,option][,option][,...]\n", QEMU_ARCH_ALL)
+ "socket][,vlan=n][,option][,option][,...]\n"
+ " old way to initialize a host network interface\n"
+ " (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
STEXI
@item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
@findex -net
From ca7eb1848bb06d9b75784d7760b83c7b0beb1102 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:49 +0200
Subject: [PATCH 02/17] net: Improve error message for -net hubport a bit
Type "hubport" is valid only with -netdev. Unfortunately, that's
detected late and the error message doesn't explain why:
$ qemu-system-i386 -net hubport,id=foo,hubid=0
qemu-system-i386: -net hubport,id=foo,hubid=0: Device 'hubport' could not be initialized
Improve the error message to "Parameter 'type' expects a net type".
Not fixed: -net hubport without the parameters required by -netdev
hubport still asks for those parameters:
$ qemu-system-i386 -net hubport
qemu-system-i386: -net hubport: Parameter 'hubid' is missing
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-2-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/hub.c | 5 +----
net/net.c | 5 +++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/hub.c b/net/hub.c
index 2b60ab9a60..261f8ccc3f 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -286,12 +286,9 @@ int net_init_hubport(const NetClientOptions *opts, const char *name,
const NetdevHubPortOptions *hubport;
assert(opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT);
+ assert(!peer);
hubport = opts->hubport;
- if (peer) {
- return -EINVAL;
- }
-
net_hub_add_port(hubport->hubid, name);
return 0;
}
diff --git a/net/net.c b/net/net.c
index 7427f6a65a..d9aaeb5341 100644
--- a/net/net.c
+++ b/net/net.c
@@ -882,6 +882,11 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
} 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;
+ }
/* missing optional values have been initialized to "all bits zero" */
name = u.net->has_id ? u.net->id : u.net->name;
}
From a30ecde6e795682d1473c45acae66a60a76fca2f Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:50 +0200
Subject: [PATCH 03/17] net: Permit incremental conversion of init functions to
Error
Error reporting for netdev_add is broken: the net_client_init_fun[]
report the actual errors with (at best) error_report(), and their
caller net_client_init1() makes up a generic error on top.
For command line and HMP, this produces an mildly ugly error cascade.
In QMP, the actual errors go to stderr, and the generic error becomes
the command's error reply.
To fix this, we need to convert the net_client_init_fun[] to Error.
To permit fixing them one by one, add an Error ** parameter to the
net_client_init_fun[]. If the call fails without returning an Error,
make up the same generic Error as before. But if it returns one, use
that instead. Since none of them does so far, no functional change.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-3-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/clients.h | 20 ++++++++++----------
net/dump.c | 3 ++-
net/hub.c | 2 +-
net/l2tpv3.c | 5 ++---
net/net.c | 15 +++++++++------
net/netmap.c | 3 ++-
net/slirp.c | 3 ++-
net/socket.c | 3 ++-
net/tap-win32.c | 3 ++-
net/tap.c | 6 ++++--
net/vde.c | 3 ++-
net/vhost-user.c | 3 ++-
12 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/net/clients.h b/net/clients.h
index 2e8fedad8d..d47530e82f 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -28,38 +28,38 @@
#include "qapi-types.h"
int net_init_dump(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#ifdef CONFIG_SLIRP
int net_init_slirp(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif
int net_init_hubport(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_socket(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_tap(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_bridge(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
int net_init_l2tpv3(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#ifdef CONFIG_VDE
int net_init_vde(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif
#ifdef CONFIG_NETMAP
int net_init_netmap(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif
int net_init_vhost_user(const NetClientOptions *opts, const char *name,
- NetClientState *peer);
+ NetClientState *peer, Error **errp);
#endif /* QEMU_NET_CLIENTS_H */
diff --git a/net/dump.c b/net/dump.c
index 9d3a09e334..214e88a768 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -146,8 +146,9 @@ static int net_dump_init(NetClientState *peer, const char *device,
}
int net_init_dump(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
int len;
const char *file;
char def_file[128];
diff --git a/net/hub.c b/net/hub.c
index 261f8ccc3f..3047f12766 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -281,7 +281,7 @@ int net_hub_id_for_client(NetClientState *nc, int *id)
}
int net_init_hubport(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
const NetdevHubPortOptions *hubport;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8c598b09bc..ed395dc126 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -536,10 +536,9 @@ static NetClientInfo net_l2tpv3_info = {
int net_init_l2tpv3(const NetClientOptions *opts,
const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
-
-
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevL2TPv3Options *l2tpv3;
NetL2TPV3State *s;
NetClientState *nc;
diff --git a/net/net.c b/net/net.c
index d9aaeb5341..3295741d1d 100644
--- a/net/net.c
+++ b/net/net.c
@@ -740,8 +740,9 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
}
static int net_init_nic(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
int idx;
NICInfo *nd;
const NetLegacyNicOptions *nic;
@@ -809,7 +810,7 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
const NetClientOptions *opts,
const char *name,
- NetClientState *peer) = {
+ NetClientState *peer, Error **errp) = {
[NET_CLIENT_OPTIONS_KIND_NIC] = net_init_nic,
#ifdef CONFIG_SLIRP
[NET_CLIENT_OPTIONS_KIND_USER] = net_init_slirp,
@@ -902,10 +903,12 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
}
- if (net_client_init_fun[opts->kind](opts, name, peer) < 0) {
- /* TODO push error reporting into init() methods */
- error_set(errp, QERR_DEVICE_INIT_FAILED,
- NetClientOptionsKind_lookup[opts->kind]);
+ 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;
}
}
diff --git a/net/netmap.c b/net/netmap.c
index 0c1772b03f..69300eb1ae 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -446,8 +446,9 @@ static NetClientInfo net_netmap_info = {
* ... -net netmap,ifname="..."
*/
int net_init_netmap(const NetClientOptions *opts,
- const char *name, NetClientState *peer)
+ const char *name, NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevNetmapOptions *netmap_opts = opts->netmap;
NetClientState *nc;
NetmapPriv me;
diff --git a/net/slirp.c b/net/slirp.c
index 9bbed7447a..0e15cf6750 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -737,8 +737,9 @@ static const char **slirp_dnssearch(const StringList *dnsname)
}
int net_init_slirp(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
struct slirp_config_str *config;
char *vnet;
int ret;
diff --git a/net/socket.c b/net/socket.c
index c30e03f5ae..5a19aa1881 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -693,8 +693,9 @@ static int net_socket_udp_init(NetClientState *peer,
}
int net_init_socket(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
Error *err = NULL;
const NetdevSocketOptions *sock;
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 8aee611f7d..f6fc9610a7 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -752,8 +752,9 @@ static int tap_win32_init(NetClientState *peer, const char *model,
}
int net_init_tap(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevTapOptions *tap;
assert(opts->kind == NET_CLIENT_OPTIONS_KIND_TAP);
diff --git a/net/tap.c b/net/tap.c
index 968df46c8c..8f06cb7382 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -531,8 +531,9 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
}
int net_init_bridge(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevBridgeOptions *bridge;
const char *helper, *br;
@@ -699,8 +700,9 @@ static int get_fds(char *str, char *fds[], int max)
}
int net_init_tap(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevTapOptions *tap;
int fd, vnet_hdr = 0, i = 0, queues;
/* for the no-fd, no-helper case */
diff --git a/net/vde.c b/net/vde.c
index 2a619fbc81..dacaa64b47 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -110,8 +110,9 @@ static int net_vde_init(NetClientState *peer, const char *model,
}
int net_init_vde(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevVdeOptions *vde;
assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VDE);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1d86a2be11..11899c53c0 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -223,8 +223,9 @@ static int net_vhost_check_net(QemuOpts *opts, void *opaque)
}
int net_init_vhost_user(const NetClientOptions *opts, const char *name,
- NetClientState *peer)
+ NetClientState *peer, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
const NetdevVhostUserOptions *vhost_user_opts;
CharDriverState *chr;
From 6630886863d4a9b3b7bcb3b0e2895d83eb269c75 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:51 +0200
Subject: [PATCH 04/17] net: Improve -net nic error reporting
When -net nic fails, it first reports a specific error, then a generic
one, like this:
$ qemu-system-x86_64 -net nic,netdev=nonexistent
qemu-system-x86_64: -net nic,netdev=nonexistent: netdev 'nonexistent' not found
qemu-system-x86_64: -net nic,netdev=nonexistent: Device 'nic' could not be initialized
Convert net_init_nic() to Error to get rid of the unwanted second
error message.
While there, tidy up an Overcapitalized Error Message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-4-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/net.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/net.c b/net/net.c
index 3295741d1d..0fe5b8bbd5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -742,7 +742,6 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
static int net_init_nic(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
int idx;
NICInfo *nd;
const NetLegacyNicOptions *nic;
@@ -752,7 +751,7 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
idx = nic_get_free_idx();
if (idx == -1 || nb_nics >= MAX_NICS) {
- error_report("Too Many NICs");
+ error_setg(errp, "too many NICs");
return -1;
}
@@ -763,7 +762,7 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
if (nic->has_netdev) {
nd->netdev = qemu_find_netdev(nic->netdev);
if (!nd->netdev) {
- error_report("netdev '%s' not found", nic->netdev);
+ error_setg(errp, "netdev '%s' not found", nic->netdev);
return -1;
}
} else {
@@ -780,19 +779,20 @@ static int net_init_nic(const NetClientOptions *opts, const char *name,
if (nic->has_macaddr &&
net_parse_macaddr(nd->macaddr.a, nic->macaddr) < 0) {
- error_report("invalid syntax for ethernet address");
+ error_setg(errp, "invalid syntax for ethernet address");
return -1;
}
if (nic->has_macaddr &&
is_multicast_ether_addr(nd->macaddr.a)) {
- error_report("NIC cannot have multicast MAC address (odd 1st byte)");
+ error_setg(errp,
+ "NIC cannot have multicast MAC address (odd 1st byte)");
return -1;
}
qemu_macaddr_default_if_unset(&nd->macaddr);
if (nic->has_vectors) {
if (nic->vectors > 0x7ffffff) {
- error_report("invalid # of vectors: %"PRIu32, nic->vectors);
+ error_setg(errp, "invalid # of vectors: %"PRIu32, nic->vectors);
return -1;
}
nd->nvectors = nic->vectors;
From 3791f83ca999edc2d11eb2006ccc1091cd712c15 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:52 +0200
Subject: [PATCH 05/17] net/dump: Improve -net/host_net_add dump error
reporting
When -net dump fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -net dump,id=foo,file=/eperm
qemu-system-x86_64: -net dump,id=foo,file=/eperm: -net dump: can't open /eperm
qemu-system-x86_64: -net dump,id=foo,file=/eperm: Device 'dump' could not be initialized
Convert net_init_tap() to Error. This suppresses the unwanted second
message.
Improve the error messages to include strerror(errno) where
appropriate.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-5-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/dump.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/dump.c b/net/dump.c
index 214e88a768..02c8064be0 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -101,7 +101,8 @@ static NetClientInfo net_dump_info = {
};
static int net_dump_init(NetClientState *peer, const char *device,
- const char *name, const char *filename, int len)
+ const char *name, const char *filename, int len,
+ Error **errp)
{
struct pcap_file_hdr hdr;
NetClientState *nc;
@@ -111,7 +112,7 @@ static int net_dump_init(NetClientState *peer, const char *device,
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0644);
if (fd < 0) {
- error_report("-net dump: can't open %s", filename);
+ error_setg_errno(errp, errno, "-net dump: can't open %s", filename);
return -1;
}
@@ -124,7 +125,7 @@ static int net_dump_init(NetClientState *peer, const char *device,
hdr.linktype = 1;
if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
- error_report("-net dump write error: %s", strerror(errno));
+ error_setg_errno(errp, errno, "-net dump write error");
close(fd);
return -1;
}
@@ -148,7 +149,6 @@ static int net_dump_init(NetClientState *peer, const char *device,
int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
int len;
const char *file;
char def_file[128];
@@ -174,7 +174,7 @@ int net_init_dump(const NetClientOptions *opts, const char *name,
if (dump->has_len) {
if (dump->len > INT_MAX) {
- error_report("invalid length: %"PRIu64, dump->len);
+ error_setg(errp, "invalid length: %"PRIu64, dump->len);
return -1;
}
len = dump->len;
@@ -182,5 +182,5 @@ int net_init_dump(const NetClientOptions *opts, const char *name,
len = 65536;
}
- return net_dump_init(peer, "dump", name, file, len);
+ return net_dump_init(peer, "dump", name, file, len, errp);
}
From da4a4eac26381c7fce3f147f3c8a7e7bb483be1e Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:53 +0200
Subject: [PATCH 06/17] tap: net_tap_fd_init() can't fail, drop dead error
handling
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-6-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 8f06cb7382..adb1022517 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -536,7 +536,6 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
/* FIXME error_setg(errp, ...) on failure */
const NetdevBridgeOptions *bridge;
const char *helper, *br;
-
TAPState *s;
int fd, vnet_hdr;
@@ -552,14 +551,8 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
}
fcntl(fd, F_SETFL, O_NONBLOCK);
-
vnet_hdr = tap_probe_vnet_hdr(fd);
-
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
- if (!s) {
- close(fd);
- return -1;
- }
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
br);
@@ -607,14 +600,9 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
int vnet_hdr, int fd)
{
Error *err = NULL;
- TAPState *s;
+ TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
- s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
- if (!s) {
- return -1;
- }
-
if (tap_set_sndbuf(s->fd, tap) < 0) {
return -1;
}
From a8a21be9855e0bb0947a7325d0d1741a8814f21e Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:54 +0200
Subject: [PATCH 07/17] tap: Improve -netdev/netdev_add/-net/... bridge error
reporting
When -netdev bridge fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -netdev bridge,id=foo
failed to launch bridge helper
qemu-system-x86_64: -netdev bridge,id=foo: Device 'bridge' could not be initialized
The first message goes to stderr. Wrong for HMP, because errors need
to go to the monitor there.
The second message goes to stderr for -netdev, to the monitor for HMP
netdev_add, and becomes the error reply for QMP netdev_add.
Convert net_bridge_run_helper() to Error, and propagate its errors
through net_init_bridge(). This ensures the error gets reported where
the user is, and suppresses the unwanted second message.
While there, improve the error messages a bit.
The above example becomes:
$ qemu-system-x86_64 -netdev bridge,id=foo
qemu-system-x86_64: -netdev bridge,id=foo: bridge helper failed
net_init_tap() also uses net_bridge_run_helper(). Propagate its
errors there as well. Improves reporting these errors with -netdev
tap & friends.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-7-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index adb1022517..23c81faab5 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -437,7 +437,8 @@ static int recv_fd(int c)
return len;
}
-static int net_bridge_run_helper(const char *helper, const char *bridge)
+static int net_bridge_run_helper(const char *helper, const char *bridge,
+ Error **errp)
{
sigset_t oldmask, mask;
int pid, status;
@@ -450,11 +451,16 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
sigprocmask(SIG_BLOCK, &mask, &oldmask);
if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+ error_setg_errno(errp, errno, "socketpair() failed");
return -1;
}
/* try to launch bridge helper */
pid = fork();
+ if (pid < 0) {
+ error_setg_errno(errp, errno, "Can't fork bridge helper");
+ return -1;
+ }
if (pid == 0) {
int open_max = sysconf(_SC_OPEN_MAX), i;
char fd_buf[6+10];
@@ -502,14 +508,16 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
}
_exit(1);
- } else if (pid > 0) {
+ } else {
int fd;
+ int saved_errno;
close(sv[1]);
do {
fd = recv_fd(sv[0]);
} while (fd == -1 && errno == EINTR);
+ saved_errno = errno;
close(sv[0]);
@@ -518,22 +526,21 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
}
sigprocmask(SIG_SETMASK, &oldmask, NULL);
if (fd < 0) {
- fprintf(stderr, "failed to recv file descriptor\n");
+ error_setg_errno(errp, saved_errno,
+ "failed to recv file descriptor");
return -1;
}
-
- if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
- return fd;
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ error_setg(errp, "bridge helper failed");
+ return -1;
}
+ return fd;
}
- fprintf(stderr, "failed to launch bridge helper\n");
- return -1;
}
int net_init_bridge(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
const NetdevBridgeOptions *bridge;
const char *helper, *br;
TAPState *s;
@@ -545,7 +552,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE;
- fd = net_bridge_run_helper(helper, br);
+ fd = net_bridge_run_helper(helper, br, errp);
if (fd == -1) {
return -1;
}
@@ -792,7 +799,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
return -1;
}
- fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE);
+ fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE,
+ errp);
if (fd == -1) {
return -1;
}
From 80b832c300c2fc39c68e0ab095d408cb9199cfa0 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:55 +0200
Subject: [PATCH 08/17] tap: Convert tap_set_sndbuf() to Error
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-8-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap-aix.c | 3 +--
net/tap-bsd.c | 3 +--
net/tap-haiku.c | 3 +--
net/tap-linux.c | 6 ++----
net/tap-solaris.c | 3 +--
net/tap.c | 4 +++-
net/tap_int.h | 2 +-
7 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/net/tap-aix.c b/net/tap-aix.c
index 804d16448d..0a3d46158f 100644
--- a/net/tap-aix.c
+++ b/net/tap-aix.c
@@ -32,9 +32,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return -1;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
- return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index bf91bd03fd..53cdd9f4e1 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -176,9 +176,8 @@ error:
}
#endif /* __FreeBSD__ */
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
- return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-haiku.c b/net/tap-haiku.c
index e5ce436d24..0905b2887d 100644
--- a/net/tap-haiku.c
+++ b/net/tap-haiku.c
@@ -32,9 +32,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return -1;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
- return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 812bf2dfc6..6fa27442bc 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -126,7 +126,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*/
#define TAP_DEFAULT_SNDBUF 0
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
int sndbuf;
@@ -139,10 +139,8 @@ int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
}
if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
- error_report("TUNSETSNDBUF ioctl failed: %s", strerror(errno));
- return -1;
+ error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
}
- return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 9c7278f1bf..7839323e20 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -198,9 +198,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
return fd;
}
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap)
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
{
- return 0;
}
int tap_probe_vnet_hdr(int fd)
diff --git a/net/tap.c b/net/tap.c
index 23c81faab5..d54222d068 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -610,7 +610,9 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
- if (tap_set_sndbuf(s->fd, tap) < 0) {
+ tap_set_sndbuf(s->fd, tap, &err);
+ if (err) {
+ error_report_err(err);
return -1;
}
diff --git a/net/tap_int.h b/net/tap_int.h
index 79afdf2d58..6df271f823 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -34,7 +34,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
-int tap_set_sndbuf(int fd, const NetdevTapOptions *tap);
+void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
int tap_probe_vnet_hdr(int fd);
int tap_probe_vnet_hdr_len(int fd, int len);
int tap_probe_has_ufo(int fd);
From 445f116cabe0c4435590244741ac3d0b8f08d91d Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:56 +0200
Subject: [PATCH 09/17] tap: Convert net_init_tap_one() to Error
[Dropped %s from "tap: open vhost char device failed: %s" since
error_setg_errno() already prints a human-readable error string and
there is no format string argument.
--Stefan]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-9-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 70 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index d54222d068..49f8586ca5 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -600,11 +600,11 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
#define MAX_TAP_QUEUES 1024
-static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
- const char *model, const char *name,
- const char *ifname, const char *script,
- const char *downscript, const char *vhostfdname,
- int vnet_hdr, int fd)
+static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
+ const char *model, const char *name,
+ const char *ifname, const char *script,
+ const char *downscript, const char *vhostfdname,
+ int vnet_hdr, int fd, Error **errp)
{
Error *err = NULL;
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
@@ -612,8 +612,8 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
tap_set_sndbuf(s->fd, tap, &err);
if (err) {
- error_report_err(err);
- return -1;
+ error_propagate(errp, err);
+ return;
}
if (tap->has_fd || tap->has_fds) {
@@ -644,30 +644,28 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
if (tap->has_vhostfd || tap->has_vhostfds) {
vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
if (vhostfd == -1) {
- error_report_err(err);
- return -1;
+ error_propagate(errp, err);
+ return;
}
} else {
vhostfd = open("/dev/vhost-net", O_RDWR);
if (vhostfd < 0) {
- error_report("tap: open vhost char device failed: %s",
- strerror(errno));
- return -1;
+ error_setg_errno(errp, errno,
+ "tap: open vhost char device failed");
+ return;
}
}
options.opaque = (void *)(uintptr_t)vhostfd;
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
- error_report("vhost-net requested but could not be initialized");
- return -1;
+ error_setg(errp,
+ "vhost-net requested but could not be initialized");
+ return;
}
} else if (tap->has_vhostfd || tap->has_vhostfds) {
- error_report("vhostfd= is not valid without vhost");
- return -1;
+ error_setg(errp, "vhostfd= is not valid without vhost");
}
-
- return 0;
}
static int get_fds(char *str, char *fds[], int max)
@@ -741,9 +739,11 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
vnet_hdr = tap_probe_vnet_hdr(fd);
- if (net_init_tap_one(tap, peer, "tap", name, NULL,
- script, downscript,
- vhostfdname, vnet_hdr, fd)) {
+ net_init_tap_one(tap, peer, "tap", name, NULL,
+ script, downscript,
+ vhostfdname, vnet_hdr, fd, &err);
+ if (err) {
+ error_report_err(err);
return -1;
}
} else if (tap->has_fds) {
@@ -786,10 +786,12 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
return -1;
}
- if (net_init_tap_one(tap, peer, "tap", name, ifname,
- script, downscript,
- tap->has_vhostfds ? vhost_fds[i] : NULL,
- vnet_hdr, fd)) {
+ net_init_tap_one(tap, peer, "tap", name, ifname,
+ script, downscript,
+ tap->has_vhostfds ? vhost_fds[i] : NULL,
+ vnet_hdr, fd, &err);
+ if (err) {
+ error_report_err(err);
return -1;
}
}
@@ -810,9 +812,11 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
fcntl(fd, F_SETFL, O_NONBLOCK);
vnet_hdr = tap_probe_vnet_hdr(fd);
- if (net_init_tap_one(tap, peer, "bridge", name, ifname,
- script, downscript, vhostfdname,
- vnet_hdr, fd)) {
+ net_init_tap_one(tap, peer, "bridge", name, ifname,
+ script, downscript, vhostfdname,
+ vnet_hdr, fd, &err);
+ if (err) {
+ error_report_err(err);
close(fd);
return -1;
}
@@ -846,10 +850,12 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
}
}
- if (net_init_tap_one(tap, peer, "tap", name, ifname,
- i >= 1 ? "no" : script,
- i >= 1 ? "no" : downscript,
- vhostfdname, vnet_hdr, fd)) {
+ net_init_tap_one(tap, peer, "tap", name, ifname,
+ i >= 1 ? "no" : script,
+ i >= 1 ? "no" : downscript,
+ vhostfdname, vnet_hdr, fd, &err);
+ if (err) {
+ error_report_err(err);
close(fd);
return -1;
}
From ac4fcf5639f44f7d863a35eaa2ad07ff31aabc01 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:57 +0200
Subject: [PATCH 10/17] tap: Convert launch_script() to Error
Fixes inappropriate use of stderr in monitor command handler.
While there, improve the messages some.
[Fixed Error **err -> Error *err local variable that broke the build.
--Stefan]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-10-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 49f8586ca5..93075c67e9 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -59,7 +59,8 @@ typedef struct TAPState {
unsigned host_vnet_hdr_len;
} TAPState;
-static int launch_script(const char *setup_script, const char *ifname, int fd);
+static void launch_script(const char *setup_script, const char *ifname,
+ int fd, Error **errp);
static int tap_can_send(void *opaque);
static void tap_send(void *opaque);
@@ -288,6 +289,7 @@ static void tap_set_offload(NetClientState *nc, int csum, int tso4,
static void tap_cleanup(NetClientState *nc)
{
TAPState *s = DO_UPCAST(TAPState, nc, nc);
+ Error *err = NULL;
if (s->vhost_net) {
vhost_net_cleanup(s->vhost_net);
@@ -296,8 +298,12 @@ static void tap_cleanup(NetClientState *nc)
qemu_purge_queued_packets(nc);
- if (s->down_script[0])
- launch_script(s->down_script, s->down_script_arg, s->fd);
+ if (s->down_script[0]) {
+ launch_script(s->down_script, s->down_script_arg, s->fd, &err);
+ if (err) {
+ error_report_err(err);
+ }
+ }
tap_read_poll(s, false);
tap_write_poll(s, false);
@@ -368,7 +374,8 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
return s;
}
-static int launch_script(const char *setup_script, const char *ifname, int fd)
+static void launch_script(const char *setup_script, const char *ifname,
+ int fd, Error **errp)
{
int pid, status;
char *args[3];
@@ -376,6 +383,11 @@ static int launch_script(const char *setup_script, const char *ifname, int fd)
/* try to launch network script */
pid = fork();
+ if (pid < 0) {
+ error_setg_errno(errp, errno, "could not launch network script %s",
+ setup_script);
+ return;
+ }
if (pid == 0) {
int open_max = sysconf(_SC_OPEN_MAX), i;
@@ -390,17 +402,17 @@ static int launch_script(const char *setup_script, const char *ifname, int fd)
*parg = NULL;
execv(setup_script, args);
_exit(1);
- } else if (pid > 0) {
+ } else {
while (waitpid(pid, &status, 0) != pid) {
/* loop */
}
if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
- return 0;
+ return;
}
+ error_setg(errp, "network script %s failed with status %d",
+ setup_script, status);
}
- fprintf(stderr, "%s: could not launch network script\n", setup_script);
- return -1;
}
static int recv_fd(int c)
@@ -571,6 +583,7 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
const char *setup_script, char *ifname,
size_t ifname_sz, int mq_required)
{
+ Error *err = NULL;
int fd, vnet_hdr_required;
if (tap->has_vnet_hdr) {
@@ -589,10 +602,13 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
if (setup_script &&
setup_script[0] != '\0' &&
- strcmp(setup_script, "no") != 0 &&
- launch_script(setup_script, ifname, fd)) {
- close(fd);
- return -1;
+ strcmp(setup_script, "no") != 0) {
+ launch_script(setup_script, ifname, fd, &err);
+ if (err) {
+ error_report_err(err);
+ close(fd);
+ return -1;
+ }
}
return fd;
From 468dd82408e950d48def28f87e4cffabfd592ace Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:58 +0200
Subject: [PATCH 11/17] tap: Permit incremental conversion of tap_open() to
Error
Convert the trivial ones immediately: tap-aix and tap-haiku.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-11-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap-aix.c | 4 ++--
net/tap-bsd.c | 6 ++++--
net/tap-haiku.c | 4 ++--
net/tap-linux.c | 3 ++-
net/tap-solaris.c | 3 ++-
net/tap.c | 13 +++++++++----
net/tap_int.h | 2 +-
7 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/net/tap-aix.c b/net/tap-aix.c
index 0a3d46158f..18fdbf3b21 100644
--- a/net/tap-aix.c
+++ b/net/tap-aix.c
@@ -26,9 +26,9 @@
#include <stdio.h>
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
- fprintf(stderr, "no tap on AIX\n");
+ error_setg(errp, "no tap on AIX");
return -1;
}
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 53cdd9f4e1..bbbe446b1b 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -35,8 +35,9 @@
#ifndef __FreeBSD__
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
int fd;
#ifdef TAPGIFNAME
struct ifreq ifr;
@@ -114,8 +115,9 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
#define PATH_NET_TAP "/dev/tap"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
int fd, s, ret;
struct ifreq ifr;
diff --git a/net/tap-haiku.c b/net/tap-haiku.c
index 0905b2887d..d18590c636 100644
--- a/net/tap-haiku.c
+++ b/net/tap-haiku.c
@@ -26,9 +26,9 @@
#include <stdio.h>
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
- fprintf(stderr, "no tap on Haiku\n");
+ error_setg(errp, "no tap on Haiku");
return -1;
}
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 6fa27442bc..be18382517 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -37,8 +37,9 @@
#define PATH_NET_TUN "/dev/net/tun"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 7839323e20..079046bab8 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -174,8 +174,9 @@ static int tap_alloc(char *dev, size_t dev_size)
}
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required)
+ int vnet_hdr_required, int mq_required, Error **errp)
{
+ /* FIXME error_setg(errp, ...) on failure */
char dev[10]="";
int fd;
if( (fd = tap_alloc(dev, sizeof(dev))) < 0 ){
diff --git a/net/tap.c b/net/tap.c
index 93075c67e9..362df4235f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -581,7 +581,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
const char *setup_script, char *ifname,
- size_t ifname_sz, int mq_required)
+ size_t ifname_sz, int mq_required, Error **errp)
{
Error *err = NULL;
int fd, vnet_hdr_required;
@@ -595,8 +595,12 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
}
TFR(fd = tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
- mq_required));
+ mq_required, errp));
if (fd < 0) {
+ /* FIXME drop when all tap_open() store an Error */
+ if (errp && !*errp) {
+ error_setg(errp, "can't open tap device");
+ }
return -1;
}
@@ -605,7 +609,7 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
strcmp(setup_script, "no") != 0) {
launch_script(setup_script, ifname, fd, &err);
if (err) {
- error_report_err(err);
+ error_propagate(errp, err);
close(fd);
return -1;
}
@@ -853,8 +857,9 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
for (i = 0; i < queues; i++) {
fd = net_tap_init(tap, &vnet_hdr, i >= 1 ? "no" : script,
- ifname, sizeof ifname, queues > 1);
+ ifname, sizeof ifname, queues > 1, &err);
if (fd == -1) {
+ error_report_err(err);
return -1;
}
diff --git a/net/tap_int.h b/net/tap_int.h
index 6df271f823..d12a409967 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -30,7 +30,7 @@
#include "qapi-types.h"
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
- int vnet_hdr_required, int mq_required);
+ int vnet_hdr_required, int mq_required, Error **errp);
ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
From 47896e2fd3dd80685434b320cb0e10164995e31c Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:58:59 +0200
Subject: [PATCH 12/17] tap-linux: Convert tap_open() to Error
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-12-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap-linux.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index be18382517..6c3caef21e 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -39,7 +39,6 @@
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
struct ifreq ifr;
int fd, ret;
int len = sizeof(struct virtio_net_hdr);
@@ -47,7 +46,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
TFR(fd = open(PATH_NET_TUN, O_RDWR));
if (fd < 0) {
- error_report("could not open %s: %m", PATH_NET_TUN);
+ error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
return -1;
}
memset(&ifr, 0, sizeof(ifr));
@@ -71,8 +70,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
if (vnet_hdr_required && !*vnet_hdr) {
- error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
@@ -87,8 +86,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
if (mq_required) {
if (!(features & IFF_MULTI_QUEUE)) {
- error_report("multiqueue required, but no kernel "
- "support for IFF_MULTI_QUEUE available");
+ error_setg(errp, "multiqueue required, but no kernel "
+ "support for IFF_MULTI_QUEUE available");
close(fd);
return -1;
} else {
@@ -103,9 +102,11 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ret = ioctl(fd, TUNSETIFF, (void *) &ifr);
if (ret != 0) {
if (ifname[0] != '\0') {
- error_report("could not configure %s (%s): %m", PATH_NET_TUN, ifr.ifr_name);
+ error_setg_errno(errp, errno, "could not configure %s (%s)",
+ PATH_NET_TUN, ifr.ifr_name);
} else {
- error_report("could not configure %s: %m", PATH_NET_TUN);
+ error_setg_errno(errp, errno, "could not configure %s",
+ PATH_NET_TUN);
}
close(fd);
return -1;
From 4bce487e14bf8949a91883a3213c2b7fa9d668bc Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:59:00 +0200
Subject: [PATCH 13/17] tap-bsd: Convert tap_open() to Error
Fixes inappropriate use of stderr in monitor command handler.
While there, improve some of the messages a bit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-13-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap-bsd.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index bbbe446b1b..5889920eac 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -37,7 +37,6 @@
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
int fd;
#ifdef TAPGIFNAME
struct ifreq ifr;
@@ -72,23 +71,19 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
}
}
if (fd < 0) {
- error_report("warning: could not open %s (%s): no virtual network emulation",
- dname, strerror(errno));
+ error_setg_errno(errp, errno, "could not open %s", dname);
return -1;
}
#ifdef TAPGIFNAME
if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) {
- fprintf(stderr, "warning: could not get tap name: %s\n",
- strerror(errno));
+ error_setg_errno(errp, errno, "could not get tap name");
return -1;
}
pstrcpy(ifname, ifname_size, ifr.ifr_name);
#else
if (fstat(fd, &s) < 0) {
- fprintf(stderr,
- "warning: could not stat /dev/tap: no virtual network emulation: %s\n",
- strerror(errno));
+ error_setg_errno(errp, errno, "could not stat %s", dname);
return -1;
}
dev = devname(s.st_rdev, S_IFCHR);
@@ -100,8 +95,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
- error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
@@ -117,13 +112,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
int fd, s, ret;
struct ifreq ifr;
TFR(fd = open(PATH_NET_TAP, O_RDWR));
if (fd < 0) {
- error_report("could not open %s: %s", PATH_NET_TAP, strerror(errno));
+ error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP);
return -1;
}
@@ -131,7 +125,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
ret = ioctl(fd, TAPGIFNAME, (void *)&ifr);
if (ret < 0) {
- error_report("could not get tap interface name");
+ error_setg_errno(errp, errno, "could not get tap interface name");
goto error;
}
@@ -139,14 +133,15 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
/* User requested the interface to have a specific name */
s = socket(AF_LOCAL, SOCK_DGRAM, 0);
if (s < 0) {
- error_report("could not open socket to set interface name");
+ error_setg_errno(errp, errno,
+ "could not open socket to set interface name");
goto error;
}
ifr.ifr_data = ifname;
ret = ioctl(s, SIOCSIFNAME, (void *)&ifr);
close(s);
if (ret < 0) {
- error_report("could not set tap interface name");
+ error_setg(errp, "could not set tap interface name");
goto error;
}
} else {
@@ -158,14 +153,14 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
- error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
goto error;
}
}
if (mq_required) {
- error_report("mq_required requested, but not kernel support"
- "for IFF_MULTI_QUEUE available");
+ error_setg(errp, "mq_required requested, but no kernel support"
+ " for IFF_MULTI_QUEUE available");
goto error;
}
From 576c6eb6700d241c9d4a6883d25720c7bbaaeccd Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:59:01 +0200
Subject: [PATCH 14/17] tap-solaris: Convert tap_open() to Error
Fixes inappropriate use of syslog().
Not fixed: leaks on error paths, suspicious non-fatal errors. FIXMEs
added instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-14-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap-solaris.c | 59 ++++++++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 079046bab8..90b2fd12f1 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -36,7 +36,6 @@
#include <netinet/udp.h>
#include <netinet/tcp.h>
#include <net/if.h>
-#include <syslog.h>
#include <stropts.h>
#include "qemu/error-report.h"
@@ -56,8 +55,10 @@ ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen)
* Allocate TAP device, returns opened fd.
* Stores dev name in the first arg(must be large enough).
*/
-static int tap_alloc(char *dev, size_t dev_size)
+static int tap_alloc(char *dev, size_t dev_size, Error **errp)
{
+ /* FIXME leaks like a sieve on error paths */
+ /* FIXME suspicious: many errors are reported, then ignored */
int tap_fd, if_fd, ppa = -1;
static int ip_fd = 0;
char *ptr;
@@ -83,14 +84,14 @@ static int tap_alloc(char *dev, size_t dev_size)
TFR(ip_fd = open("/dev/udp", O_RDWR, 0));
if (ip_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/ip (actually /dev/udp)");
- return -1;
+ error_setg(errp, "Can't open /dev/ip (actually /dev/udp)");
+ return -1;
}
TFR(tap_fd = open("/dev/tap", O_RDWR, 0));
if (tap_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/tap");
- return -1;
+ error_setg(errp, "Can't open /dev/tap");
+ return -1;
}
/* Assign a new PPA and get its unit number. */
@@ -99,20 +100,20 @@ static int tap_alloc(char *dev, size_t dev_size)
strioc_ppa.ic_len = sizeof(ppa);
strioc_ppa.ic_dp = (char *)&ppa;
if ((ppa = ioctl (tap_fd, I_STR, &strioc_ppa)) < 0)
- syslog (LOG_ERR, "Can't assign new interface");
+ error_report("Can't assign new interface");
TFR(if_fd = open("/dev/tap", O_RDWR, 0));
if (if_fd < 0) {
- syslog(LOG_ERR, "Can't open /dev/tap (2)");
- return -1;
+ error_setg(errp, "Can't open /dev/tap (2)");
+ return -1;
}
if(ioctl(if_fd, I_PUSH, "ip") < 0){
- syslog(LOG_ERR, "Can't push IP module");
- return -1;
+ error_setg(errp, "Can't push IP module");
+ return -1;
}
if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0)
- syslog(LOG_ERR, "Can't get flags\n");
+ error_report("Can't get flags");
snprintf (actual_name, 32, "tap%d", ppa);
pstrcpy(ifr.lifr_name, sizeof(ifr.lifr_name), actual_name);
@@ -121,22 +122,22 @@ static int tap_alloc(char *dev, size_t dev_size)
/* Assign ppa according to the unit number returned by tun device */
if (ioctl (if_fd, SIOCSLIFNAME, &ifr) < 0)
- syslog (LOG_ERR, "Can't set PPA %d", ppa);
+ error_report("Can't set PPA %d", ppa);
if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) <0)
- syslog (LOG_ERR, "Can't get flags\n");
+ error_report("Can't get flags");
/* Push arp module to if_fd */
if (ioctl (if_fd, I_PUSH, "arp") < 0)
- syslog (LOG_ERR, "Can't push ARP module (2)");
+ error_report("Can't push ARP module (2)");
/* Push arp module to ip_fd */
if (ioctl (ip_fd, I_POP, NULL) < 0)
- syslog (LOG_ERR, "I_POP failed\n");
+ error_report("I_POP failed");
if (ioctl (ip_fd, I_PUSH, "arp") < 0)
- syslog (LOG_ERR, "Can't push ARP module (3)\n");
+ error_report("Can't push ARP module (3)");
/* Open arp_fd */
TFR(arp_fd = open ("/dev/tap", O_RDWR, 0));
if (arp_fd < 0)
- syslog (LOG_ERR, "Can't open %s\n", "/dev/tap");
+ error_report("Can't open %s", "/dev/tap");
/* Set ifname to arp */
strioc_if.ic_cmd = SIOCSLIFNAME;
@@ -144,16 +145,16 @@ static int tap_alloc(char *dev, size_t dev_size)
strioc_if.ic_len = sizeof(ifr);
strioc_if.ic_dp = (char *)𝔦
if (ioctl(arp_fd, I_STR, &strioc_if) < 0){
- syslog (LOG_ERR, "Can't set ifname to arp\n");
+ error_report("Can't set ifname to arp");
}
if((ip_muxid = ioctl(ip_fd, I_LINK, if_fd)) < 0){
- syslog(LOG_ERR, "Can't link TAP device to IP");
- return -1;
+ error_setg(errp, "Can't link TAP device to IP");
+ return -1;
}
if ((arp_muxid = ioctl (ip_fd, link_type, arp_fd)) < 0)
- syslog (LOG_ERR, "Can't link TAP device to ARP");
+ error_report("Can't link TAP device to ARP");
close (if_fd);
@@ -166,7 +167,7 @@ static int tap_alloc(char *dev, size_t dev_size)
{
ioctl (ip_fd, I_PUNLINK , arp_muxid);
ioctl (ip_fd, I_PUNLINK, ip_muxid);
- syslog (LOG_ERR, "Can't set multiplexor id");
+ error_report("Can't set multiplexor id");
}
snprintf(dev, dev_size, "tap%d", ppa);
@@ -176,12 +177,12 @@ static int tap_alloc(char *dev, size_t dev_size)
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
char dev[10]="";
int fd;
- if( (fd = tap_alloc(dev, sizeof(dev))) < 0 ){
- fprintf(stderr, "Cannot allocate TAP device\n");
- return -1;
+
+ fd = tap_alloc(dev, sizeof(dev), errp);
+ if (fd < 0) {
+ return -1;
}
pstrcpy(ifname, ifname_size, dev);
if (*vnet_hdr) {
@@ -189,8 +190,8 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
*vnet_hdr = 0;
if (vnet_hdr_required && !*vnet_hdr) {
- error_report("vnet_hdr=1 requested, but no kernel "
- "support for IFF_VNET_HDR available");
+ error_setg(errp, "vnet_hdr=1 requested, but no kernel "
+ "support for IFF_VNET_HDR available");
close(fd);
return -1;
}
From 95c35a74fea51e307f6a3967e465a22776056b7e Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:59:02 +0200
Subject: [PATCH 15/17] tap: Finish conversion of tap_open() to Error
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-15-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 362df4235f..c1ee4f1233 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -597,10 +597,6 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
TFR(fd = tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
mq_required, errp));
if (fd < 0) {
- /* FIXME drop when all tap_open() store an Error */
- if (errp && !*errp) {
- error_setg(errp, "can't open tap device");
- }
return -1;
}
From a308817743be5cc051d3379477f54027deb0befb Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 15 May 2015 13:59:03 +0200
Subject: [PATCH 16/17] tap: Improve -netdev/netdev_add/-net/... tap error
reporting
When -netdev tap fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -netdev tap,id=foo
qemu-system-x86_64: -netdev tap,id=foo: could not configure /dev/net/tun: Operation not permitted
qemu-system-x86_64: -netdev tap,id=foo: Device 'tap' could not be initialized
With the command line, the messages go to stderr. In HMP, they go to
the monitor. In QMP, the second one becomes the error reply, and the
first one goes to stderr.
Convert net_init_tap() to Error. This suppresses the unwanted second
message, and makes the specific error the QMP error reply.
[Dropped duplicate "and" from error message as suggested by Eric Blake:
"ifname=, script=, downscript=, and vnet_hdr=, "
"queues=, and vhostfds= are invalid with helper="
--Stefan]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1431691143-1015-16-git-send-email-armbru@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/tap.c | 45 ++++++++++++++++++++++-----------------------
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index c1ee4f1233..d1ca314dcf 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -713,7 +713,6 @@ static int get_fds(char *str, char *fds[], int max)
int net_init_tap(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
{
- /* FIXME error_setg(errp, ...) on failure */
const NetdevTapOptions *tap;
int fd, vnet_hdr = 0, i = 0, queues;
/* for the no-fd, no-helper case */
@@ -731,7 +730,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
/* QEMU vlans does not support multiqueue tap, in this case peer is set.
* For -netdev, peer is always NULL. */
if (peer && (tap->has_queues || tap->has_fds || tap->has_vhostfds)) {
- error_report("Multiqueue tap cannot be used with QEMU vlans");
+ error_setg(errp, "Multiqueue tap cannot be used with QEMU vlans");
return -1;
}
@@ -739,15 +738,15 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
tap->has_fds || tap->has_vhostfds) {
- error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, fds=, and vhostfds= "
- "are invalid with fd=");
+ error_setg(errp, "ifname=, script=, downscript=, vnet_hdr=, "
+ "helper=, queues=, fds=, and vhostfds= "
+ "are invalid with fd=");
return -1;
}
fd = monitor_fd_param(cur_mon, tap->fd, &err);
if (fd == -1) {
- error_report_err(err);
+ error_propagate(errp, err);
return -1;
}
@@ -759,7 +758,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
script, downscript,
vhostfdname, vnet_hdr, fd, &err);
if (err) {
- error_report_err(err);
+ error_propagate(errp, err);
return -1;
}
} else if (tap->has_fds) {
@@ -770,9 +769,9 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
tap->has_vhostfd) {
- error_report("ifname=, script=, downscript=, vnet_hdr=, "
- "helper=, queues=, and vhostfd= "
- "are invalid with fds=");
+ error_setg(errp, "ifname=, script=, downscript=, vnet_hdr=, "
+ "helper=, queues=, and vhostfd= "
+ "are invalid with fds=");
return -1;
}
@@ -780,8 +779,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
if (tap->has_vhostfds) {
nvhosts = get_fds(tap->vhostfds, vhost_fds, MAX_TAP_QUEUES);
if (nfds != nvhosts) {
- error_report("The number of fds passed does not match the "
- "number of vhostfds passed");
+ error_setg(errp, "The number of fds passed does not match "
+ "the number of vhostfds passed");
return -1;
}
}
@@ -789,7 +788,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
for (i = 0; i < nfds; i++) {
fd = monitor_fd_param(cur_mon, fds[i], &err);
if (fd == -1) {
- error_report_err(err);
+ error_propagate(errp, err);
return -1;
}
@@ -798,7 +797,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
if (i == 0) {
vnet_hdr = tap_probe_vnet_hdr(fd);
} else if (vnet_hdr != tap_probe_vnet_hdr(fd)) {
- error_report("vnet_hdr not consistent across given tap fds");
+ error_setg(errp,
+ "vnet_hdr not consistent across given tap fds");
return -1;
}
@@ -807,15 +807,15 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
tap->has_vhostfds ? vhost_fds[i] : NULL,
vnet_hdr, fd, &err);
if (err) {
- error_report_err(err);
+ error_propagate(errp, err);
return -1;
}
}
} else if (tap->has_helper) {
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) {
- error_report("ifname=, script=, downscript=, and vnet_hdr= "
- "queues=, and vhostfds= are invalid with helper=");
+ error_setg(errp, "ifname=, script=, downscript=, vnet_hdr=, "
+ "queues=, and vhostfds= are invalid with helper=");
return -1;
}
@@ -832,13 +832,13 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
script, downscript, vhostfdname,
vnet_hdr, fd, &err);
if (err) {
- error_report_err(err);
+ error_propagate(errp, err);
close(fd);
return -1;
}
} else {
if (tap->has_vhostfds) {
- error_report("vhostfds= is invalid if fds= wasn't specified");
+ error_setg(errp, "vhostfds= is invalid if fds= wasn't specified");
return -1;
}
script = tap->has_script ? tap->script : DEFAULT_NETWORK_SCRIPT;
@@ -853,15 +853,14 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
for (i = 0; i < queues; i++) {
fd = net_tap_init(tap, &vnet_hdr, i >= 1 ? "no" : script,
- ifname, sizeof ifname, queues > 1, &err);
+ ifname, sizeof ifname, queues > 1, errp);
if (fd == -1) {
- error_report_err(err);
return -1;
}
if (queues > 1 && i == 0 && !tap->has_ifname) {
if (tap_fd_get_ifname(fd, ifname)) {
- error_report("Fail to get ifname");
+ error_setg(errp, "Fail to get ifname");
close(fd);
return -1;
}
@@ -872,7 +871,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
i >= 1 ? "no" : downscript,
vhostfdname, vnet_hdr, fd, &err);
if (err) {
- error_report_err(err);
+ error_propagate(errp, err);
close(fd);
return -1;
}
From 2bc22a58e16f0650e56dccfac9495e5aef58e2ef Mon Sep 17 00:00:00 2001
From: Shannon Zhao <shannon.zhao@linaro.org>
Date: Thu, 21 May 2015 17:44:48 +0800
Subject: [PATCH 17/17] net/net: Record usage status of mac address
Currently QEMU dynamically generates mac address for the NIC which
doesn't specify the mac address. But when we hotplug a NIC without
specifying mac address, the mac address will increase for the same NIC
along with hotplug and hot-unplug, and at last it will overflow. And if
we codeplug one NIC with mac address e.g. "52:54:00:12:34:56", then
hotplug one NIC without specifying mac address and the mac address of
the hotplugged NIC is duplicate of "52:54:00:12:34:56".
This patch add a mac_table to record the usage status and free the mac
address when the NIC is unrealized.
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/net.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 55 insertions(+), 4 deletions(-)
diff --git a/net/net.c b/net/net.c
index 0fe5b8bbd5..db6be12a1e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -167,19 +167,68 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6])
macaddr[3], macaddr[4], macaddr[5]);
}
+static int mac_table[256] = {0};
+
+static void qemu_macaddr_set_used(MACAddr *macaddr)
+{
+ int index;
+
+ for (index = 0x56; index < 0xFF; index++) {
+ if (macaddr->a[5] == index) {
+ mac_table[index]++;
+ }
+ }
+}
+
+static void qemu_macaddr_set_free(MACAddr *macaddr)
+{
+ int index;
+ static const MACAddr base = { .a = { 0x52, 0x54, 0x00, 0x12, 0x34, 0 } };
+
+ if (memcmp(macaddr->a, &base.a, (sizeof(base.a) - 1)) != 0) {
+ return;
+ }
+ for (index = 0x56; index < 0xFF; index++) {
+ if (macaddr->a[5] == index) {
+ mac_table[index]--;
+ }
+ }
+}
+
+static int qemu_macaddr_get_free(void)
+{
+ int index;
+
+ for (index = 0x56; index < 0xFF; index++) {
+ if (mac_table[index] == 0) {
+ return index;
+ }
+ }
+
+ return -1;
+}
+
void qemu_macaddr_default_if_unset(MACAddr *macaddr)
{
- static int index = 0;
static const MACAddr zero = { .a = { 0,0,0,0,0,0 } };
+ static const MACAddr base = { .a = { 0x52, 0x54, 0x00, 0x12, 0x34, 0 } };
+
+ if (memcmp(macaddr, &zero, sizeof(zero)) != 0) {
+ if (memcmp(macaddr->a, &base.a, (sizeof(base.a) - 1)) != 0) {
+ return;
+ } else {
+ qemu_macaddr_set_used(macaddr);
+ return;
+ }
+ }
- if (memcmp(macaddr, &zero, sizeof(zero)) != 0)
- return;
macaddr->a[0] = 0x52;
macaddr->a[1] = 0x54;
macaddr->a[2] = 0x00;
macaddr->a[3] = 0x12;
macaddr->a[4] = 0x34;
- macaddr->a[5] = 0x56 + index++;
+ macaddr->a[5] = qemu_macaddr_get_free();
+ qemu_macaddr_set_used(macaddr);
}
/**
@@ -374,6 +423,8 @@ void qemu_del_nic(NICState *nic)
{
int i, queues = MAX(nic->conf->peers.queues, 1);
+ qemu_macaddr_set_free(&nic->conf->macaddr);
+
/* If this is a peer NIC and peer has already been deleted, free it now. */
if (nic->peer_deleted) {
for (i = 0; i < queues; i++) {