[Cluster-devel] [PATCHv2 dlm-tool 1/3] dlm_controld: increase uevent recv buffer

[Cluster-devel] [PATCHv2 dlm-tool 1/3] dlm_controld: increase uevent recv buffer

[Alexander Aring]

This patch increases the uevent recv buffer from 256 bytes to 4096
bytes. To ensure everything fits into one recv() call.
---
 dlm_controld/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index 7cf6348e..2c534a1e 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -682,7 +682,7 @@ const char *dlm_mode_str(int mode)
 
 /* recv "online" (join) and "offline" (leave) messages from dlm via uevents */
 
-#define MAX_LINE_UEVENT 256
+#define MAX_LINE_UEVENT 4096
 
 static void process_uevent(int ci)
 {
-- 
2.31.1

[Cluster-devel] [PATCHv2 dlm-tool 2/3] dlm_controld: constify lsnames

[Alexander Aring]

This patch constify some ls name parameter which are used read only in
ls_create() and ls_find().
---
 dlm_controld/dlm_daemon.h | 2 +-
 dlm_controld/main.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
index b829e0de..f0bad90f 100644
--- a/dlm_controld/dlm_daemon.h
+++ b/dlm_controld/dlm_daemon.h
@@ -479,7 +479,7 @@ int client_add(int fd, void (*workfn)(int ci), void (*deadfn)(int ci));
 int client_fd(int ci);
 void client_ignore(int ci, int fd);
 void client_back(int ci, int fd);
-struct lockspace *find_ls(char *name);
+struct lockspace *find_ls(const char *name);
 struct lockspace *find_ls_id(uint32_t id);
 const char *dlm_mode_str(int mode);
 void cluster_dead(int ci);
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index 2c534a1e..31489d54 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -537,7 +537,7 @@ static int check_run_operation(char *uuid_str, uint32_t flags, struct dlmc_run_c
 	return 0;
 }
 
-static struct lockspace *create_ls(char *name)
+static struct lockspace *create_ls(const char *name)
 {
 	struct lockspace *ls;
 
@@ -562,7 +562,7 @@ static struct lockspace *create_ls(char *name)
 	return ls;
 }
 
-struct lockspace *find_ls(char *name)
+struct lockspace *find_ls(const char *name)
 {
 	struct lockspace *ls;
 
-- 
2.31.1

[Cluster-devel] [PATCHv2 dlm-tool 3/3] dlm_controld: better uevent filtering

[Alexander Aring]

When I did test with dlm_locktorture module I got several log messages
about:

uevent message has 3 args: add@/module/dlm_locktorture
uevent message has 3 args: remove@/module/dlm_locktorture

which are not expected and not able to parse by dlm_controld
process_uevent() function, because mismatch of argument counts.
Debugging it more, I figured out that those uevents are for
loading/unloading the dlm_locktorture module and there are uevents for
loading and unloading modules which have nothing todo with dlm lockspace
uevent handling.

The current filter works as:

if (!strstr(buf, "dlm"))

for matching the dlm joining/leaving uevent string which looks like:

offline@/kernel/dlm/locktorture

to avoid matching with other uevent which has somehow the string "dlm"
in it, we switch to the match the uevent env variables for action,
devpath (just to check if it's set) and subsystem. Additional the dlm
subsystem sets the LOCKSPACE variable which can be used to get the
lockspace name instead of extracting it previously from the devpath.

The code to decode the uevent envs were taken from the gfs2_controld
utility [0].

[0] https://github.com/andyprice/gfs2-utils/blob/91c3e9a69ed70d3d522f5b47015da5e5868722ec/group/gfs_controld/main.c
---
 dlm_controld/main.c | 126 +++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 55 deletions(-)

diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index 31489d54..c9d1c5f1 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -46,6 +46,50 @@ struct client {
 	struct lockspace *ls;
 };
 
+enum {
+	Env_ACTION = 0,
+	Env_DEVPATH,
+	Env_SUBSYSTEM,
+	Env_LOCKSPACE,
+	Env_Last, /* Flag for end of vars */
+};
+
+static const char *uevent_vars[] = {
+	[Env_ACTION]		= "ACTION=",
+	[Env_DEVPATH]		= "DEVPATH=",
+	[Env_SUBSYSTEM]		= "SUBSYSTEM=",
+	[Env_LOCKSPACE]		= "LOCKSPACE=",
+};
+
+static void decode_uevent(const char *buf, unsigned len, const char *vars[],
+			  unsigned nvars, const char *vals[])
+{
+	const char *ptr;
+	unsigned int i;
+	int slen, vlen;
+
+	memset(vals, 0, sizeof(const char *) * nvars);
+
+	while (len > 0) {
+		ptr = buf;
+		slen = strlen(ptr);
+		buf += slen;
+		len -= slen;
+		buf++;
+		len--;
+
+		for (i = 0; i < nvars; i++) {
+			vlen = strlen(vars[i]);
+			if (vlen > slen)
+				continue;
+			if (memcmp(vars[i], ptr, vlen) != 0)
+				continue;
+			vals[i] = ptr + vlen;
+			break;
+		}
+	}
+}
+
 int do_read(int fd, void *buf, size_t count)
 {
 	int rv, off = 0;
@@ -627,38 +671,6 @@ static void fs_register_del(char *name)
 	}
 }
 
-#define MAXARGS 8
-
-static char *get_args(char *buf, int *argc, char **argv, char sep, int want)
-{
-	char *p = buf, *rp = NULL;
-	int i;
-
-	argv[0] = p;
-
-	for (i = 1; i < MAXARGS; i++) {
-		p = strchr(buf, sep);
-		if (!p)
-			break;
-		*p = '\0';
-
-		if (want == i) {
-			rp = p + 1;
-			break;
-		}
-
-		argv[i] = p + 1;
-		buf = p + 1;
-	}
-	*argc = i;
-
-	/* we ended by hitting \0, return the point following that */
-	if (!rp)
-		rp = strchr(buf, '\0') + 1;
-
-	return rp;
-}
-
 const char *dlm_mode_str(int mode)
 {
 	switch (mode) {
@@ -686,13 +698,12 @@ const char *dlm_mode_str(int mode)
 
 static void process_uevent(int ci)
 {
+	const char *uevent_vals[Env_Last];
 	struct lockspace *ls;
 	char buf[MAX_LINE_UEVENT];
-	char *argv[MAXARGS], *act, *sys;
-	int rv, argc = 0;
+	int rv;
 
 	memset(buf, 0, sizeof(buf));
-	memset(argv, 0, sizeof(char *) * MAXARGS);
 
  retry_recv:
 	rv = recv(client[ci].fd, &buf, sizeof(buf), 0);
@@ -704,35 +715,38 @@ static void process_uevent(int ci)
 		return;
 	}
 
-	if (!strstr(buf, "dlm"))
-		return;
+	decode_uevent(buf, rv, uevent_vars, Env_Last, uevent_vals);
 
-	log_debug("uevent: %s", buf);
-
-	get_args(buf, &argc, argv, '/', 4);
-	if (argc != 4)
-		log_error("uevent message has %d args", argc);
-	act = argv[0];
-	sys = argv[2];
-
-	if (!act || !sys || !argv[3])
+	if (!uevent_vals[Env_ACTION] ||
+	    !uevent_vals[Env_DEVPATH] ||
+	    !uevent_vals[Env_SUBSYSTEM] ||
+	    !uevent_vals[Env_LOCKSPACE]) {
+		log_debug("failed to validate uevent, action: %p, devpath: %p, subsystem: %p, lockspace: %p",
+			  uevent_vals[Env_ACTION], uevent_vals[Env_DEVPATH],
+			  uevent_vals[Env_SUBSYSTEM],
+			  uevent_vals[Env_LOCKSPACE]);
 		return;
+	}
 
-	if (strncmp(sys, "dlm", 3))
+	if (strcmp(uevent_vals[Env_SUBSYSTEM], "dlm")) {
+		log_debug("uevent looks like dlm but came not from dlm subsystem");
 		return;
+	}
 
-	log_debug("kernel: %s %s", act, argv[3]);
+	log_debug("uevent action: %s, devpath: %s, devpath: %s, lockspace: %s",
+		  uevent_vals[Env_ACTION], uevent_vals[Env_SUBSYSTEM],
+		  uevent_vals[Env_DEVPATH], uevent_vals[Env_LOCKSPACE]);
 
 	rv = 0;
 
-	if (!strcmp(act, "online@")) {
-		ls = find_ls(argv[3]);
+	if (!strcmp(uevent_vals[Env_ACTION], "online")) {
+		ls = find_ls(uevent_vals[Env_LOCKSPACE]);
 		if (ls) {
 			rv = -EEXIST;
 			goto out;
 		}
 
-		ls = create_ls(argv[3]);
+		ls = create_ls(uevent_vals[Env_LOCKSPACE]);
 		if (!ls) {
 			rv = -ENOMEM;
 			goto out;
@@ -747,8 +761,8 @@ static void process_uevent(int ci)
 			goto out;
 		}
 
-	} else if (!strcmp(act, "offline@")) {
-		ls = find_ls(argv[3]);
+	} else if (!strcmp(uevent_vals[Env_ACTION], "offline")) {
+		ls = find_ls(uevent_vals[Env_LOCKSPACE]);
 		if (!ls) {
 			rv = -ENOENT;
 			goto out;
@@ -758,8 +772,10 @@ static void process_uevent(int ci)
 	}
  out:
 	if (rv < 0)
-		log_error("process_uevent %s error %d errno %d",
-			  act, rv, errno);
+		log_error("%s action: %s, devpath: %s, devpath: %s, lockspace: %s - error %d errno %d",
+			  __func__, uevent_vals[Env_ACTION],
+			  uevent_vals[Env_SUBSYSTEM], uevent_vals[Env_DEVPATH],
+			  uevent_vals[Env_LOCKSPACE], rv, errno);
 }
 
 static int setup_uevent(void)
-- 
2.31.1