Remove use of libsysfs from bridge code

ChangeLog

+Tue Jun 26 18:57:00 EST 2007 Daniel P. Berrange <berrange@redhat.com>
+
+	* qemud/Makefile.am, qemud/bridge.c, configure.in, libvirt.spec.in
+	Don't use libsysfs for bridge APIs, call brctl directly.
+	* qemud/driver.c: Set the STP and forward-delay params if provided
+
 Tue Jun 26 18:56:00 EST 2007 Daniel P. Berrange <berrange@redhat.com>
 
 	* src/test.c: Don't try to handle URIs with a hostname

configure.in

@@ -221,22 +221,6 @@ else
     fi
 fi
 
-dnl
-dnl check for libsyfs (>= 2.0.0); allow disabling bridge parameters support altogether
-dnl
-AC_ARG_ENABLE(bridge-params,
-              AC_HELP_STRING([--disable-bridge-params],
-                             [disable support for setting bridge parameters using libsysfs [default=no]]),,
-              enable_bridge_params=yes)
-
-if test x"$enable_bridge_params" == "xyes"; then
-    AC_CHECK_LIB(sysfs, sysfs_open_device,
-                 [AC_CHECK_HEADER(sysfs/libsysfs.h,
-		                  AC_DEFINE(ENABLE_BRIDGE_PARAMS, , [enable setting bridge parameters using libsysfs])
-				  SYSFS_LIBS="-lsysfs" AC_SUBST(SYSFS_LIBS),
-				  AC_MSG_ERROR([You must install libsysfs in order to compile libvirt]))])
-fi
-
 dnl
 dnl check for kernel headers required by qemud/bridge.c
 dnl

libvirt.spec.in

@@ -14,13 +14,13 @@ Requires: libxml2
 Requires: readline
 Requires: ncurses
 Requires: dnsmasq
+Requires: bridge-utils
+Requires: iptables
 BuildRequires: xen-devel
 BuildRequires: libxml2-devel
 BuildRequires: readline-devel
 BuildRequires: ncurses-devel
 BuildRequires: gettext
-BuildRequires: libsysfs-devel
-BuildRequires: /sbin/iptables
 BuildRequires: gnutls-devel
 Obsoletes: libvir
 ExclusiveArch: i386 x86_64 ia64

qemud/Makefile.am

@@ -28,7 +28,7 @@ libvirt_qemud_CFLAGS = \
 	-DREMOTE_PID_FILE="\"$(REMOTE_PID_FILE)\"" \
         -DGETTEXT_PACKAGE=\"$(PACKAGE)\"
 
-libvirt_qemud_LDFLAGS = $(WARN_CFLAGS) $(LIBXML_LIBS) $(SYSFS_LIBS)
+libvirt_qemud_LDFLAGS = $(WARN_CFLAGS) $(LIBXML_LIBS)
 libvirt_qemud_DEPENDENCIES = ../src/libvirt.la
 libvirt_qemud_LDADD = ../src/libvirt.la
 

qemud/bridge.c

@@ -33,6 +33,8 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/ioctl.h>
+#include <paths.h>
+#include <sys/wait.h>
 
 #include <linux/param.h>     /* HZ                 */
 #include <linux/sockios.h>   /* SIOCBRADDBR etc.   */
@@ -43,6 +45,7 @@
 
 #define MAX_BRIDGE_ID 256
 
+#define BRCTL_PATH "/usr/sbin/brctl"
 #define JIFFIES_TO_MS(j) (((j)*1000)/HZ)
 #define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000)
 
@@ -423,185 +426,138 @@ brGetInetNetmask(brControl *ctl,
     return brGetInetAddr(ctl, ifname, SIOCGIFNETMASK, addr, maxlen);
 }
 
-#ifdef ENABLE_BRIDGE_PARAMS
-
-#include <sysfs/libsysfs.h>
-
 static int
-brSysfsPrep(struct sysfs_class_device **dev,
-            struct sysfs_attribute **attr,
-            const char *bridge,
-            const char *attrname)
+brctlSpawn(char * const *argv)
 {
-    *dev = NULL;
-    *attr = NULL;
+    pid_t pid, ret;
+    int status;
+    int null = -1;
 
-    if (!(*dev = sysfs_open_class_device("net", bridge)))
+    if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0)
         return errno;
 
