[Cluster-devel] [PATCH dlm-tool] dlm_tool: handle dynamic length lockspace dump

[Cluster-devel] [PATCH dlm-tool] dlm_tool: handle dynamic length lockspace dump

[Alexander Aring]

Currently there is a maximum lockspace dump value which I think was
there because a dlm user lockspace limitation of device minor numbers.
However for kernel lockspaces e.g. gfs2 there will no device files
created and so we can indeed have more than 128 lockspaces registered.
If this is the case dlm_tool will simple not dump any lockspace because
it hits the static pre-allocated array size to dump lockspace
information.

This patch is changing the current static behaviour to first get the
dlmc_header header and calculate the additional payload size and
allocate the dump lockspace array so that no static limitation exists
anymore.

I tested this patch series with a hacked dlm test module which registers
300 dlm lockspaces.

Reported-by: Reid Wahl <nwahl@redhat.com>
---
 dlm_controld/lib.c           | 54 +++++++++++++++++-------------------
 dlm_controld/libdlmcontrol.h |  3 +-
 dlm_controld/main.c          | 11 ++------
 dlm_tool/main.c              | 14 ++++++----
 4 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/dlm_controld/lib.c b/dlm_controld/lib.c
index 8cbdd27f..bc92a4b7 100644
--- a/dlm_controld/lib.c
+++ b/dlm_controld/lib.c
@@ -667,24 +667,11 @@ int dlmc_lockspace_info(char *name, struct dlmc_lockspace *lockspace)
 	return rv;
 }
 
-int dlmc_lockspaces(int max, int *count, struct dlmc_lockspace *lss)
+int dlmc_lockspaces(int *count, struct dlmc_lockspace **lss)
 {
-	struct dlmc_header h, *rh;
-	char *reply;
+	struct dlmc_header h;
 	int reply_len;
-	int fd, rv, result, ls_count;
-
-	init_header(&h, DLMC_CMD_LOCKSPACES, NULL, 0);
-	h.data = max;
-
-	reply_len = sizeof(struct dlmc_header) +
-		    (max * sizeof(struct dlmc_lockspace));
-	reply = malloc(reply_len);
-	if (!reply) {
-		rv = -1;
-		goto out;
-	}
-	memset(reply, 0, reply_len);
+	int fd, rv, result;
 
 	fd = do_connect(DLMC_QUERY_SOCK_PATH);
 	if (fd < 0) {
@@ -692,31 +679,40 @@ int dlmc_lockspaces(int max, int *count, struct dlmc_lockspace *lss)
 		goto out;
 	}
 
+	init_header(&h, DLMC_CMD_LOCKSPACES, NULL, 0);
+
 	rv = do_write(fd, &h, sizeof(h));
 	if (rv < 0)
 		goto out_close;
 
-	/* won't usually get back the full reply_len */
-	do_read(fd, reply, reply_len);
+	rv = do_read(fd, &h, sizeof(h));
+	if (rv <0)
+		goto out_close;
 
-	rh = (struct dlmc_header *)reply;
-	result = rh->data;
-	if (result < 0 && result != -E2BIG) {
+	result = h.data;
+	if (result < 0) {
 		rv = result;
 		goto out_close;
 	}
 
-	if (result == -E2BIG) {
-		*count = -E2BIG;
-		ls_count = max;
-	} else {
-		*count = result;
-		ls_count = result;
+	*count = result;
+
+	reply_len = h.len - sizeof(struct dlmc_header);
+	*lss = malloc(reply_len);
+	if (!*lss) {
+		rv = -1;
+		goto out;
+	}
+	memset(*lss, 0, reply_len);
+
+	rv = do_read(fd, *lss, reply_len);
+	if (rv < 0) {
+		free(*lss);
+		goto out;
 	}
+
 	rv = 0;
 
-	memcpy(lss, (char *)reply + sizeof(struct dlmc_header),
-	       ls_count * sizeof(struct dlmc_lockspace));
  out_close:
 	close(fd);
  out:
diff --git a/dlm_controld/libdlmcontrol.h b/dlm_controld/libdlmcontrol.h
index a106171b..a8654f3e 100644
--- a/dlm_controld/libdlmcontrol.h
+++ b/dlm_controld/libdlmcontrol.h
@@ -87,7 +87,8 @@ int dlmc_dump_log_plock(char *buf);
 int dlmc_dump_plocks(char *name, char *buf);
 int dlmc_lockspace_info(char *lsname, struct dlmc_lockspace *ls);
 int dlmc_node_info(char *lsname, int nodeid, struct dlmc_node *node);
-int dlmc_lockspaces(int max, int *count, struct dlmc_lockspace *lss);
+/* caller need to free *lss */
+int dlmc_lockspaces(int *count, struct dlmc_lockspace **lss);
 int dlmc_lockspace_nodes(char *lsname, int type, int max, int *count,
 			 struct dlmc_node *nodes);
 int dlmc_print_status(uint32_t flags);
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index 57844dc1..a98dae56 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -1070,7 +1070,7 @@ static void query_node_info(int fd, char *name, int nodeid)
 		 (char *)&node, sizeof(node));
 }
 
-static void query_lockspaces(int fd, int max)
+static void query_lockspaces(int fd)
 {
 	int ls_count = 0;
 	struct dlmc_lockspace *lss = NULL;
@@ -1083,12 +1083,7 @@ static void query_lockspaces(int fd, int max)
 		goto out;
 	}
 
-	if (ls_count > max) {
-		result = -E2BIG;
-		ls_count = max;
-	} else {
-		result = ls_count;
-	}
+	result = ls_count;
  out:
 	do_reply(fd, DLMC_CMD_LOCKSPACES, NULL, result, 0,
 		 (char *)lss, ls_count * sizeof(struct dlmc_lockspace));
@@ -1375,7 +1370,7 @@ static void *process_queries(void *arg)
 			query_node_info(f, h.name, h.data);
 			break;
 		case DLMC_CMD_LOCKSPACES:
-			query_lockspaces(f, h.data);
+			query_lockspaces(f);
 			break;
 		case DLMC_CMD_LOCKSPACE_NODES:
 			query_lockspace_nodes(f, h.name, h.option, h.data);
diff --git a/dlm_tool/main.c b/dlm_tool/main.c
index 04ff40f8..f4e69649 100644
--- a/dlm_tool/main.c
+++ b/dlm_tool/main.c
@@ -67,7 +67,6 @@ static int summarize;
 char run_command[DLMC_RUN_COMMAND_LEN];
 char run_uuid[DLMC_RUN_UUID_LEN];
 
-#define MAX_LS 128
 #define MAX_NODES 128
 
 /* from linux/fs/dlm/dlm_internal.h */
@@ -91,7 +90,7 @@ char run_uuid[DLMC_RUN_UUID_LEN];
 #define DLM_MSG_PURGE           14
 
 
-struct dlmc_lockspace lss[MAX_LS];
+struct dlmc_lockspace *lss;
 struct dlmc_node nodes[MAX_NODES];
 
 struct rinfo {
@@ -1410,13 +1409,16 @@ static void do_list(char *name)
 	int rv;
 	int i;
 
-	memset(lss, 0, sizeof(lss));
-
 	if (name) {
+		/* get only one specific lockspace by name */
 		ls_count = 1;
+		lss = malloc(sizeof(struct dlmc_lockspace));
+		if (!lss)
+			exit(EXIT_FAILURE);
+
 		rv = dlmc_lockspace_info(name, lss);
 	} else {
-		rv = dlmc_lockspaces(MAX_LS, &ls_count, lss);
+		rv = dlmc_lockspaces(&ls_count, &lss);
 	}
 
 	if (rv < 0)
@@ -1450,6 +1452,8 @@ static void do_list(char *name)
  next:
 		printf("\n");
 	}
+
+	free(lss);
 }
 
 static void do_deadlock_check(char *name)
