Re: [libvirt] [PATCH 02/33] Make all brXXX APIs raise errors, instead of returning errnos

From: "Daniel P. Berrange&quot;&lt;berrange(a)redhat.com&gt; 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 | 1 + src/lxc/lxc_driver.c | 7 +- src/network/bridge_driver.c | 78 +++----------- src/qemu/qemu_command.c | 23 +---- src/uml/uml_conf.c | 28 +----- src/uml/uml_driver.c | 14 +-- src/util/bridge.c | 262 +++++++++++++++++++++++++++++-------------- 7 files changed, 196 insertions(+), 217 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 5ce35ae..bd1d7bd 100644 --- 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 2505efc..8ee1306 100644 --- 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 053acfd..d5d2472 100644 --- 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 ee18858..6fbdc20 100644 --- 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 92d132b..477a178 100644 --- 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 b87fa60..8ba06a5 100644 --- 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 ae1d601..351e482 100644 --- 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 delete bridge %s"), 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(errno,

+ _("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,6 +217,8 @@ cleanup: int brDeleteBridge(const char *brname ATTRIBUTE_UNUSED) { + virReportSystemError(errno, + _("Unable to delete bridge %s"), brname);

return EINVAL; } # 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(errno, + _("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(errno, + _("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; + } + + ret = 0; - return errno; +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 */