As handling software via cgroups is more and more common, PID files are not always needed. This change adds --disable-pid-socket-path to ./configure to build OpenvSwitch without this PID section of the socket path. This offers a way for software to easier know the socket path for daemons independent on which platform it is running on.
Today the socket path is by default one of: Linux: RUNDIR/daemon.$PID.ctl Windows: RUNDIR/daemon.ctl For Linux it is thus commonplace to also run the daemon with --pidfile to be able to communicate with the daemon. For Windows there is no such need (or possibility). In order to preserve default behavior the new flag ships as enabled. Documentation currently describes the default behavior only. ovsdb-client is updated to support setting explicit socket path when running in detached mode. This closes https://github.com/openvswitch/ovs-issues/issues/114. Signed-off-by: Christian Svensson <blue...@google.com> --- AUTHORS | 1 + Makefile.am | 8 ++++++++ configure.ac | 1 + lib/unixctl.c | 4 +++- m4/openvswitch.m4 | 26 ++++++++++++++++++++++++++ ovsdb/ovsdb-client.c | 13 +++++++++++-- python/automake.mk | 8 +++++--- python/ovs/dirs.py | 2 ++ python/ovs/dirs.py.template | 2 ++ python/ovs/unixctl/__init__.py | 3 +++ python/ovs/unixctl/server.py | 12 ++++-------- tests/atlocal.in | 1 + tests/ovn-controller-vtep.at | 4 ++-- tests/ovsdb-lock.at | 2 +- tests/ovsdb-monitor.at | 7 +++---- tests/unixctl-py.at | 18 ++++++++++-------- utilities/ovs-appctl.c | 9 ++++++--- 17 files changed, 89 insertions(+), 32 deletions(-) diff --git a/AUTHORS b/AUTHORS index b598f4b..1226958 100644 --- a/AUTHORS +++ b/AUTHORS @@ -44,6 +44,7 @@ Casey Barker crbar...@google.com Chandra Sekhar Vejendla csvej...@us.ibm.com Christoph Jaeger c...@linux.com Chris Wright chr...@sous-sol.org +Christian Svensson blue...@google.com Chuck Short zul...@ubuntu.com Ciara Loftus ciara.lof...@intel.com Cong Wang amw...@redhat.com diff --git a/Makefile.am b/Makefile.am index 49010b3..77d2417 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,6 +9,7 @@ AUTOMAKE_OPTIONS = foreign subdir-objects ACLOCAL_AMFLAGS = -I m4 SUBDIRS = datapath +AM_DISTCHECK_CONFIGURE_FLAGS = AM_CPPFLAGS = $(SSL_CFLAGS) AM_LDFLAGS = $(SSL_LDFLAGS) AM_LDFLAGS += $(OVS_LDFLAGS) @@ -42,6 +43,13 @@ AM_CPPFLAGS += -DNDEBUG AM_CFLAGS += -fomit-frame-pointer endif +if WITH_PID_SOCKET_PATH +AM_DISTCHECK_CONFIGURE_FLAGS += '--enable-pid-socket-path' +AM_CPPFLAGS += -DWITH_PID_SOCKET_PATH +else +AM_DISTCHECK_CONFIGURE_FLAGS += '--disable-pid-socket-path' +endif + if WIN32 psep=";" else diff --git a/configure.ac b/configure.ac index 05d80d5..399f5c3 100644 --- a/configure.ac +++ b/configure.ac @@ -101,6 +101,7 @@ OVS_CHECK_DOT OVS_CHECK_IF_PACKET OVS_CHECK_IF_DL OVS_CHECK_STRTOK_R +OVS_CHECK_PID_SOCKET_PATH AC_CHECK_DECLS([sys_siglist], [], [], [[#include <signal.h>]]) AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec], [], [], [[#include <sys/stat.h>]]) diff --git a/lib/unixctl.c b/lib/unixctl.c index 57d6577..7ba65c0 100644 --- a/lib/unixctl.c +++ b/lib/unixctl.c @@ -188,6 +188,8 @@ unixctl_command_reply_error(struct unixctl_conn *conn, const char *error) /* Creates a unixctl server listening on 'path', which for POSIX may be: * * - NULL, in which case <rundir>/<program>.<pid>.ctl is used. + * If built with --disable-pid-socket-path it is instead + * <rundir>/<program>.ctl. * * - A name that does not start with '/', in which case it is put in * <rundir>. @@ -236,7 +238,7 @@ unixctl_server_create(const char *path, struct unixctl_server **serverp) punix_path = xasprintf("punix:%s", abs_path); free(abs_path); } else { -#ifndef _WIN32 +#ifdef WITH_PID_SOCKET_PATH punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(), program_name, (long int) getpid()); #else diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index a448223..28315f7 100644 --- a/m4/openvswitch.m4 +++ b/m4/openvswitch.m4 @@ -589,3 +589,29 @@ AC_DEFUN([OVS_CHECK_PRAGMA_MESSAGE], [AC_DEFINE(HAVE_PRAGMA_MESSAGE,1,[Define if compiler supports #pragma message directive])]) ]) + +dnl Checks if user wants to include PID in default socket path. +AC_DEFUN([OVS_CHECK_PID_SOCKET_PATH], + [AC_ARG_ENABLE( + [pid-socket-path], + [AC_HELP_STRING([--disable-pid-socket-path], + [Do not include PID in socket path])], + [case "${enableval}" in + (yes) with_pid_socket_path=yes ;; + (no) with_pid_socket_path=no ;; + (*) AC_MSG_ERROR([bad value ${enableval} for +--enable-pid-socket-path]) ;; + esac], + [if test "$WIN32" = yes; then + with_pid_socket_path=no + else + with_pid_socket_path=yes + fi]) + AC_SUBST([WITH_PID_SOCKET_PATH]) + if test $with_pid_socket_path != no; then + WITH_PID_SOCKET_PATH=yes + else + WITH_PID_SOCKET_PATH=no + fi + AM_CONDITIONAL([WITH_PID_SOCKET_PATH], + [test x$with_pid_socket_path = xyes])]) diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index 1f83f3b..b12650c 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -71,6 +71,8 @@ struct ovsdb_client_command { /* --timestamp: Print a timestamp before each update on "monitor" command? */ static bool timestamp; +static char *unixctl_path = NULL; + /* Format for table output. */ static struct table_style table_style = TABLE_STYLE_DEFAULT; @@ -171,6 +173,7 @@ parse_options(int argc, char *argv[]) { enum { OPT_BOOTSTRAP_CA_CERT = UCHAR_MAX + 1, + OPT_UNIXCTL, OPT_TIMESTAMP, VLOG_OPTION_ENUMS, DAEMON_OPTION_ENUMS, @@ -180,6 +183,7 @@ parse_options(int argc, char *argv[]) {"help", no_argument, NULL, 'h'}, {"version", no_argument, NULL, 'V'}, {"timestamp", no_argument, NULL, OPT_TIMESTAMP}, + {"unixctl", required_argument, NULL, OPT_UNIXCTL}, VLOG_LONG_OPTIONS, DAEMON_LONG_OPTIONS, #ifdef HAVE_OPENSSL @@ -220,6 +224,10 @@ parse_options(int argc, char *argv[]) timestamp = true; break; + case OPT_UNIXCTL: + unixctl_path = optarg; + break; + case '?': exit(EXIT_FAILURE); @@ -288,6 +296,7 @@ usage(void) daemon_usage(); vlog_usage(); printf("\nOther options:\n" + " --unixctl=SOCKET override default control socket name\n" " -h, --help display this help message\n" " -V, --version display version information\n"); exit(EXIT_SUCCESS); @@ -956,7 +965,7 @@ do_monitor__(struct jsonrpc *rpc, const char *database, if (get_detach()) { int error; - error = unixctl_server_create(NULL, &unixctl); + error = unixctl_server_create(unixctl_path, &unixctl); if (error) { ovs_fatal(error, "failed to create unixctl server"); } @@ -1470,7 +1479,7 @@ do_lock(struct jsonrpc *rpc, const char *method, const char *lock) if (get_detach()) { int error; - error = unixctl_server_create(NULL, &unixctl); + error = unixctl_server_create(unixctl_path, &unixctl); if (error) { ovs_fatal(error, "failed to create unixctl server"); } diff --git a/python/automake.mk b/python/automake.mk index 1c8fa38..eb9a120 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -73,6 +73,7 @@ ovs-install-data-local: -e 's,[@]bindir[@],$(bindir),g' \ -e 's,[@]sysconfdir[@],$(sysconfdir),g' \ -e 's,[@]DBDIR[@],$(DBDIR),g' \ + -e 's,[@]WITH_PID_SOCKET_PATH[@],$(WITH_PID_SOCKET_PATH),g' \ < $(srcdir)/python/ovs/dirs.py.template \ > python/ovs/dirs.py.tmp $(MKDIR_P) $(DESTDIR)$(pkgdatadir)/python/ovs @@ -101,7 +102,7 @@ $(srcdir)/python/ovs/version.py: config.status if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi ALL_LOCAL += $(srcdir)/python/ovs/dirs.py -$(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template +$(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template config.status $(AM_V_GEN)sed \ -e '/^##/d' \ -e 's,[@]pkgdatadir[@],/usr/local/share/openvswitch,g' \ @@ -110,6 +111,7 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template -e 's,[@]bindir[@],/usr/local/bin,g' \ -e 's,[@]sysconfdir[@],/usr/local/etc,g' \ -e 's,[@]DBDIR[@],/usr/local/etc/openvswitch,g' \ - < $? > $@.tmp && \ - mv $@.tmp $@ + -e 's,[@]WITH_PID_SOCKET_PATH[@],$(WITH_PID_SOCKET_PATH),g' \ + < $< > $(@F).tmp && \ + if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi EXTRA_DIST += python/ovs/dirs.py.template diff --git a/python/ovs/dirs.py b/python/ovs/dirs.py index c67aecb..5de5c4a 100644 --- a/python/ovs/dirs.py +++ b/python/ovs/dirs.py @@ -21,6 +21,8 @@ PKGDATADIR = os.environ.get("OVS_PKGDATADIR", """/usr/local/share/openvswitch""" RUNDIR = os.environ.get("OVS_RUNDIR", """/var/run""") LOGDIR = os.environ.get("OVS_LOGDIR", """/usr/local/var/log""") BINDIR = os.environ.get("OVS_BINDIR", """/usr/local/bin""") +WITH_PID_SOCKET_PATH = ("y" in + os.environ.get("OVS_WITH_PID_SOCKET_PATH", "yes")) DBDIR = os.environ.get("OVS_DBDIR") if not DBDIR: diff --git a/python/ovs/dirs.py.template b/python/ovs/dirs.py.template index fb31b74..0bfbf9b 100644 --- a/python/ovs/dirs.py.template +++ b/python/ovs/dirs.py.template @@ -21,6 +21,8 @@ PKGDATADIR = os.environ.get("OVS_PKGDATADIR", """@pkgdatadir@""") RUNDIR = os.environ.get("OVS_RUNDIR", """@RUNDIR@""") LOGDIR = os.environ.get("OVS_LOGDIR", """@LOGDIR@""") BINDIR = os.environ.get("OVS_BINDIR", """@bindir@""") +WITH_PID_SOCKET_PATH = ("y" in + os.environ.get("OVS_WITH_PID_SOCKET_PATH", "@WITH_PID_SOCKET_PATH@")) DBDIR = os.environ.get("OVS_DBDIR") if not DBDIR: diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py index 48b56d4..5477c6e 100644 --- a/python/ovs/unixctl/__init__.py +++ b/python/ovs/unixctl/__init__.py @@ -76,6 +76,9 @@ def socket_name_from_target(target): if target.startswith('/') or target.find(':') > -1: return 0, target + if not ovs.dirs.WITH_PID_SOCKET_PATH: + return 0, "%s/%s.ctl" % (ovs.dirs.RUNDIR, target) + pidfile_name = "%s/%s.pid" % (ovs.dirs.RUNDIR, target) pid = ovs.daemon.read_pidfile(pidfile_name) if pid < 0: diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index 8595ed8..1283131 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -15,7 +15,6 @@ import copy import errno import os -import sys import six from six.moves import range @@ -188,14 +187,11 @@ class UnixctlServer(object): if path is not None: path = "punix:%s" % ovs.util.abs_file_name(ovs.dirs.RUNDIR, path) + elif ovs.dirs.WITH_PID_SOCKET_PATH: + path = "punix:%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, + ovs.util.PROGRAM_NAME, os.getpid()) else: - if sys.platform == "win32": - path = "punix:%s/%s.ctl" % (ovs.dirs.RUNDIR, - ovs.util.PROGRAM_NAME) - else: - path = "punix:%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, - ovs.util.PROGRAM_NAME, - os.getpid()) + path = "punix:%s/%s.ctl" % (ovs.dirs.RUNDIR, ovs.util.PROGRAM_NAME) if version is None: version = ovs.version.VERSION diff --git a/tests/atlocal.in b/tests/atlocal.in index 55070d8..e1741cf 100644 --- a/tests/atlocal.in +++ b/tests/atlocal.in @@ -2,6 +2,7 @@ HAVE_OPENSSL='@HAVE_OPENSSL@' HAVE_PYTHON='@HAVE_PYTHON@' HAVE_PYTHON3='@HAVE_PYTHON3@' +WITH_PID_SOCKET_PATH='@WITH_PID_SOCKET_PATH@' EGREP='@EGREP@' PERL='@PERL@' diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at index 8eca16c..5b5f09b 100644 --- a/tests/ovn-controller-vtep.at +++ b/tests/ovn-controller-vtep.at @@ -25,8 +25,8 @@ m4_define([OVN_CONTROLLER_VTEP_START], dnl Start ovsdb-server. AT_CHECK([ovsdb-server --detach --no-chdir --pidfile=$OVS_RUNDIR/ovsdb-server.pid --log-file --remote=punix:$OVS_RUNDIR/db.sock vswitchd.db vtep.db], [0], [], [stderr]) - AT_CHECK([ovsdb-server --detach --no-chdir --pidfile=$OVS_RUNDIR/ovsdb-nb-server.pid --log-file=ovsdb-nb-server.log --remote=punix:$OVS_RUNDIR/ovnnb_db.sock ovn-nb.db], [0], [], [stderr]) - AT_CHECK([ovsdb-server --detach --no-chdir --pidfile=$OVS_RUNDIR/ovsdb-sb-server.pid --log-file=ovsdb-sb-server.log --remote=punix:$OVS_RUNDIR/ovnsb_db.sock ovn-sb.db ovn-sb.db], [0], [], [stderr]) + AT_CHECK([ovsdb-server --detach --no-chdir --unixctl=$OVS_RUNDIR/ovsdb-nb-server.ctl --pidfile=$OVS_RUNDIR/ovsdb-nb-server.pid --log-file=ovsdb-nb-server.log --remote=punix:$OVS_RUNDIR/ovnnb_db.sock ovn-nb.db], [0], [], [stderr]) + AT_CHECK([ovsdb-server --detach --no-chdir --unixctl=$OVS_RUNDIR/ovsdb-sb-server.ctl --pidfile=$OVS_RUNDIR/ovsdb-sb-server.pid --log-file=ovsdb-sb-server.log --remote=punix:$OVS_RUNDIR/ovnsb_db.sock ovn-sb.db ovn-sb.db], [0], [], [stderr]) on_exit "kill `cat ovsdb-server.pid` `cat ovsdb-nb-server.pid` `cat ovsdb-sb-server.pid`" AT_CHECK([[sed < stderr ' /vlog|INFO|opened log file/d diff --git a/tests/ovsdb-lock.at b/tests/ovsdb-lock.at index 7152c5d..af5aa27 100644 --- a/tests/ovsdb-lock.at +++ b/tests/ovsdb-lock.at @@ -37,7 +37,7 @@ AT_CLEANUP OVSDB_CHECK_LOCK_SETUP([unlock], [positive]) AT_CHECK([ovsdb-client --detach --pidfile lock unix:socket lock0 >c1-output 2>&1], [0], [], []) -AT_CHECK([ovsdb-client --detach lock unix:socket lock0 >c2-output 2>&1], +AT_CHECK([ovsdb-client --detach --unixctl=dummy.ctl lock unix:socket lock0 >c2-output 2>&1], [0], [], []) AT_CHECK([ovs-appctl -t ovsdb-client unlock lock0], [0], [], []) OVS_APP_EXIT_AND_WAIT([ovsdb-server]) diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at index 6d51a1a..0f9fdea 100644 --- a/tests/ovsdb-monitor.at +++ b/tests/ovsdb-monitor.at @@ -73,19 +73,18 @@ m4_define([OVSDB_CHECK_MONITOR_COND], AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/server-pid --remote=punix:socket --unixctl="`pwd`"/unixctl --log-file="`pwd`"/ovsdb-server-log db >/dev/null 2>&1], [0], [], []) if test "$IS_WIN32" = "yes"; then - AT_CHECK([ovsdb-client -vjsonrpc --pidfile="`pwd`"/client-pid -d json monitor-cond --format=csv unix:socket $4 '[$8]' $5 $9 > output &], + AT_CHECK([ovsdb-client -vjsonrpc --unixctl="`pwd`"/client-ctl --pidfile="`pwd`"/client-pid -d json monitor-cond --format=csv unix:socket $4 '[$8]' $5 $9 > output &], [0], [ignore], [ignore], [kill `cat server-pid`]) sleep 1 else - AT_CHECK([ ovsdb-client -vjsonrpc --detach --no-chdir --pidfile="`pwd`"/client-pid -d json monitor-cond --format=csv unix:socket $4 '[$8]' $5 $9 > output], + AT_CHECK([ ovsdb-client -vjsonrpc --detach --no-chdir --unixctl="`pwd`"/client-ctl --pidfile="`pwd`"/client-pid -d json monitor-cond --format=csv unix:socket $4 '[$8]' $5 $9 > output], [0], [ignore], [ignore], [kill `cat server-pid`]) fi m4_foreach([txn], [$6], [AT_CHECK([ovsdb-client transact unix:socket 'txn'], [0], [ignore], [ignore], [kill `cat server-pid client-pid`])]) - CLIENT_PID=`cat "$OVS_RUNDIR"/client-pid 2>/dev/null` m4_foreach([cond], [$10], - [AT_CHECK([ovs-appctl -t "`pwd`"/ovsdb-client.$CLIENT_PID.ctl ovsdb-client/cond_change $5 'cond'], [0], [ignore], [ignore])]) + [AT_CHECK([ovs-appctl -t "`pwd`"/client-ctl ovsdb-client/cond_change $5 'cond'], [0], [ignore], [ignore])]) AT_CHECK([ovsdb-client transact unix:socket '[["$4"]]'], [0], [ignore], [ignore], [kill `cat server-pid client-pid`]) AT_CHECK([ovs-appctl -t "`pwd`"/unixctl -e exit], [0], [ignore], [ignore]) diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at index 2031897..f75230b 100644 --- a/tests/unixctl-py.at +++ b/tests/unixctl-py.at @@ -88,20 +88,22 @@ m4_define([UNIXCTL_BAD_TARGET_PYN], [AT_SETUP([unixctl bad target - $1]) AT_SKIP_IF([test $2 = no]) - AT_CHECK([PYAPPCTL_PYN([$3]) -t bogus doit], [1], [], [stderr]) - AT_CHECK_UNQUOTED([tail -1 stderr], [0], [dnl + if test "$WITH_PID_SOCKET_PATH" = "yes"; then + AT_CHECK([PYAPPCTL_PYN([$3]) -t bogus doit], [1], [], [stderr]) + AT_CHECK_UNQUOTED([tail -1 stderr], [0], [dnl appctl.py: cannot read pidfile "`pwd`/bogus.pid" (No such file or directory) ]) - if test "$IS_WIN32" = "no"; then - AT_CHECK([PYAPPCTL_PYN([$3]) -t /bogus/path.pid doit], [1], [], [stderr]) - AT_CHECK([tail -1 stderr], [0], [dnl + if test "$IS_WIN32" = "no"; then + AT_CHECK([PYAPPCTL_PYN([$3]) -t /bogus/path.pid doit], [1], [], [stderr]) + AT_CHECK([tail -1 stderr], [0], [dnl appctl.py: cannot connect to "/bogus/path.pid" (No such file or directory) ]) - else - AT_CHECK([PYAPPCTL_PYN([$3]) -t c:/bogus/path.pid doit], [1], [], [stderr]) - AT_CHECK([tail -1 stderr], [0], [dnl + else + AT_CHECK([PYAPPCTL_PYN([$3]) -t c:/bogus/path.pid doit], [1], [], [stderr]) + AT_CHECK([tail -1 stderr], [0], [dnl appctl.py: cannot connect to "c:/bogus/path.pid" (No such file or directory) ]) + fi fi AT_CLEANUP]) diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c index 8f87cc4..62172b8 100644 --- a/utilities/ovs-appctl.c +++ b/utilities/ovs-appctl.c @@ -201,6 +201,12 @@ connect_to_target(const char *target) #ifndef _WIN32 if (target[0] != '/') { +#else + /* On windows, if the 'target' contains ':', we make an assumption that + * it is an absolute path. */ + if (!strchr(target, ':')) { +#endif +#ifdef WITH_PID_SOCKET_PATH char *pidfile_name; pid_t pid; @@ -213,9 +219,6 @@ connect_to_target(const char *target) socket_name = xasprintf("%s/%s.%ld.ctl", ovs_rundir(), target, (long int) pid); #else - /* On windows, if the 'target' contains ':', we make an assumption that - * it is an absolute path. */ - if (!strchr(target, ':')) { socket_name = xasprintf("%s/%s.ctl", ovs_rundir(), target); #endif } else { -- 2.8.0.rc3.226.g39d4020 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