-- 
2.31.1

[Cluster-devel] [PATCH dlm-tool] dlm_tool: handle dynamic length lockspace dump

[Alexander Aring]

Hi,

On Tue, Mar 8, 2022 at 3:21 PM Alexander Aring <aahringo@redhat.com> wrote:
...
> +++ b/dlm_controld/main.c
> @@ -1070,7 +1070,7 @@ static void query_node_info(int fd, char *name, int nodeid)
>                  (char *)&node, sizeof(node));
>  }
>
> -static void query_lockspaces(int fd, int max)
> +static void query_lockspaces(int fd)
>  {
>         int ls_count = 0;
>         struct dlmc_lockspace *lss = NULL;
> @@ -1083,12 +1083,7 @@ static void query_lockspaces(int fd, int max)
>                 goto out;
>         }
>
> -       if (ls_count > max) {
> -               result = -E2BIG;
> -               ls_count = max;
> -       } else {
> -               result = ls_count;
> -       }
> +       result = ls_count;
>   out:
>         do_reply(fd, DLMC_CMD_LOCKSPACES, NULL, result, 0,
>                  (char *)lss, ls_count * sizeof(struct dlmc_lockspace));
> @@ -1375,7 +1370,7 @@ static void *process_queries(void *arg)
>                         query_node_info(f, h.name, h.data);
>                         break;
>                 case DLMC_CMD_LOCKSPACES:
> -                       query_lockspaces(f, h.data);
> +                       query_lockspaces(f);
>                         break;
>                 case DLMC_CMD_LOCKSPACE_NODES:
>                         query_lockspace_nodes(f, h.name, h.option, h.data);
> diff --git a/dlm_tool/main.c b/dlm_tool/main.c
> index 04ff40f8..f4e69649 100644
> --- a/dlm_tool/main.c
> +++ b/dlm_tool/main.c
> @@ -67,7 +67,6 @@ static int summarize;
>  char run_command[DLMC_RUN_COMMAND_LEN];
>  char run_uuid[DLMC_RUN_UUID_LEN];
>
> -#define MAX_LS 128
>  #define MAX_NODES 128
>
>  /* from linux/fs/dlm/dlm_internal.h */
> @@ -91,7 +90,7 @@ char run_uuid[DLMC_RUN_UUID_LEN];
>  #define DLM_MSG_PURGE           14
>
>
> -struct dlmc_lockspace lss[MAX_LS];
> +struct dlmc_lockspace *lss;

there should be no need anymore to handle this globally, we should be
able to move this pointer into the function. I will fix that as soon
as I apply this patch.

- Alex