12:58 p.m.
brSetInetAddress can only set a single IP address on the bridge, and
uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace
it and brSetInetNetmask with a single function that uses the external
"ip addr add" command to add an address/prefix to the interface - this
supports IPv6, and allows adding multiple addresses to the interface.
Although it isn't currently used in the code, we also add a
brDelInetAddress for completeness' sake.
Also, while we're modifying bridge.c, we change brSetForwardDelay and
brSetEnableSTP to use the new virCommand API rather than the
deprecated virRun, and also log an error message in bridge_driver.c if
either of those fail (previously the failure would be completely
silent).
NB: it's a bit bothersome that there is no difference in error
reporting between being unable to run one of these commands, and the
command returning a non-0 exit status, but there is no precedent in
bridge.c for logging error messages locally rather than pushing them
up the call chain, and what's pushed up is only 'success' or
'failure'; no exit codes. Perhaps a non-0 exit could be passed up
directly as the return, other errors could return -1, and callers
could check for ret != 0 rather than ret < 0?
---
V2 Changes:
* Any prefix arg is now unsigned int rather than int.
configure.ac | 3 +
src/libvirt_bridge.syms | 3 +-
src/network/bridge_driver.c | 50 ++++++++-------
src/util/bridge.c | 141 +++++++++++++++++++++++++------------------
src/util/bridge.h | 10 ++-
5 files changed, 118 insertions(+), 89 deletions(-)
diff --git a/configure.ac b/configure.ac
index 73c1176..4db613a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,6 +306,9 @@ if test x"$with_rhel5_api" = x"yes"; then
AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API])
fi
+AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH])
+AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary])
+
AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables
binary])
diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms
index 2658291..c3773bd 100644
--- a/src/libvirt_bridge.syms
+++ b/src/libvirt_bridge.syms
@@ -6,15 +6,16 @@
# bridge.h
brAddBridge;
+brAddInetAddress;
brAddInterface;
brAddTap;
brDeleteTap;
brDeleteBridge;
+brDelInetAddress;
brHasBridge;
brInit;
brSetEnableSTP;
brSetForwardDelay;
-brSetInetAddress;
brSetInetNetmask;
brSetInterfaceUp;
brShutdown;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 41c8478..ba9ff96 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1142,37 +1142,39 @@ static int networkStartNetworkDaemon(struct network_driver
*driver,
if (networkDisableIPV6(network) < 0)
goto err_delbr;
- if (brSetForwardDelay(driver->brctl, network->def->bridge,
network->def->delay) < 0)
- goto err_delbr;
-
- if (brSetEnableSTP(driver->brctl, network->def->bridge,
network->def->stp ? 1 : 0) < 0)
- goto err_delbr;
-
- if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress) &&
- (err = brSetInetAddress(driver->brctl, network->def->bridge,
- &network->def->ipAddress))) {
- virReportSystemError(err,
- _("cannot set IP address on bridge
'%s'"),
- network->def->bridge);
+ if ((err = brSetForwardDelay(driver->brctl, network->def->bridge,
+ network->def->delay))) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot set forward delay on bridge
'%s'"),
+ network->def->bridge);
goto err_delbr;
}
- virSocketAddr netmask;
-
- if (virNetworkDefNetmask(network->def, &netmask) < 0) {
-
+ if ((err = brSetEnableSTP(driver->brctl, network->def->bridge,
+ network->def->stp ? 1 : 0))) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("bridge '%s' has an invalid netmask or IP
address"),
- network->def->bridge);
+ _("cannot set STP '%s' on bridge
'%s'"),
+ network->def->stp ? "on" : "off",
network->def->bridge);
goto err_delbr;
}
- if ((err = brSetInetNetmask(driver->brctl, network->def->bridge,
- &netmask))) {
- virReportSystemError(err,
- _("cannot set netmask on bridge '%s'"),
- network->def->bridge);
- goto err_delbr;
+ if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) {
+ int prefix = virNetworkDefPrefix(network->def);
+
+ if (prefix < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP
address"),
+ network->def->bridge);
+ goto err_delbr;
+ }
+
+ if ((err = brAddInetAddress(driver->brctl, network->def->bridge,
+ &network->def->ipAddress, prefix))) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot set IP address on bridge
'%s'"),
+ network->def->bridge);
+ goto err_delbr;
+ }
}
if ((err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) {
diff --git a/src/util/bridge.c b/src/util/bridge.c
index 97cf73f..dcd3f39 100644
--- a/src/util/bridge.c
+++ b/src/util/bridge.c
@@ -46,6 +46,7 @@
# include <net/if_arp.h> /* ARPHRD_ETHER */
# include "internal.h"
+# include "command.h"
# include "memory.h"
# include "util.h"
# include "logging.h"
@@ -655,73 +656,84 @@ brGetInterfaceUp(brControl *ctl,
return 0;
}
-static int
-brSetInetAddr(brControl *ctl,
- const char *ifname,
- int cmd,
- virSocketAddr *addr)
-{
- struct ifreq ifr;
-
- if (!ctl || !ctl->fd || !ifname || !addr)
- return EINVAL;
-
- memset(&ifr, 0, sizeof(struct ifreq));
-
- if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
- return EINVAL;
-
- if (!VIR_SOCKET_IS_FAMILY(addr, AF_INET))
- return EINVAL;
-
- ifr.ifr_addr = addr->data.sa;
-
- if (ioctl(ctl->fd, cmd, &ifr) < 0)
- return errno;
-
- return 0;
-}
-
/**
- * brSetInetAddress:
+ * brAddInetAddress:
* @ctl: bridge control pointer
* @ifname: the interface name
- * @addr: the string representation of the IP address
+ * @addr: the IP address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
*
- * Function to bind the interface to an IP address, it should handle
- * IPV4 and IPv6. The string for addr would be of the form
- * "ddd.ddd.ddd.ddd" assuming the common IPv4 format.
+ * Add an IP address to an interface. This function *does not* remove
+ * any previously added IP addresses - that must be done separately with
+ * brDelInetAddress.
*
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 in case of error.
*/
int
-brSetInetAddress(brControl *ctl,
+brAddInetAddress(brControl *ctl ATTRIBUTE_UNUSED,
const char *ifname,
- virSocketAddr *addr)
+ virSocketAddr *addr,
+ unsigned int prefix)
{
- return brSetInetAddr(ctl, ifname, SIOCSIFADDR, addr);
+ virCommandPtr cmd;
+ char *addrstr;
+ int ret = -1;
+
+ if (!(addrstr = virSocketFormatAddr(addr)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "addr", "add", NULL);
+ virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
+ virCommandAddArgList(cmd, "dev", ifname, NULL);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ virCommandFree(cmd);
+ return ret;
}
/**
- * brSetInetNetmask:
+ * brDelInetAddress:
* @ctl: bridge control pointer
* @ifname: the interface name
- * @addr: the string representation of the netmask
+ * @addr: the IP address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
*
- * Function to set the netmask of an interface, it should handle
- * IPV4 and IPv6 forms. The string for addr would be of the form
- * "ddd.ddd.ddd.ddd" assuming the common IPv4 format.
+ * Delete an IP address from an interface.
*
- * Returns 0 in case of success or an errno code in case of failure.
+ * Returns 0 in case of success or -1 in case of error.
*/
int
-brSetInetNetmask(brControl *ctl,
+brDelInetAddress(brControl *ctl ATTRIBUTE_UNUSED,
const char *ifname,
- virSocketAddr *addr)
+ virSocketAddr *addr,
+ unsigned int prefix)
{
- return brSetInetAddr(ctl, ifname, SIOCSIFNETMASK, addr);
+ virCommandPtr cmd;
+ char *addrstr;
+ int ret = -1;
+
+ if (!(addrstr = virSocketFormatAddr(addr)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "addr", "del", NULL);
+ virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
+ virCommandAddArgList(cmd, "dev", ifname, NULL);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ virCommandFree(cmd);
+ return ret;
}
/**
@@ -740,17 +752,20 @@ brSetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED,
const char *bridge,
int delay)
{
- char delayStr[30];
- const char *const progargv[] = {
- BRCTL, "setfd", bridge, delayStr, NULL
- };
+ virCommandPtr cmd;
+ int ret = -1;
- snprintf(delayStr, sizeof(delayStr), "%d", delay);
+ cmd = virCommandNew(BRCTL);
+ virCommandAddArgList(cmd, "setfd", bridge, NULL);
+ virCommandAddArgFormat(cmd, "%d", delay);
- if (virRun(progargv, NULL) < 0)
- return -1;
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
- return 0;
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
}
/**
@@ -769,15 +784,21 @@ brSetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED,
const char *bridge,
int enable)
{
- const char *setting = enable ? "on" : "off";
- const char *const progargv[] = {
- BRCTL, "stp", bridge, setting, NULL
- };
+ virCommandPtr cmd;
+ int ret = -1;
- if (virRun(progargv, NULL) < 0)
- return -1;
+ cmd = virCommandNew(BRCTL);
+ virCommandAddArgList(cmd, "stp", bridge,
+ enable ? "on" : "off",
+ NULL);
- return 0;
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
}
#endif /* WITH_BRIDGE */
diff --git a/src/util/bridge.h b/src/util/bridge.h
index 3ef38d9..e8e7eca 100644
--- a/src/util/bridge.h
+++ b/src/util/bridge.h
@@ -83,12 +83,14 @@ int brGetInterfaceUp (brControl *ctl,
const char *ifname,
int *up);
-int brSetInetAddress (brControl *ctl,
+int brAddInetAddress (brControl *ctl,
const char *ifname,
- virSocketAddr *addr);
-int brSetInetNetmask (brControl *ctl,
+ virSocketAddr *addr,
+ unsigned int prefix);
+int brDelInetAddress (brControl *ctl,
const char *ifname,
- virSocketAddr *addr);
+ virSocketAddr *addr,
+ unsigned int prefix);
int brSetForwardDelay (brControl *ctl,
const char *bridge,
--
1.7.3.4