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

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

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) {
+        if (brAddInterface(bridge, parentVeth) < 0)
-            virReportSystemError(ret,
-                                 _("Failed to add %s device to %s"),
-                                 parentVeth, bridge);
             goto error_exit;
-        }
 
         if (vethInterfaceUpOrDown(parentVeth, 1) < 0)
             goto error_exit;

src/network/bridge_driver.c

@@ -1678,12 +1678,8 @@ networkAddAddrToBridge(virNetworkObjPtr network,
     }
 
     if (brAddInetAddress(network->def->bridge,
-                         &ipdef->address, prefix) < 0) {
+                         &ipdef->address, prefix) < 0)
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set IP address on bridge '%s'"),
-                           network->def->bridge);
         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))) {
+    if (brAddBridge(network->def->bridge) < 0)
-        virReportSystemError(err,
-                             _("cannot create bridge '%s'"),
-                             network->def->bridge);
         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,
+        if (brAddTap(network->def->bridge,
-                            &macTapIfName, network->def->mac, 0, false, NULL))) {
+                     &macTapIfName, network->def->mac, 0, false, NULL) < 0) {
-            virReportSystemError(err,
-                                 _("cannot create dummy tap device '%s' to set mac"
-                                   " address on bridge '%s'"),
-                                 macTapIfName, network->def->bridge);
             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)) {
+                          network->def->delay) < 0)
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set forward delay on bridge '%s'"),
-                           network->def->bridge);
         goto err1;
-    }
 
     if (brSetEnableSTP(network->def->bridge,
-                       network->def->stp ? 1 : 0)) {
+                       network->def->stp ? 1 : 0) < 0)
-        networkReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot set STP '%s' on bridge '%s'"),
-                           network->def->stp ? "on" : "off", network->def->bridge);
         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))) {
+    if (brSetInterfaceUp(network->def->bridge, 1) < 0)
-        virReportSystemError(err,
-                             _("failed to bring the bridge '%s' up"),
-                             network->def->bridge);
         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))) {
+    ignore_value(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));
-    }
 
  err2:
     if (!save_err)
@@ -1843,22 +1815,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
     if (!save_err)
         save_err = virSaveLastError();
 
-    if ((err = brDeleteTap(macTapIfName))) {
+    ignore_value(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));
-    }
     VIR_FREE(macTapIfName);
 
  err0:
     if (!save_err)
         save_err = virSaveLastError();
-    if ((err = brDeleteBridge(network->def->bridge))) {
+    ignore_value(brDeleteBridge(network->def->bridge));
-        char ebuf[1024];
-        VIR_WARN("Failed to delete bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
 
     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))) {
+            ignore_value(brDeleteTap(macTapIfName));
-                VIR_WARN("Failed to delete dummy tap device '%s' on bridge '%s' : %s",
-                         macTapIfName, network->def->bridge,
-                         virStrerror(err, ebuf, sizeof ebuf));
-            }
             VIR_FREE(macTapIfName);
         }
     }
 
