Re: [libvirt] [PATCH 07/33] Remove usage of brctl command line tool

From: "Daniel P. Berrange&quot;&lt;berrange(a)redhat.com&gt; Convert the virNetDevBridgeSetSTP and virNetDevBridgeSetSTPDelay to use ioctls instead of spawning brctl. Implement the virNetDevBridgeGetSTP and virNetDevBridgeGetSTPDelay methods which were declared in the header but never existed * src/util/bridge.c: Convert to use bridge ioctls instead of brctl --- configure.ac | 2 - libvirt.spec.in | 2 - src/util/bridge.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 184 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index 5753c08..0ce020d 100644 --- a/configure.ac +++ b/configure.ac @@ -197,8 +197,6 @@ AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"], [Location or name of the dnsmasq program]) AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) -AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"], - [Location or name of the brctl program (see bridge-utils)]) AC_DEFINE_UNQUOTED([TC],["$TC"], [Location or name of the tc profram (see iproute2)]) if test -n "$UDEVADM"; then diff --git a/libvirt.spec.in b/libvirt.spec.in index 261f34c..eda0bcc 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -251,7 +251,6 @@ Requires: %{name}-client = %{version}-%{release} # Used by many of the drivers, so turn it on whenever the # daemon is present %if %{with_libvirtd} -Requires: bridge-utils # for modprobe of pci devices Requires: module-init-tools # for /sbin/ip& /sbin/tc @@ -380,7 +379,6 @@ BuildRequires: radvd %if %{with_nwfilter} BuildRequires: ebtables %endif -BuildRequires: bridge-utils BuildRequires: module-init-tools %if %{with_sasl} BuildRequires: cyrus-sasl-devel diff --git a/src/util/bridge.c b/src/util/bridge.c index 3a7012c..0265e81 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -52,6 +52,7 @@ # include "logging.h" # include "network.h" # include "virterror_internal.h" +# include "intprops.h" # define JIFFIES_TO_MS(j) (((j)*1000)/HZ) # define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000) @@ -99,6 +100,112 @@ static int virNetDevSetupControl(const char *ifname, return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM); } +# define SYSFS_NET_DIR "/sys/class/net" +/* + * Bridge parameters can be set via sysfs on newish kernels, + * or by ioctl on older kernels. Perhaps we could just use + * ioctl for every kernel, but its not clear what the long + * term lifespan of the ioctl interface is... + */ +static int virNetDevBridgeSet(const char *brname, + const char *paramname, /* sysfs param name */ + unsigned long value, /* new value */ + int fd, /* control socket */ + struct ifreq *ifr) /* pre-filled bridge name */ +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname)< 0) { + virReportOOMError(); + return -1; + } + + if (virFileExists(path)) { + char valuestr[INT_BUFSIZE_BOUND(value)]; + snprintf(valuestr, sizeof(valuestr), "%lu", value); + if (virFileWriteStr(path, valuestr, 0)< 0) { + virReportSystemError(errno, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + } else { + unsigned long paramid; + if (STREQ(paramname, "stp_state")) { + paramid = BRCTL_SET_BRIDGE_STP_STATE; + } else if (STREQ(paramname, "forward_delay")) { + paramid = BRCTL_SET_BRIDGE_FORWARD_DELAY; + } else { + virReportSystemError(EINVAL, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + unsigned long args[] = { paramid, value, 0, 0 }; + ifr->ifr_data = (char*)&args; + if (ioctl(fd, SIOCDEVPRIVATE, ifr)< 0) { + virReportSystemError(errno, + _("Unable to set bridge %s %s"), brname, paramname); + goto cleanup; + } + } + + ret = 0; +cleanup: + VIR_FREE(path); + return ret; +} + + +static int virNetDevBridgeGet(const char *brname, + const char *paramname, /* sysfs param name */ + unsigned long *value, /* current value */ + int fd, /* control socket */ + struct ifreq *ifr) /* pre-filled bridge name */ +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname)< 0) { + virReportOOMError(); + return -1; + } + + if (virFileExists(path)) { + char *valuestr; + if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long),&valuestr)< 0) + goto cleanup; + + if (virStrToLong_ul(valuestr, NULL, 10, value)< 0) { + virReportSystemError(EINVAL, + _("Unable to get bridge %s %s"), brname, paramname); + } + } else { + struct __bridge_info info; + unsigned long args[] = { BRCTL_GET_BRIDGE_INFO, (unsigned long)&info, 0, 0 }; + ifr->ifr_data = (char*)&args; + if (ioctl(fd, SIOCDEVPRIVATE, ifr)< 0) { + virReportSystemError(errno, + _("Unable to get bridge %s %s"), brname, paramname); + goto cleanup; + } + + if (STREQ(paramname, "stp_state")) { + *value = info.stp_enabled; + } else if (STREQ(paramname, "forward_delay")) { + *value = info.forward_delay; + } else { + virReportSystemError(EINVAL, + _("Unable to get bridge %s %s"), brname, paramname); + goto cleanup; + } + } + + ret = 0; +cleanup: + VIR_FREE(path); + return ret; +} + /** * virNetDevBridgeCreate: @@ -817,22 +924,54 @@ cleanup: int virNetDevBridgeSetSTPDelay(const char *brname, int delay) { - virCommandPtr cmd; + int fd = -1;