-    if (!(*attr = sysfs_get_classdev_attr(*dev, attrname))) {
-        int err = errno;
-
-        sysfs_close_class_device(*dev);
-        *dev = NULL;
-
-        return err;
+    pid = fork();
+    if (pid == -1) {
+        int saved_errno = errno;
+        close(null);
+        return saved_errno;
     }
 
-    return 0;
-}
+    if (pid == 0) { /* child */
+        dup2(null, STDIN_FILENO);
+        dup2(null, STDOUT_FILENO);
+        dup2(null, STDERR_FILENO);
+        close(null);
 
-static int
-brSysfsWriteInt(struct sysfs_attribute *attr,
-                int value)
-{
-    char buf[32];
-    int len;
+        execvp(argv[0], argv);
+
+        _exit (1);
+    }
 
-    len = snprintf(buf, sizeof(buf), "%d\n", value);
+    close(null);
 
-    if (len > (int)sizeof(buf))
-        len = sizeof(buf); /* paranoia, shouldn't happen */
+    while ((ret = waitpid(pid, &status, 0) == -1) && errno == EINTR);
+    if (ret == -1)
+        return errno;
 
-    return sysfs_write_attribute(attr, buf, len) == 0 ? 0 : errno;
+    return (WIFEXITED(status) && WEXITSTATUS(status) == 0) ? 0 : EINVAL;
 }
 
 int
-brSetForwardDelay(brControl *ctl,
+brSetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED,
                   const char *bridge,
                   int delay)
 {
-    struct sysfs_class_device *dev;
-    struct sysfs_attribute *attr;
-    int err = 0;
+    char **argv;
+    int retval = ENOMEM;
+    int n;
+    char delayStr[30];
 
-    if (!ctl || !bridge)
-        return EINVAL;
+    n = 1 + /* brctl */
+        1 + /* setfd */
+        1 + /* brige name */
+        1; /* value */
 
-    if ((err = brSysfsPrep(&dev, &attr, bridge, SYSFS_BRIDGE_ATTR "/forward_delay")))
-        return err;
+    snprintf(delayStr, sizeof(delayStr), "%d", delay);
 
-    err = brSysfsWriteInt(attr, MS_TO_JIFFIES(delay));
+    if (!(argv = (char **)calloc(n + 1, sizeof(char *))))
+        goto error;
 
-    sysfs_close_class_device(dev);
+    n = 0;
 
-    return err;
-}
+    if (!(argv[n++] = strdup(BRCTL_PATH)))
+        goto error;
 
-int
-brGetForwardDelay(brControl *ctl,
-                  const char *bridge,
-                  int *delayp)
-{
-    struct sysfs_class_device *dev;
-    struct sysfs_attribute *attr;
-    int err = 0;
+    if (!(argv[n++] = strdup("setfd")))
+        goto error;
 
-    if (!ctl || !bridge || !delayp)
-        return EINVAL;
+    if (!(argv[n++] = strdup(bridge)))
+        goto error;
 
-    if ((err = brSysfsPrep(&dev, &attr, bridge, SYSFS_BRIDGE_ATTR "/forward_delay")))
-        return err;
+    if (!(argv[n++] = strdup(delayStr)))
+        goto error;
 
-    *delayp = strtoul(attr->value, NULL, 0);
+    argv[n++] = NULL;
 
-    if (errno != ERANGE) {
-        *delayp = JIFFIES_TO_MS(*delayp);
-    } else {
-        err = errno;
-    }
+    retval = brctlSpawn(argv);
 
-    sysfs_close_class_device(dev);
+ error:
+    if (argv) {
+        n = 0;
+        while (argv[n])
+            free(argv[n++]);
+        free(argv);
+    }
 
-    return err;
+    return retval;
 }
 
 int
