[ovs-dev] [PATCH] tests: Allow unit tests to run as root.

[Ethan Jackson]

The unit tests did not allow users to run them as root because
ovs-vswitchd would destroy all of the existing 'system' datapaths.
This patch prevents ovs-vswitchd from registering 'system'
datapaths when running unit tests preventing the issue.
---
 lib/dpif.c              |   15 +++++++++++++++
 lib/dpif.h              |    1 +
 tests/ofproto-macros.at |    8 ++------
 vswitchd/ovs-vswitchd.c |    7 +++++++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index cc6e805..5f5417b 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -68,6 +68,7 @@ struct registered_dpif_class {
     int refcount;
 };
 static struct shash dpif_classes = SHASH_INITIALIZER(&dpif_classes);
+static struct sset dpif_blacklist = SSET_INITIALIZER(&dpif_blacklist);
 
 /* Rate limit for individual messages going to or from the datapath, output at
  * DBG level.  This is very high because, if these are enabled, it is because
@@ -108,6 +109,12 @@ dp_register_provider(const struct dpif_class *new_class)
 {
     struct registered_dpif_class *registered_class;
 
+    if (sset_contains(&dpif_blacklist, new_class->type)) {
+        VLOG_DBG("attempted to register blacklisted provider: %s",
+                 new_class->type);
+        return EINVAL;
+    }
+
     if (shash_find(&dpif_classes, new_class->type)) {
         VLOG_WARN("attempted to register duplicate datapath provider: %s",
                   new_class->type);
@@ -151,6 +158,14 @@ dp_unregister_provider(const char *type)
     return 0;
 }
 
+/* Blacklists a provider.  Causes future calls of dp_register_provider() with
+ * a dpif_class which implements 'type' to fail. */
+void
+dp_blacklist_provider(const char *type)
+{
+    sset_add(&dpif_blacklist, type);
+}
+
 /* Clears 'types' and enumerates the types of all currently registered datapath
  * providers into it.  The caller must first initialize the sset. */
 void
diff --git a/lib/dpif.h b/lib/dpif.h
index 223f990..0ff2389 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -40,6 +40,7 @@ struct dpif_class;
 
 int dp_register_provider(const struct dpif_class *);
 int dp_unregister_provider(const char *type);
+void dp_blacklist_provider(const char *type);
 void dp_enumerate_types(struct sset *types);
 const char *dpif_normalize_type(const char *);
 
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 5bc431c..fc5bba1 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -3,11 +3,7 @@ m4_define([STRIP_DURATION], [[sed 's/\bduration=[0-9.]*s/duration=?s/']])
 m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m'])
 
 m4_define([OVS_VSWITCHD_START],
-  [dnl Skip this test if running as root.  Otherwise ovs-vswitchd will tear
-   dnl down any existing datapaths if the kernel module is loaded.
-   AT_SKIP_IF([test "`id -u`" = 0])
-
-   OVS_RUNDIR=$PWD; export OVS_RUNDIR
+  [OVS_RUNDIR=$PWD; export OVS_RUNDIR
    OVS_LOGDIR=$PWD; export OVS_LOGDIR
    OVS_SYSCONFDIR=$PWD; export OVS_SYSCONFDIR
    trap 'kill `cat ovsdb-server.pid ovs-vswitchd.pid`' 0
@@ -26,7 +22,7 @@ m4_define([OVS_VSWITCHD_START],
    AT_CHECK([ovs-vsctl --no-wait init])
 
    dnl Start ovs-vswitchd.
-   AT_CHECK([ovs-vswitchd --detach --pidfile --enable-dummy --log-file], [0], [], [stderr])
+   AT_CHECK([ovs-vswitchd --detach --pidfile --enable-dummy --disable-system --log-file], [0], [], [stderr])
    AT_CAPTURE_FILE([ovs-vswitchd.log])
    AT_CHECK([[sed < stderr '
 /vlog|INFO|opened log file/d
diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index 359e3ab..bdc3533 100644
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -31,6 +31,7 @@
 #include "compiler.h"
 #include "daemon.h"
 #include "dirs.h"
+#include "dpif.h"
 #include "dummy.h"
 #include "leak-checker.h"
 #include "netdev.h"
@@ -121,6 +122,7 @@ parse_options(int argc, char *argv[])
         LEAK_CHECKER_OPTION_ENUMS,
         OPT_BOOTSTRAP_CA_CERT,
         OPT_ENABLE_DUMMY,
+        OPT_DISABLE_SYSTEM,
         DAEMON_OPTION_ENUMS
     };
     static struct option long_options[] = {
@@ -134,6 +136,7 @@ parse_options(int argc, char *argv[])
         {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
         {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
         {"enable-dummy", no_argument, NULL, OPT_ENABLE_DUMMY},
+        {"disable-system", no_argument, NULL, OPT_DISABLE_SYSTEM},
         {NULL, 0, NULL, 0},
     };
     char *short_options = long_options_to_short_options(long_options);
@@ -181,6 +184,10 @@ parse_options(int argc, char *argv[])
             dummy_enable();
             break;
 
+        case OPT_DISABLE_SYSTEM:
+            dp_blacklist_provider("system");
+            break;
+
         case '?':
             exit(EXIT_FAILURE);
 
-- 
1.7.7.1