Make all brXXX APIs raise errors, instead of returning errnos

Currently every caller of the brXXX APIs has to store the returned
errno value and then raise an error message. This results in
inconsistent error messages across drivers, additional burden on
the callers and makes the error reporting inaccurate since it is
hard to distinguish different scenarios from 1 errno value.

* src/util/bridge.c: Raise errors instead of returning errnos
* src/lxc/lxc_driver.c, src/network/bridge_driver.c,
  src/qemu/qemu_command.c, src/uml/uml_conf.c,
  src/uml/uml_driver.c: Remove error reporting code

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 5ce35ae60fe35525edafe62855dd13fc6ae85aec..bd1d7bd0c6132fd1ae59bced6d31875a7801b4b6 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -102,6 +102,7 @@ src/test/test_driver.c
 src/uml/uml_conf.c
 src/uml/uml_driver.c
 src/util/authhelper.c
+src/util/bridge.c
 src/util/cgroup.c
 src/util/command.c
 src/util/conf.c

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1187719aa87874c98a2c8809c33b73ddfd283a28..2f26011c9a39479d3a66c805e739b52658a4b88f 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1194,7 +1194,6 @@ static int lxcSetupInterfaces(virConnectPtr conn,
 {
     int rc = -1, i;
     char *bridge = NULL;
-    int ret;
 
     for (i = 0 ; i < def->nnets ; i++) {
         char *parentVeth;
@@ -1270,12 +1269,8 @@ static int lxcSetupInterfaces(virConnectPtr conn,
                 goto error_exit;
         }
 
-        if ((ret = brAddInterface(bridge, parentVeth)) != 0) {
-            virReportSystemError(ret,
-                                 _("Failed to add %s device to %s"),
-                                 parentVeth, bridge);
+        if (brAddInterface(bridge, parentVeth) < 0)
             goto error_exit;
-        }
 
         if (vethInterfaceUpOrDown(parentVeth, 1) < 0)
             goto error_exit;

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 053acfd537b15844476bfd4a2892ab97e554b0af..d5d2472f346400fcb1d36470110b6ccb11c34a8f 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1678,12 +1678,8 @@ networkAddAddrToBridge(virNetworkObjPtr network,
     }
 
     if (brAddInetAddress(network->def->bridge,
-                         &ipdef->address, prefix) < 0) {
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set IP address on bridge '%s'"),
-                           network->def->bridge);
+                         &ipdef->address, prefix) < 0)
         return -1;
-    }
 
     return 0;
 }
@@ -1692,7 +1688,7 @@ static int
 networkStartNetworkVirtual(struct network_driver *driver,
                           virNetworkObjPtr network)
 {
-    int ii, err;
+    int ii;
     bool v4present = false, v6present = false;
     virErrorPtr save_err = NULL;
     virNetworkIpDefPtr ipdef;
@@ -1703,12 +1699,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
         return -1;
 
     /* Create and configure the bridge device */
-    if ((err = brAddBridge(network->def->bridge))) {
-        virReportSystemError(err,
-                             _("cannot create bridge '%s'"),
-                             network->def->bridge);
+    if (brAddBridge(network->def->bridge) < 0)
         return -1;
-    }
 
     if (network->def->mac_specified) {
         /* To set a mac for the bridge, we need to define a dummy tap
@@ -1722,12 +1714,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
             virReportOOMError();
             goto err0;
         }
-        if ((err = brAddTap(network->def->bridge,
-                            &macTapIfName, network->def->mac, 0, false, NULL))) {
-            virReportSystemError(err,
-                                 _("cannot create dummy tap device '%s' to set mac"
-                                   " address on bridge '%s'"),
-                                 macTapIfName, network->def->bridge);
+        if (brAddTap(network->def->bridge,
+                     &macTapIfName, network->def->mac, 0, false, NULL) < 0) {
             VIR_FREE(macTapIfName);
             goto err0;
         }
@@ -1735,20 +1723,12 @@ networkStartNetworkVirtual(struct network_driver *driver,
 
     /* Set bridge options */
     if (brSetForwardDelay(network->def->bridge,
-                          network->def->delay)) {
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set forward delay on bridge '%s'"),
-                           network->def->bridge);
+                          network->def->delay) < 0)
         goto err1;
