[libvirt] [PATCH 07/33] Remove usage of brctl command line tool
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Nov 8 14:48:34 UTC 2011
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> 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;
Also applicable for the other patches, the initialization to -1 doesn't
seem necessary.
> 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
ACK
More information about the libvir-list
mailing list