-    if ((err = brSetInterfaceUp(network->def->bridge, 0))) {
+    ignore_value(brSetInterfaceUp(network->def->bridge, 0));
-        VIR_WARN("Failed to bring down bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
 
     networkRemoveIptablesRules(driver, network);
 
-    if ((err = brDeleteBridge(network->def->bridge))) {
+    ignore_value(brDeleteBridge(network->def->bridge));
-        VIR_WARN("Failed to delete bridge '%s' : %s",
-                 network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
-    }
 
     /* See if its still alive and really really kill it */
     if (network->dnsmasqPid > 0 &&

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 < 0) {
-        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 (template_ifname)
             VIR_FREE(net->ifname);
         tapfd = -1;

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,
+    if (brAddTap(bridge, &net->ifname, tapmac,
-                        &net->ifname,
+                 0, true, NULL) < 0) {
-                        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 (template_ifname)
             VIR_FREE(net->ifname);
         goto error;

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);
+        ignore_value(brDeleteTap(def->ifname));
-        err = brDeleteTap(def->ifname);
-        if (err) {
-            VIR_ERROR(_("Cleanup failed %d"), err);
-            ret = -1;
-        }
     }
-    VIR_ERROR(_("Cleanup tap done"));
-    return ret;
 }
 
 static int umlStartVMDaemon(virConnectPtr conn,

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,
+brAddInterface(const char *brname,
-               const char *ifname ATTRIBUTE_UNUSED)
+               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,
+brDeleteInterface(const char *brname,
-                  const char *ifname ATTRIBUTE_UNUSED)
+                  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.
+ * Returns the MTU value in case of success, or -1 on failure.
- * On error, returns -1 and sets errno accordingly
  */
 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);
+    if (brCreateTap(ifname, vnet_hdr, tapfd) < 0)
-
+        return -1;
-    /* fd has been closed in brCreateTap() when it failed. */
-    if (errno)
-        goto error;
 
     /* 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)))
+    if (brSetInterfaceMac(*ifname, macaddr) < 0)
-        goto close_fd;
+        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)))
+    if (brSetInterfaceMtu(brname, *ifname) < 0)
-        goto close_fd;
+        goto error;
-    if ((errno = brAddInterface(brname, *ifname)))
+
-        goto close_fd;
+    if (brAddInterface(brname, *ifname) < 0)
-    if (up && ((errno = brSetInterfaceUp(*ifname, 1))))
+        goto error;
-        goto close_fd;
+
+    if (brSetInterfaceUp(*ifname, up) < 0)
+        goto error;
 
     return 0;
 
-close_fd:
+error:
     if (tapfd)
         VIR_FORCE_CLOSE(*tapfd);
-error:
+    return -1;
-    return errno;
 }
 
 int brDeleteTap(const char *ifname)
 {
     struct ifreq try;
     int fd;
+    int ret = -1;
 
-    if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
+    if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
-        return errno;
+        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;
+        virReportSystemError(ERANGE,
-        goto error;
+                             _("Network interface name '%s' is too long"),
+                             ifname);
+        goto cleanup;
     }
 
-    if (ioctl(fd, TUNSETIFF, &try) == 0) {
+    if (ioctl(fd, TUNSETIFF, &try) < 0) {
-        if ((errno = ioctl(fd, TUNSETPERSIST, 0)))
+        virReportSystemError(errno, "%s",
-            goto error;
+                             _("Unable to associate TAP device"));
+        goto cleanup;
     }
 
- error:
+    if (ioctl(fd, TUNSETPERSIST, 0) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to make TAP device non-persistent"));
+        goto cleanup;
+    }
+
+    ret = 0;
+
+cleanup:
     VIR_FORCE_CLOSE(fd);
-
+    return ret;
-    return errno;
 }
 
 
@@ -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)
+    if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
-        return errno;
+        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;
+        virReportSystemError(ERANGE,
-        goto error;
+                             _("Network interface name '%s' is too long"),
+                             *ifname);
+        goto cleanup;
+
     }
 
-    if (ioctl(fd, TUNSETIFF, &ifr) < 0)
+    if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
-        goto error;
+        virReportSystemError(errno,
+                             _("Unable to create tap device %s"),
+                             NULLSTR(*ifname));
+        goto cleanup;
+    }
 
     if (!tapfd &&
-        (errno = ioctl(fd, TUNSETPERSIST, 1)))
+        (errno = ioctl(fd, TUNSETPERSIST, 1))) {
-        goto error;
+        virReportSystemError(errno,
+                             _("Unable to set tap device %s to persistent"),
+                             NULLSTR(*ifname));
+        goto cleanup;
+    }
+
     VIR_FREE(*ifname);
-    if (!(*ifname = strdup(ifr.ifr_name)))
+    if (!(*ifname = strdup(ifr.ifr_name))) {
-        goto error;
+        virReportOOMError();
-    if(tapfd)
+        goto cleanup;
+    }
+    if (tapfd)
         *tapfd = fd;
     else
         VIR_FORCE_CLOSE(fd);
-    return 0;
 
- error:
+    ret = 0;
-    VIR_FORCE_CLOSE(fd);
 
-    return errno;
+cleanup:
+    if (ret < 0)
+        VIR_FORCE_CLOSE(fd);
+
+    return ret;
 }
 
 #endif /* WITH_BRIDGE */