-    }
 
     if (brSetEnableSTP(network->def->bridge,
-                       network->def->stp ? 1 : 0)) {
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set STP '%s' on bridge '%s'"),
-                           network->def->stp ? "on" : "off", network->def->bridge);
+                       network->def->stp ? 1 : 0) < 0)
         goto err1;
-    }
 
     /* Disable IPv6 on the bridge if there are no IPv6 addresses
      * defined, and set other IPv6 sysctl tunables appropriately.
@@ -1775,12 +1755,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
     }
 
     /* Bring up the bridge interface */
-    if ((err = brSetInterfaceUp(network->def->bridge, 1))) {
-        virReportSystemError(err,
-                             _("failed to bring the bridge '%s' up"),
-                             network->def->bridge);
+    if (brSetInterfaceUp(network->def->bridge, 1) < 0)
         goto err2;
-    }
 
     /* If forwardType != NONE, turn on global IP forwarding */
     if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
@@ -1828,11 +1804,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
  err3:
     if (!save_err)
         save_err = virSaveLastError();
-    if ((err = brSetInterfaceUp(network->def->bridge, 0))) {
-        char ebuf[1024];
-        VIR_WARN("Failed to bring down bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
+    ignore_value(brSetInterfaceUp(network->def->bridge, 0));
 
  err2:
     if (!save_err)
@@ -1843,22 +1815,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
     if (!save_err)
         save_err = virSaveLastError();
 
-    if ((err = brDeleteTap(macTapIfName))) {
-        char ebuf[1024];
-        VIR_WARN("Failed to delete dummy tap device '%s' on bridge '%s' : %s",
-                 macTapIfName, network->def->bridge,
-                 virStrerror(err, ebuf, sizeof ebuf));
-    }
+    ignore_value(brDeleteTap(macTapIfName));
     VIR_FREE(macTapIfName);
 
  err0:
     if (!save_err)
         save_err = virSaveLastError();
-    if ((err = brDeleteBridge(network->def->bridge))) {
-        char ebuf[1024];
-        VIR_WARN("Failed to delete bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
+    ignore_value(brDeleteBridge(network->def->bridge));
 
     if (save_err) {
         virSetError(save_err);
@@ -1870,9 +1833,6 @@ networkStartNetworkVirtual(struct network_driver *driver,
 static int networkShutdownNetworkVirtual(struct network_driver *driver,
                                         virNetworkObjPtr network)
 {
-    int err;
-    char ebuf[1024];
-
     if (virBandwidthDisable(network->def->bridge, true) < 0) {
         VIR_WARN("Failed to disable QoS on %s",
                  network->def->name);
@@ -1899,26 +1859,16 @@ static int networkShutdownNetworkVirtual(struct network_driver *driver,
         if (!macTapIfName) {
             virReportOOMError();
         } else {
-            if ((err = brDeleteTap(macTapIfName))) {
-                VIR_WARN("Failed to delete dummy tap device '%s' on bridge '%s' : %s",
-                         macTapIfName, network->def->bridge,
-                         virStrerror(err, ebuf, sizeof ebuf));
-            }
+            ignore_value(brDeleteTap(macTapIfName));
             VIR_FREE(macTapIfName);
         }
     }
 
-    if ((err = brSetInterfaceUp(network->def->bridge, 0))) {
-        VIR_WARN("Failed to bring down bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
+    ignore_value(brSetInterfaceUp(network->def->bridge, 0));
 
     networkRemoveIptablesRules(driver, network);
 
-    if ((err = brDeleteBridge(network->def->bridge))) {
-        VIR_WARN("Failed to delete bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
+    ignore_value(brDeleteBridge(network->def->bridge));
 
     /* See if its still alive and really really kill it */
     if (network->dnsmasqPid > 0 &&

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ee18858e08cb666db776f60ceccbadab91c08169..6fbdc20ed4824e4959aeadf285a96235eed84fe5 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -282,28 +282,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
     err = brAddTap(brname, &net->ifname, tapmac,
                    vnet_hdr, true, &tapfd);
     virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);
-    if (err) {
-        if (err == ENOTSUP) {
-            /* In this particular case, give a better diagnostic. */
-            qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                            _("Failed to add tap interface to bridge. "
-                              "%s is not a bridge device"), brname);
-        } else if (err == ENOENT) {
-            /* When the tun drive is missing, give a better message. */
-            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                            _("Failed to add tap interface to bridge. "
-                              "Your kernel is missing the 'tun' module or "
-                              "CONFIG_TUN, or you need to add the "
-                              "/dev/net/tun device node."));
-        } else if (template_ifname) {
-            virReportSystemError(err,
-                                 _("Failed to add tap interface to bridge '%s'"),
-                                 brname);
-        } else {
-            virReportSystemError(err,
-                                 _("Failed to add tap interface '%s' to bridge '%s'"),
-                                 net->ifname, brname);
-        }
+    if (err < 0) {
         if (template_ifname)
             VIR_FREE(net->ifname);
         tapfd = -1;

diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 92d132b93a40188d80ea4d1e4fcd491eb747255d..477a178c77b2c82545891ea1751a741c30567134 100644 (file)
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -122,7 +122,6 @@ umlConnectTapDevice(virConnectPtr conn,
                     const char *bridge)
 {
     bool template_ifname = false;
-    int err;
     unsigned char tapmac[VIR_MAC_BUFLEN];
 
     if (!net->ifname ||
@@ -137,31 +136,8 @@ umlConnectTapDevice(virConnectPtr conn,
 
     memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
     tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
-    if ((err = brAddTap(bridge,
-                        &net->ifname,
-                        tapmac,
-                        0,
-                        true,
-                        NULL))) {
-        if (err == ENOTSUP) {
-            /* In this particular case, give a better diagnostic. */
-            umlReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Failed to add tap interface to bridge. "
-                             "%s is not a bridge device"), bridge);
-        } else if (err == ENOENT) {
-            virReportSystemError(err, "%s",
-                    _("Failed to add tap interface to bridge. Your kernel "
-                      "is missing the 'tun' module or CONFIG_TUN, or you need "
-                      "to add the /dev/net/tun device node."));
-        } else if (template_ifname) {
-            virReportSystemError(err,
-                                 _("Failed to add tap interface to bridge '%s'"),
-                                 bridge);
-        } else {
-            virReportSystemError(err,
-                                 _("Failed to add tap interface '%s' to bridge '%s'"),
-                                 net->ifname, bridge);
-        }
+    if (brAddTap(bridge, &net->ifname, tapmac,
+                 0, true, NULL) < 0) {
         if (template_ifname)
             VIR_FREE(net->ifname);
         goto error;

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index b87fa60f0779263f016fc125af19992144cc9fc8..8ba06a570bfbc6b071e05d49731b5bfc6ccd80ac 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -952,11 +952,8 @@ error:
 }
 
 
-static int umlCleanupTapDevices(virDomainObjPtr vm) {
+static void umlCleanupTapDevices(virDomainObjPtr vm) {
     int i;
-    int err;
-    int ret = 0;
-    VIR_ERROR(_("Cleanup tap"));
 
     for (i = 0 ; i < vm->def->nnets ; i++) {
         virDomainNetDefPtr def = vm->def->nets[i];
@@ -965,15 +962,8 @@ static int umlCleanupTapDevices(virDomainObjPtr vm) {
             def->type != VIR_DOMAIN_NET_TYPE_NETWORK)
             continue;
 
-        VIR_ERROR(_("Cleanup '%s'"), def->ifname);
-        err = brDeleteTap(def->ifname);
-        if (err) {
-            VIR_ERROR(_("Cleanup failed %d"), err);
-            ret = -1;
-        }
+        ignore_value(brDeleteTap(def->ifname));
     }
-    VIR_ERROR(_("Cleanup tap done"));
-    return ret;
 }
 
 static int umlStartVMDaemon(virConnectPtr conn,

diff --git a/src/util/bridge.c b/src/util/bridge.c
index ae1d6011a62ea68fa024b7034daca2b3806ff65a..794a7fff672e023c08f2fb995ab2520d4bf3c3f5 100644 (file)
--- a/src/util/bridge.c
+++ b/src/util/bridge.c
@@ -51,10 +51,12 @@
 # include "util.h"
 # include "logging.h"
 # include "network.h"
+# include "virterror_internal.h"
 
 # define JIFFIES_TO_MS(j) (((j)*1000)/HZ)
 # define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000)
 
+# define VIR_FROM_THIS VIR_FROM_NONE
 
 static int brSetupControlFull(const char *ifname,
                               struct ifreq *ifr,
@@ -67,15 +69,22 @@ static int brSetupControlFull(const char *ifname,
         memset(ifr, 0, sizeof(*ifr));
 
         if (virStrcpyStatic(ifr->ifr_name, ifname) == NULL) {
-            errno = EINVAL;
+            virReportSystemError(ERANGE,
+                                 _("Network interface name '%s' is too long"),
+                                 ifname);
             return -1;
         }
     }
 
-    if ((fd = socket(domain, type, 0)) < 0)
+    if ((fd = socket(domain, type, 0)) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Cannot open network interface control socket"));
         return -1;
+    }
 
     if (virSetInherit(fd, false) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Cannot set close-on-exec flag for socket"));
         VIR_FORCE_CLOSE(fd);
         return -1;
     }
@@ -97,7 +106,7 @@ static int brSetupControl(const char *ifname,
  *
  * This function register a new bridge
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 on failure
  */
 # ifdef SIOCBRADDBR
 int
@@ -107,10 +116,11 @@ brAddBridge(const char *brname)
     int ret = -1;
 
     if ((fd = brSetupControl(NULL, NULL)) < 0)
-        return errno;
+        return -1;
 
     if (ioctl(fd, SIOCBRADDBR, brname) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Unable to create bridge %s"), brname);
         goto cleanup;
     }
 
@@ -121,13 +131,23 @@ cleanup:
     return ret;
 }
 # else
-int brAddBridge (const char *brname ATTRIBUTE_UNUSED)
+int brAddBridge(const char *brname)
 {
-    return EINVAL;
+    virReportSystemError(ENOSYS,
+                         _("Unable to create bridge %s"), brname);
+    return -1;
 }
 # endif
 
 # ifdef SIOCBRDELBR
+/**
+ * brHasBridge:
+ * @brname
+ *
+ * Check if the bridge @brname exists
+ *
+ * Returns 1 if it exists, 0 if it does not, -1 on error
+ */
 int
 brHasBridge(const char *brname)
 {
@@ -136,12 +156,18 @@ brHasBridge(const char *brname)
     struct ifreq ifr;
 
     if ((fd = brSetupControl(brname, &ifr)) < 0)
-        return errno;
+        return -1;
 
-    if (ioctl(fd, SIOCGIFFLAGS, &ifr))
+    if (ioctl(fd, SIOCGIFFLAGS, &ifr)) {
+        if (errno == ENODEV)
+            ret = 0;
+        else
+            virReportSystemError(errno,
+                                 _("Unable to get bridge %s flags"), brname);
         goto cleanup;
+    }
 
-    ret = 0;
+    ret = 1;
 
 cleanup:
     VIR_FORCE_CLOSE(fd);
@@ -149,9 +175,11 @@ cleanup:
 }
 # else
 int
-brHasBridge(const char *brname ATTRIBUTE_UNUSED)
+brHasBridge(const char *brname)
 {
-    return EINVAL;
+    virReportSystemError(ENOSYS,
+                         _("Unable to check bridge %s"), brname);
+    return -1;
 }
 # endif
 
@@ -171,10 +199,11 @@ brDeleteBridge(const char *brname)
     int ret = -1;
 
     if ((fd = brSetupControl(NULL, NULL)) < 0)
-        return errno;
+        return -1;
 
     if (ioctl(fd, SIOCBRDELBR, brname) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Unable to delete bridge %s"), brname);
         goto cleanup;
     }
 
@@ -188,7 +217,9 @@ cleanup:
 int
 brDeleteBridge(const char *brname ATTRIBUTE_UNUSED)
 {
-    return EINVAL;
+    virReportSystemError(ENOSYS,
+                         _("Unable to delete bridge %s"), brname);
+    return -1;
 }
 # endif
 
@@ -211,15 +242,17 @@ brAddInterface(const char *brname,
     struct ifreq ifr;
 
     if ((fd = brSetupControl(brname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     if (!(ifr.ifr_ifindex = if_nametoindex(ifname))) {
-        ret = errno;
+        virReportSystemError(ENODEV,
+                             _("Unable to get interface index for %s"), ifname);
         goto cleanup;
     }
 
     if (ioctl(fd, SIOCBRADDIF, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Unable to add bridge %s port %s"), brname, ifname);
         goto cleanup;
     }
 
@@ -230,10 +263,12 @@ cleanup:
 }
 # else
 int
-brAddInterface(const char *brname ATTRIBUTE_UNUSED,
-               const char *ifname ATTRIBUTE_UNUSED)
+brAddInterface(const char *brname,
+               const char *ifname)
 {
-    return EINVAL;
+    virReportSystemError(ENOSYS,
+                         _("Unable to add bridge %s port %s"), brname, ifname);
+    return -1;
 }
 # endif
 
@@ -256,15 +291,18 @@ brDeleteInterface(const char *brname,
     struct ifreq ifr;
 
     if ((fd = brSetupControl(brname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     if (!(ifr.ifr_ifindex = if_nametoindex(ifname))) {
-        ret = errno;
+        virReportSystemError(ENODEV,
+                             _("Unable to get interface index for %s"), ifname);
+
         goto cleanup;
     }
 
     if (ioctl(fd, SIOCBRDELIF, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Unable to remove bridge %s port %s"), brname, ifname);
         goto cleanup;
     }
 
@@ -275,10 +313,12 @@ cleanup:
 }
 # else
 int
-brDeleteInterface(const char *brname ATTRIBUTE_UNUSED,
-                  const char *ifname ATTRIBUTE_UNUSED)
+brDeleteInterface(const char *brname,
+                  const char *ifname)
 {
-    return EINVAL;
+    virReportSystemError(errno,
+                         _("Unable to remove bridge %s port %s"), brname, ifname);
+    return -1;
 }
 # endif
 
@@ -290,7 +330,7 @@ brDeleteInterface(const char *brname ATTRIBUTE_UNUSED,
  * This function sets the @macaddr for a given interface @ifname. This
  * gets rid of the kernel's automatically assigned random MAC.
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 on failure
  */
 int
 brSetInterfaceMac(const char *ifname,
@@ -301,18 +341,22 @@ brSetInterfaceMac(const char *ifname,
     struct ifreq ifr;
 
     if ((fd = brSetupControl(ifname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     /* To fill ifr.ifr_hdaddr.sa_family field */
     if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Cannot get interface MAC on '%s'"),
+                             ifname);
         goto cleanup;
     }
 
     memcpy(ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN);
 
     if (ioctl(fd, SIOCSIFHWADDR, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Cannot set interface MAC on '%s'"),
+                             ifname);
         goto cleanup;
     }
 
@@ -329,8 +373,7 @@ cleanup:
  *
  * This function gets the @mtu value set for a given interface @ifname.
  *
- * Returns the MTU value in case of success.
- * On error, returns -1 and sets errno accordingly
+ * Returns the MTU value in case of success, or -1 on failure.
  */
 static int ifGetMtu(const char *ifname)
 {
@@ -341,8 +384,12 @@ static int ifGetMtu(const char *ifname)
     if ((fd = brSetupControl(ifname, &ifr)) < 0)
         return -1;
 
-    if (ioctl(fd, SIOCGIFMTU, &ifr) < 0)
+    if (ioctl(fd, SIOCGIFMTU, &ifr) < 0) {
+        virReportSystemError(errno,
+                             _("Cannot get interface MTU on '%s'"),
+                             ifname);
         goto cleanup;
+    }
 
     ret = ifr.ifr_mtu;
 
@@ -359,7 +406,7 @@ cleanup:
  * This function sets the @mtu for a given interface @ifname.  Typically
  * used on a tap device to set up for Jumbo Frames.
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success, or -1 on failure
  */
 static int ifSetMtu(const char *ifname, int mtu)
 {
@@ -368,12 +415,14 @@ static int ifSetMtu(const char *ifname, int mtu)
     struct ifreq ifr;
 
     if ((fd = brSetupControl(ifname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     ifr.ifr_mtu = mtu;
 
     if (ioctl(fd, SIOCSIFMTU, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Cannot set interface MTU on '%s'"),
+                             ifname);
         goto cleanup;
     }
 
@@ -391,7 +440,7 @@ cleanup:
  *
  * Sets the interface mtu to the same MTU of the bridge
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success, or -1 on failure
  */
 static int brSetInterfaceMtu(const char *brname,
                              const char *ifname)
@@ -399,7 +448,7 @@ static int brSetInterfaceMtu(const char *brname,
     int mtu = ifGetMtu(brname);
 
     if (mtu < 0)
-        return errno;
+        return -1;
 
     return ifSetMtu(ifname, mtu);
 }
@@ -421,7 +470,7 @@ static int brSetInterfaceMtu(const char *brname,
  * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
  * the supplied tapfd.
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 1 if VnetHdr is supported, 0 if not supported
  */
 # ifdef IFF_VNET_HDR
 static int
@@ -479,7 +528,7 @@ brProbeVnetHdr(int tapfd)
  * persistent and closed. The caller must use brDeleteTap to remove
  * a persistent TAP devices when it is no longer needed.
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 on failure
  */
 int
 brAddTap(const char *brname,
@@ -489,11 +538,8 @@ brAddTap(const char *brname,
          bool up,
          int *tapfd)
 {
-    errno = brCreateTap(ifname, vnet_hdr, tapfd);
-
-    /* fd has been closed in brCreateTap() when it failed. */
-    if (errno)
-        goto error;
+    if (brCreateTap(ifname, vnet_hdr, tapfd) < 0)
+        return -1;
 
     /* We need to set the interface MAC before adding it
      * to the bridge, because the bridge assumes the lowest
@@ -501,53 +547,69 @@ brAddTap(const char *brname,
      * seeing the kernel allocate random MAC for the TAP
      * device before we set our static MAC.
      */
-    if ((errno = brSetInterfaceMac(*ifname, macaddr)))
-        goto close_fd;
+    if (brSetInterfaceMac(*ifname, macaddr) < 0)
+        goto error;
+
     /* We need to set the interface MTU before adding it
      * to the bridge, because the bridge will have its
      * MTU adjusted automatically when we add the new interface.
      */
-    if ((errno = brSetInterfaceMtu(brname, *ifname)))
-        goto close_fd;
-    if ((errno = brAddInterface(brname, *ifname)))
-        goto close_fd;
-    if (up && ((errno = brSetInterfaceUp(*ifname, 1))))
-        goto close_fd;
+    if (brSetInterfaceMtu(brname, *ifname) < 0)
+        goto error;
+
+    if (brAddInterface(brname, *ifname) < 0)
+        goto error;
+
+    if (brSetInterfaceUp(*ifname, up) < 0)
+        goto error;
 
     return 0;
 
-close_fd:
+error:
     if (tapfd)
         VIR_FORCE_CLOSE(*tapfd);
-error:
-    return errno;
+    return -1;
 }
 
 int brDeleteTap(const char *ifname)
 {
     struct ifreq try;
     int fd;
+    int ret = -1;
 
-    if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
-        return errno;
+    if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to open /dev/net/tun, is tun module loaded?"));
+        return -1;
+    }
 
     memset(&try, 0, sizeof(struct ifreq));
     try.ifr_flags = IFF_TAP|IFF_NO_PI;
 
     if (virStrcpyStatic(try.ifr_name, ifname) == NULL) {
-        errno = EINVAL;
-        goto error;
+        virReportSystemError(ERANGE,
+                             _("Network interface name '%s' is too long"),
+                             ifname);
+        goto cleanup;
     }
 
-    if (ioctl(fd, TUNSETIFF, &try) == 0) {
-        if ((errno = ioctl(fd, TUNSETPERSIST, 0)))
-            goto error;
+    if (ioctl(fd, TUNSETIFF, &try) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to associate TAP device"));
+        goto cleanup;
     }
 
- error:
-    VIR_FORCE_CLOSE(fd);
+    if (ioctl(fd, TUNSETPERSIST, 0) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to make TAP device non-persistent"));
+        goto cleanup;
+    }
 
-    return errno;
+    ret = 0;
+
+cleanup:
+    VIR_FORCE_CLOSE(fd);
+    return ret;
 }
 
 
@@ -558,7 +620,7 @@ int brDeleteTap(const char *ifname)
  *
  * Function to control if an interface is activated (up, 1) or not (down, 0)
  *
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 on error.
  */
 int
 brSetInterfaceUp(const char *ifname,
@@ -570,10 +632,12 @@ brSetInterfaceUp(const char *ifname,
     int ifflags;
 
     if ((fd = brSetupControl(ifname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Cannot get interface flags on '%s'"),
+                             ifname);
         goto cleanup;
     }
 
@@ -585,7 +649,9 @@ brSetInterfaceUp(const char *ifname,
     if (ifr.ifr_flags != ifflags) {
         ifr.ifr_flags = ifflags;
         if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
-            ret = errno;
+            virReportSystemError(errno,
+                                 _("Cannot set interface flags on '%s'"),
+                                 ifname);
             goto cleanup;
         }
     }
@@ -615,10 +681,12 @@ brGetInterfaceUp(const char *ifname,
     struct ifreq ifr;
 
     if ((fd = brSetupControl(ifname, &ifr)) < 0)
-        return errno;
+        return -1;
 
     if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
-        ret = errno;
+        virReportSystemError(errno,
+                             _("Cannot get interface flags on '%s'"),
+                             ifname);
         goto cleanup;
     }
 
@@ -799,9 +867,13 @@ brCreateTap(char **ifname,
 {
     int fd;
     struct ifreq ifr;
+    int ret = -1;
 
-    if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
-        return errno;
+    if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to open /dev/net/tun, is tun module loaded?"));
+        return -1;
+    }
 
     memset(&ifr, 0, sizeof(ifr));
 
@@ -813,29 +885,45 @@ brCreateTap(char **ifname,
 # endif
 
     if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
-        errno = EINVAL;
-        goto error;
+        virReportSystemError(ERANGE,
+                             _("Network interface name '%s' is too long"),
+                             *ifname);
+        goto cleanup;
+
     }
 
-    if (ioctl(fd, TUNSETIFF, &ifr) < 0)
-        goto error;
+    if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to create tap device %s"),
+                             NULLSTR(*ifname));
+        goto cleanup;
+    }
 
     if (!tapfd &&
-        (errno = ioctl(fd, TUNSETPERSIST, 1)))
-        goto error;
+        (errno = ioctl(fd, TUNSETPERSIST, 1))) {
+        virReportSystemError(errno,
+                             _("Unable to set tap device %s to persistent"),
+                             NULLSTR(*ifname));
+        goto cleanup;
+    }
+
     VIR_FREE(*ifname);
-    if (!(*ifname = strdup(ifr.ifr_name)))
-        goto error;
-    if(tapfd)
+    if (!(*ifname = strdup(ifr.ifr_name))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (tapfd)
         *tapfd = fd;
     else
         VIR_FORCE_CLOSE(fd);
-    return 0;
 
- error:
-    VIR_FORCE_CLOSE(fd);
+    ret = 0;
 
-    return errno;
+cleanup:
+    if (ret < 0)
+        VIR_FORCE_CLOSE(fd);
+
+    return ret;
 }
 
 #endif /* WITH_BRIDGE */