[PATCH 0/7] Add sanlock daemon tests

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

Modify sanlock so we can run sanlock daemon in the tests as current
user:
- Add an option to set the runtime directory via SANLOCK_RUN_DIR
  enviroment variable
- Add an option to disable privileged operations via
  SANLOCK_PRIVILEGED=0 enviroment variable

Both options should not be used in production, so we log a warning when
they are enabled.

SANLOCK_RUN_DIR effects both "sanlock daemon", "sanlock client" and
usage of libsanlock and libsanlock_client libraries, so we can test both
the executable and the python bindings.

With these modifications and the existing command line options, we can
run sanlock daemon in the tests as the current user. A new test module
was added with:

- sanlock_daemon() fixture that start the daemon at the start of the test
  session, and stop it at the end of the session.
- run() function that use modified environment to access the daemon.

The first test initialize a lockspace using "sanlock client init".

Nir Soffer (7):
  Fix lockfile() to use the given "dir" argument
  Add dir argument to sanlock_socket_address()
  Move SANLK_RUN_DIR to sanlock_internal.h
  Add environment variables helpers
  Add SANLOCK_RUN_DIR environment variable
  Add SANLOCK_PRIVILEGED environment variable
  Start the daemon tests module

 src/Makefile           |  9 +++++---
 src/client.c           |  8 ++++++-
 src/env.c              | 34 +++++++++++++++++++++++++++
 src/env.h              | 15 ++++++++++++
 src/lockfile.c         |  7 +++---
 src/main.c             | 25 ++++++++++++++++----
 src/sanlock_internal.h |  4 ++++
 src/sanlock_sock.c     |  4 ++--
 src/sanlock_sock.h     |  3 +--
 tests/daemon_test.py   | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 155 insertions(+), 17 deletions(-)
 create mode 100644 src/env.c
 create mode 100644 src/env.h
 create mode 100644 tests/daemon_test.py

-- 
2.9.5

[PATCH 1/7] Fix lockfile() to use the given "dir" argument

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

lockfile() was using SANLK_RUN_DIR for creating the run directory, and
the dir argument for building the lockfile path. Use only dir so we can
create a lockfile in another directory. This will be useful for running
sanlock as non-privileged user during the tests.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/lockfile.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/lockfile.c b/src/lockfile.c
index e3b720c..1839f56 100644
--- a/src/lockfile.c
+++ b/src/lockfile.c
@@ -25,7 +25,6 @@
 #include <sys/un.h>
 
 #include "sanlock_internal.h"
-#include "sanlock_sock.h"
 #include "log.h"
 #include "lockfile.h"
 
@@ -38,17 +37,17 @@ int lockfile(const char *dir, const char *name, int uid, int gid)
 	int fd, rv;
 
 	old_umask = umask(0022);
-	rv = mkdir(SANLK_RUN_DIR, 0775);
+	rv = mkdir(dir, 0775);
 	if (rv < 0 && errno != EEXIST) {
 		umask(old_umask);
 		return rv;
 	}
 	umask(old_umask);
 
-	rv = chown(SANLK_RUN_DIR, uid, gid);
+	rv = chown(dir, uid, gid);
 	if (rv < 0) {
 		log_error("lockfile chown error %s: %s",
-			  SANLK_RUN_DIR, strerror(errno));
+			  dir, strerror(errno));
 		return rv;
 	}
 
-- 
2.9.5

[PATCH 2/7] Add dir argument to sanlock_socket_address()

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

Like lockfile(), sanlock_socket_address() accepts now a dir argument,
and use it to create the socket. This would allow running sanlock
daemon and client in the tests as non-privileged user.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/client.c       | 2 +-
 src/main.c         | 2 +-
 src/sanlock_sock.c | 4 ++--
 src/sanlock_sock.h | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/client.c b/src/client.c
index ad78c21..9c83eb7 100644
--- a/src/client.c
+++ b/src/client.c
@@ -48,7 +48,7 @@ static int connect_socket(int *sock_fd)
 	if (s < 0)
 		return -errno;
 
