Oops, this patch doesn't work properly. Please ignore it.
Ethan
On Thu, Nov 17, 2011 at 19:38, Ethan Jackson <et...@nicira.com> wrote:
> 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..2b41760 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_WARN("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
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
