RE: [PATCH 1/4] FIX: Takeover mdmon when grow is continued from command line

-----Original Message-----
From: NeilBrown [mailto:neilb@suse.de]
Sent: Monday, January 30, 2012 1:24 AM
To: Kwolek, Adam
Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin; Williams, Dan
J
Subject: Re: [PATCH 1/4] FIX: Takeover mdmon when grow is continued from
command line

On Thu, 12 Jan 2012 08:12:31 +0100 Adam Kwolek [off-list ref]
wrote:

quoted

When reshape with '--continue' option is executed mdadm doesn't know,
if mdmon is switched to using '--takeover' option to current file
system context. To resolve this, on reshape continuation when mdmon is
run, execute mdmon takeover.

Lack of takeovered mdmon can cause e.g.:
- multivolume reshape is broken when reshaped volume is changed
- size of reshaped volume dowsn't increase

I don't think this is right.

In particular, the way we seem to be going with systemd means that we might
not end up using '--takeover' at all.  So performing a --takeover transparently
without being explicitly asked to is not a good idea.

--continue is only used once the final root is mounted and all the early boot
complications are out of the way.
So can't we simple require that --takeover - if it is being used - be performed
before any --continue??

i.e. is there a reason we cannot ensure this works properly simply by making sure
boot scripts do the right things in the right order.

NeilBrown

This particular patch was written for process automation only. 
What about remaining patches from this series?

BR
Adam

quoted

Signed-off-by: Adam Kwolek <redacted>
---

 Grow.c  |    2 ++
 mdadm.h |    1 +
 util.c  |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/Grow.c b/Grow.c
index b2c1360..f0422d2 100644
--- a/Grow.c
+++ b/Grow.c

@@ -3876,6 +3876,8 @@ int Grow_continue_command(char *devname, int

fd,

quoted

 		 */
 		if (!mdmon_running(container_dev))
 			start_mdmon(container_dev);
+		else
+			takeover_mdmon(container_dev);
 		ping_monitor(buf);

 		if (mdmon_running(container_dev))

diff --git a/mdadm.h b/mdadm.h
index 381ef86..f274ae7 100644
--- a/mdadm.h
+++ b/mdadm.h

@@ -1223,6 +1223,7 @@ extern int mdmon_pid(int devnum);  extern int

check_env(char *name);  extern __u32 random32(void);  extern int
start_mdmon(int devnum);
+extern int takeover_mdmon(int devnum);

 extern int child_monitor(int afd, struct mdinfo *sra, struct reshape *reshape,
 			 struct supertype *st, unsigned long stripes, diff --git

a/util.c

quoted

b/util.c index 6985a70..e95a366 100644

--- a/util.c
+++ b/util.c

@@ -1581,11 +1581,13 @@ int mdmon_running(int devnum)
 	return 0;
 }

-int start_mdmon(int devnum)
+#define MDMON_TAKEOVER_WAIT_COUNTER 15 int execute_mdmon(int

devnum,

quoted

+int takeover)
 {
 	int i, skipped;
 	int len;
-	pid_t pid;
+	pid_t pid;
+	pid_t pid_org;
 	int status;
 	char pathbuf[1024];
 	char *paths[4] = {

@@ -1611,6 +1613,12 @@ int start_mdmon(int devnum)
 	} else
 		pathbuf[0] = '\0';

+	if (takeover) {
+		pid_org = mdmon_pid(devnum);
+		if (pid_org < 0)
+			takeover = 0;
+	}
+
 	switch(fork()) {
 	case 0:
 		/* FIXME yuk. CLOSE_EXEC?? */

@@ -1620,24 +1628,51 @@ int start_mdmon(int devnum)
 				skipped++;
 			else
 				skipped = 0;
-
-		for (i=0; paths[i]; i++)
-			if (paths[i][0])
-				execl(paths[i], "mdmon",
-				      devnum2devname(devnum),
-				      NULL);
+		for (i = 0; paths[i]; i++) {
+			if (paths[i][0]) {
+				if (!takeover)
+					execl(paths[i], "mdmon",
+					      devnum2devname(devnum),
+					      NULL);
+				else
+					execl(paths[i], "mdmon",
+					      "--takeover",
+					      devnum2devname(devnum),
+					      NULL);
+			}
+		}
 		exit(1);
 	case -1: fprintf(stderr, Name ": cannot run mdmon. "
 			 "Array remains readonly\n");
 		return -1;
 	default: /* parent - good */
-		pid = wait(&status);
+		i = 0;
+		do {
+			if (i)
+				sleep(1);
+			pid = wait(&status);
+			if (takeover && (pid == pid_org))
+				pid = -1;
+			i++;
+		} while (takeover &&
+			 (i < MDMON_TAKEOVER_WAIT_COUNTER) &&
+			 (pid < 0));
 		if (pid < 0 || status != 0)
 			return -1;
 	}
 	return 0;
 }

+int start_mdmon(int devnum)
+{
+	return execute_mdmon(devnum, 0);
+}
+
+int takeover_mdmon(int devnum)
+{
+	return execute_mdmon(devnum, 1);
+}
+
 int check_env(char *name)
 {
 	char *val = getenv(name);