-	rv = sanlock_socket_address(&addr);
+	rv = sanlock_socket_address(SANLK_RUN_DIR, &addr);
 	if (rv < 0) {
 		close(s);
 		return rv;
diff --git a/src/main.c b/src/main.c
index 9fc033f..e2c882f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1290,7 +1290,7 @@ static int setup_listener(void)
 	struct sockaddr_un addr;
 	int rv, fd, ci;
 
-	rv = sanlock_socket_address(&addr);
+	rv = sanlock_socket_address(SANLK_RUN_DIR, &addr);
 	if (rv < 0)
 		return rv;
 
diff --git a/src/sanlock_sock.c b/src/sanlock_sock.c
index e7efe8e..b83f9ae 100644
--- a/src/sanlock_sock.c
+++ b/src/sanlock_sock.c
@@ -18,12 +18,12 @@
 #include "sanlock.h"
 #include "sanlock_sock.h"
 
-int sanlock_socket_address(struct sockaddr_un *addr)
+int sanlock_socket_address(const char *dir, struct sockaddr_un *addr)
 {
 	memset(addr, 0, sizeof(struct sockaddr_un));
 	addr->sun_family = AF_LOCAL;
 	snprintf(addr->sun_path, sizeof(addr->sun_path) - 1, "%s/%s",
-		 SANLK_RUN_DIR, SANLK_SOCKET_NAME);
+		 dir, SANLK_SOCKET_NAME);
 	return 0;
 }
 
diff --git a/src/sanlock_sock.h b/src/sanlock_sock.h
index 78821c2..4ffdb70 100644
--- a/src/sanlock_sock.h
+++ b/src/sanlock_sock.h
@@ -89,7 +89,7 @@ struct sanlk_state {
 	char str[0]; /* string of internal state */
 };
 