-brSetEnableSTP(brControl *ctl,
+brSetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED,
                const char *bridge,
                int enable)
 {
-    struct sysfs_class_device *dev;
-    struct sysfs_attribute *attr;
-    int err = 0;
+    char **argv;
+    int retval = ENOMEM;
+    int n;
 
-    if (!ctl || !bridge)
-        return EINVAL;
+    n = 1 + /* brctl */
+        1 + /* setfd */
+        1 + /* brige name */
+        1;  /* value */
 
-    if ((err = brSysfsPrep(&dev, &attr, bridge, SYSFS_BRIDGE_ATTR "/stp_state")))
-        return err;
+    if (!(argv = (char **)calloc(n + 1, sizeof(char *))))
+        goto error;
 
-    err = brSysfsWriteInt(attr, (enable == 0) ? 0 : 1);
+    n = 0;
 
-    sysfs_close_class_device(dev);
+    if (!(argv[n++] = strdup(BRCTL_PATH)))
+        goto error;
 
-    return err;
-}
+    if (!(argv[n++] = strdup("setfd")))
+        goto error;
 
-int
-brGetEnableSTP(brControl *ctl,
-               const char *bridge,
-               int *enablep)
-{
-    struct sysfs_class_device *dev;
-    struct sysfs_attribute *attr;
-    int err = 0;
+    if (!(argv[n++] = strdup(bridge)))
+        goto error;
 
-    if (!ctl || !bridge || !enablep)
-        return EINVAL;
+    if (!(argv[n++] = strdup(enable ? "on" : "off")))
+        goto error;
 
-    if ((err = brSysfsPrep(&dev, &attr, bridge, SYSFS_BRIDGE_ATTR "/stp_state")))
-        return err;
+    argv[n++] = NULL;
 
-    *enablep = strtoul(attr->value, NULL, 0);
+    retval = brctlSpawn(argv);
 
-    if (errno != ERANGE) {
-        *enablep = (*enablep == 0) ? 0 : 1;
-    } else {
-        err = errno;
+ error:
+    if (argv) {
+        n = 0;
+        while (argv[n])
+            free(argv[n++]);
+        free(argv);
     }
 
-    sysfs_close_class_device(dev);
-
-    return err;
-}
-
-#else /* ENABLE_BRIDGE_PARAMS */
-
-int
-brSetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED,
-                  const char *bridge ATTRIBUTE_UNUSED,
-                  int delay ATTRIBUTE_UNUSED)
-{
-    return 0;
-}
-
-int
-brGetForwardDelay(brControl *ctl ATTRIBUTE_UNUSED,
-                  const char *bridge ATTRIBUTE_UNUSED,
-                  int *delay ATTRIBUTE_UNUSED)
-{
-    return 0;
+    return retval;
 }
 
-int
-brSetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED,
-               const char *bridge ATTRIBUTE_UNUSED,
-               int enable ATTRIBUTE_UNUSED)
-{
-    return 0;
-}
-
-int
-brGetEnableSTP(brControl *ctl ATTRIBUTE_UNUSED,
-               const char *bridge ATTRIBUTE_UNUSED,
-               int *enable ATTRIBUTE_UNUSED)
-{
-    return 0;
-}
-
-#endif /* ENABLE_BRIDGE_PARAMS */
-
 /*
  * Local variables:
  *  indent-tabs-mode: nil

qemud/driver.c

@@ -1154,6 +1154,22 @@ int qemudStartNetworkDaemon(struct qemud_driver *driver,
         return -1;
     }
 
+
+    if (network->def->forwardDelay &&
+        (err = brSetForwardDelay(driver->brctl, network->bridge, network->def->forwardDelay))) {
+        qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "failed to set bridge forward delay to %d\n",
+                         network->def->forwardDelay);
+        goto err_delbr;
+    }
+
+    if ((err = brSetForwardDelay(driver->brctl, network->bridge, network->def->disableSTP ? 0 : 1))) {
+        qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "failed to set bridge STP to %s\n",
+                         network->def->disableSTP ? "off" : "on");
+        goto err_delbr;
+    }
+
     if (network->def->ipAddress[0] &&
         (err = brSetInetAddress(driver->brctl, network->bridge, network->def->ipAddress))) {
         qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,