int ret = -1; + struct ifreq ifr; + + if ((fd = virNetDevSetupControl(brname,&ifr))< 0) + goto cleanup; - cmd = virCommandNew(BRCTL); - virCommandAddArgList(cmd, "setfd", brname, NULL); - virCommandAddArgFormat(cmd, "%d", delay); + ret = virNetDevBridgeSet(brname, "stp_state", MS_TO_JIFFIES(delay), + fd,&ifr); - if (virCommandRun(cmd, NULL)< 0) +cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + + +/** + * virNetDevBridgeGetSTPDelay: + * @brname: the bridge device name + * @delayms: the forward delay in milliseconds + * + * Retrives the forward delay for the bridge device @brname + * storing it in @delayms. The forward delay is only meaningful + * if STP is enabled + * + * Returns 0 on success, -1 on error+ + */ +int virNetDevBridgeGetSTPDelay(const char *brname, + int *delayms) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + unsigned long i; + + if ((fd = virNetDevSetupControl(brname,&ifr))< 0) goto cleanup; - ret = 0; + ret = virNetDevBridgeGet(brname, "stp_state",&i, + fd,&ifr); + *delayms = JIFFIES_TO_MS(i); + cleanup: - virCommandFree(cmd); + VIR_FORCE_CLOSE(fd); return ret; } + /** * virNetDevBridgeSetSTP: * @brname: the bridge name @@ -846,23 +985,53 @@ cleanup: int virNetDevBridgeSetSTP(const char *brname, bool enable) { - virCommandPtr cmd; + int fd = -1; int ret = -1; + struct ifreq ifr; - cmd = virCommandNew(BRCTL); - virCommandAddArgList(cmd, "stp", brname, - enable ? "on" : "off", - NULL); + if ((fd = virNetDevSetupControl(brname,&ifr))< 0) + goto cleanup; - if (virCommandRun(cmd, NULL)< 0) + ret = virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0, + fd,&ifr); + +cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + + +/** + * virNetDevBridgeGetSTP: + * @brname: the bridge device name + * @enabled: returns the STP state + * + * Determine the state of the spanning tree protocol on + * the device @brname, returning the state in @enabled + * + * Returns 0 on success, -1 on error + */ +int virNetDevBridgeGetSTP(const char *brname, + bool *enabled) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + unsigned long i; + + if ((fd = virNetDevSetupControl(brname,&ifr))< 0) goto cleanup; - ret = 0; + ret = virNetDevBridgeGet(brname, "stp_state",&i, + fd,&ifr); + *enabled = i ? true : false; + cleanup: - virCommandFree(cmd); + VIR_FORCE_CLOSE(fd); return ret; } + /** * brCreateTap: * @ifname: the interface name