-int sanlock_socket_address(struct sockaddr_un *addr);
+int sanlock_socket_address(const char *dir, struct sockaddr_un *addr);
 
 struct event_cb {
 	struct sm_header h;
-- 
2.9.5

[PATCH 3/7] Move SANLK_RUN_DIR to sanlock_internal.h

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

It was defined before in sanlock_sock.h, but now sanlock socket address
does not depend on this constant.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/client.c           | 1 +
 src/sanlock_internal.h | 1 +
 src/sanlock_sock.h     | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/client.c b/src/client.c
index 9c83eb7..6c0fcec 100644
--- a/src/client.c
+++ b/src/client.c
@@ -29,6 +29,7 @@
 #include <sys/un.h>
 
 #include "sanlock.h"
+#include "sanlock_internal.h"
 #include "sanlock_resource.h"
 #include "sanlock_admin.h"
 #include "sanlock_sock.h"
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index e75979d..46581b7 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -43,6 +43,7 @@
 
 #define COMMAND_MAX 4096
 
+#define SANLK_RUN_DIR "/var/run/sanlock"
 #define SANLK_LOG_DIR "/var/log"
 #define SANLK_LOGFILE_NAME "sanlock.log"
 #define SANLK_LOCKFILE_NAME "sanlock.pid"
diff --git a/src/sanlock_sock.h b/src/sanlock_sock.h
index 4ffdb70..9b8aad7 100644
--- a/src/sanlock_sock.h
+++ b/src/sanlock_sock.h
@@ -10,7 +10,6 @@
 #ifndef __SANLOCK_SOCK_H__
 #define __SANLOCK_SOCK_H__
 
-#define SANLK_RUN_DIR "/var/run/sanlock"
 #define SANLK_SOCKET_NAME "sanlock.sock"
 
 #define SM_MAGIC 0x04282010
-- 
2.9.5

[PATCH 4/7] Add environment variables helpers

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

These helpers make it easy to get an environment variable falling back
to default value if the environment variable is not set. This will be
used for using a special sanlock run directory and disabling privileged
operations when running sanlock in the tests.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/Makefile |  9 ++++++---
 src/env.c    | 34 ++++++++++++++++++++++++++++++++++
 src/env.h    | 15 +++++++++++++++
 3 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 src/env.c
 create mode 100644 src/env.h

diff --git a/src/Makefile b/src/Makefile
index 69fcd13..3ca25c9 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -37,7 +37,8 @@ CMD_SOURCE = \
 	monotime.c \
 	cmd.c \
 	client_cmd.c \
-	sanlock_sock.c
+	sanlock_sock.c \
+	env.c
 
 LIB_ENTIRE_SOURCE = \
 	client.c \
@@ -51,11 +52,13 @@ LIB_ENTIRE_SOURCE = \
 	task.c \
 	timeouts.c \
 	direct_lib.c \
-	monotime.c
+	monotime.c \
+	env.c
 
 LIB_CLIENT_SOURCE = \
 	client.c \
-	sanlock_sock.c
+	sanlock_sock.c \
+	env.c
 
 LIBPC_ENTIRE_SOURCE = libsanlock.pc.in
 LIBPC_CLIENT_SOURCE = libsanlock_client.pc.in
diff --git a/src/env.c b/src/env.c
new file mode 100644
index 0000000..e1d2f3a
--- /dev/null
+++ b/src/env.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License v2 or (at your option) any later version.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+#include "env.h"
+
+const char *env_get(const char *key, const char *defval)
+{
+	const char *val;
+
+	val = getenv(key);
+	if (val == NULL)
+		return defval;
+
+	return val;
+}
+
+int env_get_bool(const char *key, int defval)
+{
+	const char *val;
+
+	val = getenv(key);
+	if (val == NULL)
+		return defval;
+
+	return strcmp(val, "1") ? 0 : 1;
+}
diff --git a/src/env.h b/src/env.h
new file mode 100644
index 0000000..ca3a3ff
--- /dev/null
+++ b/src/env.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright 2018 Red Hat, Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License v2 or (at your option) any later version.
+ */
+
+#ifndef	__ENV_H__
+#define	__ENV_H__
+
+const char *env_get(const char *key, const char *defval);
+int env_get_bool(const char *key, int defval);
+
+#endif
-- 
2.9.5

[PATCH 5/7] Add SANLOCK_RUN_DIR environment variable

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

Using this environment variable, we can use custom run directory for
placing sanlock socket and lockfile. This allows running sanlock as
non-privileged user in the tests.

When running the sanlock executable, we warn if using non-standard run
directory, since this should be used only for testing.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/client.c           |  7 ++++++-
 src/main.c             | 12 +++++++++---
 src/sanlock_internal.h |  4 +++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/client.c b/src/client.c
index 6c0fcec..fce6c60 100644
--- a/src/client.c
+++ b/src/client.c
@@ -34,6 +34,7 @@
 #include "sanlock_admin.h"
 #include "sanlock_sock.h"
 #include "sanlock_rv.h"
+#include "env.h"
 
 #ifndef GNUC_UNUSED
 #define GNUC_UNUSED __attribute__((__unused__))
@@ -43,13 +44,17 @@ static int connect_socket(int *sock_fd)
 {
 	int rv, s;
 	struct sockaddr_un addr;
+	static const char *run_dir;
 
 	*sock_fd = -1;
 	s = socket(AF_LOCAL, SOCK_STREAM, 0);
 	if (s < 0)
 		return -errno;
 
-	rv = sanlock_socket_address(SANLK_RUN_DIR, &addr);
+	if (run_dir == NULL)
+		run_dir = env_get("SANLOCK_RUN_DIR", DEFAULT_RUN_DIR);
+
+	rv = sanlock_socket_address(run_dir, &addr);
 	if (rv < 0) {
 		close(s);
 		return rv;
diff --git a/src/main.c b/src/main.c
index e2c882f..747e4c2 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,6 +55,7 @@
 #include "helper.h"
 #include "timeouts.h"
 #include "paxos_lease.h"
+#include "env.h"
 
 #define SIGRUNPATH 100 /* anything that's not SIGTERM/SIGKILL */
 
@@ -86,6 +87,7 @@ static struct thread_pool pool;
 static struct random_data rand_data;
 static char rand_state[32];
 static pthread_mutex_t rand_mutex = PTHREAD_MUTEX_INITIALIZER;
+static const char *run_dir = NULL;
 
 static void close_helper(void)
 {
@@ -1290,7 +1292,7 @@ static int setup_listener(void)
 	struct sockaddr_un addr;
 	int rv, fd, ci;
 
-	rv = sanlock_socket_address(SANLK_RUN_DIR, &addr);
+	rv = sanlock_socket_address(run_dir, &addr);
 	if (rv < 0)
 		return rv;
 
@@ -1631,6 +1633,7 @@ static int do_daemon(void)
 {
 	int fd, rv;
 
+	run_dir = env_get(SANLOCK_RUN_DIR, DEFAULT_RUN_DIR);
 
 	/* This can take a while so do it before forking. */
 	setup_groups();
@@ -1665,7 +1668,10 @@ static int do_daemon(void)
 	setup_signals();
 	setup_logging();
 
-	fd = lockfile(SANLK_RUN_DIR, SANLK_LOCKFILE_NAME, com.uid, com.gid);
+	if (strcmp(run_dir, DEFAULT_RUN_DIR))
+		log_warn("Using non-standard run directory '%s'", run_dir);
+
+	fd = lockfile(run_dir, SANLK_LOCKFILE_NAME, com.uid, com.gid);
 	if (fd < 0) {
 		close_logging();
 		return fd;
@@ -1706,7 +1712,7 @@ static int do_daemon(void)
  out:
 	/* order reversed from setup so lockfile is last */
 	close_logging();
-	unlink_lockfile(fd, SANLK_RUN_DIR, SANLK_LOCKFILE_NAME);
+	unlink_lockfile(fd, run_dir, SANLK_LOCKFILE_NAME);
 	return rv;
 }
 
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index 46581b7..87d8473 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -43,7 +43,9 @@
 
 #define COMMAND_MAX 4096
 
-#define SANLK_RUN_DIR "/var/run/sanlock"
+#define SANLOCK_RUN_DIR "SANLOCK_RUN_DIR"
+#define DEFAULT_RUN_DIR "/var/run/sanlock"
+
 #define SANLK_LOG_DIR "/var/log"
 #define SANLK_LOGFILE_NAME "sanlock.log"
 #define SANLK_LOCKFILE_NAME "sanlock.pid"
-- 
2.9.5

[PATCH 6/7] Add SANLOCK_PRIVILEGED environment variable

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

Setting this variable to 0, sanlock daemon will skip operations that
requires privileges. This allows running sanlock daemon as regular user
in the tests.

When running the daemon, we log a warning if this variable is set to to
non default value.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 src/main.c             | 13 +++++++++++--
 src/sanlock_internal.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/main.c b/src/main.c
index 747e4c2..5be949b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -88,6 +88,7 @@ static struct random_data rand_data;
 static char rand_state[32];
 static pthread_mutex_t rand_mutex = PTHREAD_MUTEX_INITIALIZER;
 static const char *run_dir = NULL;
+static int privileged = 1;
 
 static void close_helper(void)
 {
@@ -1428,6 +1429,9 @@ static void setup_limits(void)
 	int rv;
 	struct rlimit rlim = { .rlim_cur = -1, .rlim_max= -1 };
 
+	if (!privileged)
+		return;
+
 	rv = setrlimit(RLIMIT_MEMLOCK, &rlim);
 	if (rv < 0) {
 		log_error("cannot set the limits for memlock %i", errno);
@@ -1451,7 +1455,7 @@ static void setup_groups(void)
 {
 	int rv;
 
-	if (!com.uname || !com.gname)
+	if (!com.uname || !com.gname || !privileged)
 		return;
 
 	rv = initgroups(com.uname, com.gid);
@@ -1464,7 +1468,7 @@ static void setup_uid_gid(void)
 {
 	int rv;
 
-	if (!com.uname || !com.gname)
+	if (!com.uname || !com.gname || !privileged)
 		return;
 
 	rv = setgid(com.gid);
@@ -1634,6 +1638,7 @@ static int do_daemon(void)
 	int fd, rv;
 
 	run_dir = env_get(SANLOCK_RUN_DIR, DEFAULT_RUN_DIR);
+	privileged = env_get_bool(SANLOCK_PRIVILEGED, 1);
 
 	/* This can take a while so do it before forking. */
 	setup_groups();
@@ -1671,6 +1676,10 @@ static int do_daemon(void)
 	if (strcmp(run_dir, DEFAULT_RUN_DIR))
 		log_warn("Using non-standard run directory '%s'", run_dir);
 
+	if (!privileged)
+		log_warn("Running in unprivileged mode");
+
+
 	fd = lockfile(run_dir, SANLK_LOCKFILE_NAME, com.uid, com.gid);
 	if (fd < 0) {
 		close_logging();
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index 87d8473..047f4db 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -45,6 +45,7 @@
 
 #define SANLOCK_RUN_DIR "SANLOCK_RUN_DIR"
 #define DEFAULT_RUN_DIR "/var/run/sanlock"
+#define SANLOCK_PRIVILEGED "SANLOCK_PRIVILEGED"
 
 #define SANLK_LOG_DIR "/var/log"
 #define SANLK_LOGFILE_NAME "sanlock.log"
-- 
2.9.5

[PATCH 7/7] Start the daemon tests module

[Nir Soffer]

From: Nir Soffer <nsoffer(a)redhat.com&gt;

The daemon test module provide the infrastructure to run sanlock daemon
in the tests as the current user. Add a simple test to initialize a
lockspace as example how to use it.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 tests/daemon_test.py | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 tests/daemon_test.py

diff --git a/tests/daemon_test.py b/tests/daemon_test.py
new file mode 100644
index 0000000..a3d299f
--- /dev/null
+++ b/tests/daemon_test.py
@@ -0,0 +1,63 @@
+"""
+Test sanlock client operations.
+"""
+
+import io
+import os
+import struct
+import subprocess
+
+import pytest
+
+tests_dir = os.path.dirname(__file__)
+SANLOCK = os.path.join(tests_dir, os.pardir, "src", "sanlock")
+
+ENV = dict(os.environ)
+ENV["SANLOCK_RUN_DIR"] = "/tmp/sanlock"
+ENV["SANLOCK_PRIVILEGED"] = "0"
+
+
+(a)pytest.fixture(scope=&quot;session&quot;)
+def sanlock_daemon():
+    cmd = [SANLOCK, "daemon",
+           # no fork and print all logging to stderr
+           "-D",
+           # don't use watchdog through wdmd
+           "-w", "0",
+           # don't use mlockall
+           "-l", "0",
+           # don't use high priority (RR) scheduling
+           "-h", "0",
+           # run as current user instead of "sanlock"
+           "-U", ENV["USER"],
+           "-G", ENV["USER"]]
+    p = subprocess.Popen(cmd, env=ENV)
+    try:
+        yield
+    finally:
+        p.terminate()
+        p.wait()
+
+
+def test_init_lockspace(tmpdir, sanlock_daemon):
+    path = tmpdir.join("lockspace")
+    with io.open(str(path), "wb") as f:
+        # Poison with junk data.
+        f.write(b"x" * 1024**2 + b"X" * 512)
+
+    lockspace = "name:1:%s:0" % path
+    run(SANLOCK, "client", "init", "-s", lockspace)
+
+    with io.open(str(path), "rb") as f:
+        magic, = struct.unpack("< I", f.read(4))
+        assert magic == 0x12212010
+
+        # TODO: check more stuff here...
+
+        # Do not modify data after the lockspace area.
+        f.seek(1024**2, io.SEEK_SET)
+        assert f.read(512) == b"X" * 512
+
+
+def run(*args):
+    return subprocess.check_output(args, env=ENV)
-- 
2.9.5