[PATCH v3 0/4] add --offroot argument to mdadm/mdmon

[PATCH v3 0/4] add --offroot argument to mdadm/mdmon

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

Here is v3, also known as the perfect<tm> version of the of the
systemd/dracut fixes for preboot/offroot/initrd handling in
mdadm/mdmon. I have changed the argument now to --offroot based on the
input I received. 

Cheers,
Jes


Jes Sorensen (4):
  mdmon: Use getopt_long() to parse command line options
  Add --offroot argument to mdadm
  Add --offroot argument to mdmon
  Spawn mdmon with --offroot if mdadm was launched with --offroot

 ReadMe.c   |    5 +++++
 mdadm.8.in |   12 ++++++++++++
 mdadm.c    |   10 ++++++++++
 mdadm.h    |    3 +++
 mdmon.8    |   12 +++++++++++-
 mdmon.c    |   59 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 util.c     |   17 +++++++++++++----
 7 files changed, 102 insertions(+), 16 deletions(-)

-- 
1.7.8.3

[PATCH 1/4] mdmon: Use getopt_long() to parse command line options

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This changes mdmon over to use getopt_long() for option parsing,
making it easier to add new options. In addition this patch introduces
a short version -t for --takeover and adds -h/--help.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdmon.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index b6ae0e6..a65c4a4 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -265,7 +265,14 @@ static int do_fork(void)
 
 void usage(void)
 {
-	fprintf(stderr, "Usage: mdmon [--all] [--takeover] CONTAINER\n");
+	fprintf(stderr,
+"Usage: mdmon [options] CONTAINER\n"
+"\n"
+"Options are:\n"
+"  --help        -h   : This message\n"
+"  --all              : All devices\n"
+"  --takeover    -t   : Takeover container\n"
+);
 	exit(2);
 }
 
@@ -277,25 +284,47 @@ int main(int argc, char *argv[])
 	int devnum;
 	char *devname;
 	int status = 0;
-	int arg;
+	int opt;
 	int all = 0;
 	int takeover = 0;
-
-	for (arg = 1; arg < argc; arg++) {
-		if (strncmp(argv[arg], "--all",5) == 0 ||
-		    strcmp(argv[arg], "/proc/mdstat") == 0) {
-			container_name = argv[arg];
+	static struct option options[] = {
+		{"all", 0, NULL, 'a'},
+		{"takeover", 0, NULL, 't'},
+		{"help", 0, NULL, 'h'},
+		{NULL, 0, NULL, 0}
+	};
+
+	while ((opt = getopt_long(argc, argv, "th", options, NULL)) != -1) {
+		switch (opt) {
+		case 'a':
+			container_name = argv[optind-1];
 			all = 1;
-		} else if (strcmp(argv[arg], "--takeover") == 0)
+			break;
+		case 't':
+			container_name = optarg;
 			takeover = 1;
-		else if (container_name == NULL)
-			container_name = argv[arg];
-		else
+			break;
+		case 'h':
+		default:
 			usage();
+			break;
+		}
 	}
+
+	if (all == 0 && container_name == NULL) {
+		if (argv[optind])
+			container_name = argv[optind];
+	}
+
 	if (container_name == NULL)
 		usage();
 
+	if (argc - optind > 1)
+		usage();
+
+	if (strcmp(container_name, "/proc/mdstat") == 0)
+		all = 1;
+
 	if (all) {
 		struct mdstat_ent *mdstat, *e;
 		int container_len = strlen(container_name);
-- 
1.7.8.3

[PATCH 2/4] Add --offroot argument to mdadm

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

When --offroot is specified, mdadm will change the first character of
argv[0] to '@'. This is used to signal to systemd that mdadm was
launched from initramfs and should not be shut down before returning
to the initramfs.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 ReadMe.c   |    5 +++++
 mdadm.8.in |   12 ++++++++++++
 mdadm.c    |    9 +++++++++
 mdadm.h    |    1 +
 4 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/ReadMe.c b/ReadMe.c
index 9aa798b..a60e11b 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -111,6 +111,7 @@ struct option long_options[] = {
     {"kill-subarray", 1, 0, KillSubarray},
     {"update-subarray", 1, 0, UpdateSubarray},
     {"udev-rules", 2, 0, UdevRules},
+    {"offroot", 0, 0, OffRootOpt},
 
     /* synonyms */
     {"monitor",   0, 0, 'F'},
@@ -268,6 +269,10 @@ char OptionHelp[] =
 "  --query       -Q   : Display general information about how a\n"
 "                       device relates to the md driver\n"
 "  --auto-detect      : Start arrays auto-detected by the kernel\n"
+"  --offroot          : Set first character of argv[0] to @ to indicate the\n"
+"                       application was launched from initrd/initramfs and\n"
+"                       should not be shutdown by systemd as part of the\n"
+"                       regular shutdown process.\n"
 ;
 /*
 "\n"
diff --git a/mdadm.8.in b/mdadm.8.in
index 27be110..4f06a88 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -255,6 +255,18 @@ Avoid printing purely informative messages.  With this,
 will be silent unless there is something really important to report.
 
 .TP
+.BR \-\-offroot
+Set first character of argv[0] to @ to indicate mdadm was launched
+from initrd/initramfs and should not be shutdown by systemd as part of
+the regular shutdown process. This option is normally only used by
+the system's initscripts. Please see here for more details on how
+systemd handled argv[0]:
+.IP
+.B http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons
+.PP
+
+
+.TP
 .BR \-f ", " \-\-force
 Be more forceful about certain operations.  See the various modes for
 the exact meaning of this option in different contexts.
diff --git a/mdadm.c b/mdadm.c
index f07fac2..78f28d5 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -174,6 +174,15 @@ int main(int argc, char *argv[])
 				homehost = optarg;
 			continue;
 
+		/*
+		 * --offroot sets first char of argv[0] to @. This is used
+		 * by systemd to signal that the tast was launched from
+		 * initrd/initramfs and should be preserved during shutdown
+		 */
+		case OffRootOpt:
+			argv[0][0] = '@';
+			continue;
+
 		case ':':
 		case '?':
 			fputs(Usage, stderr);
diff --git a/mdadm.h b/mdadm.h
index 381ef86..fec93aa 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -321,6 +321,7 @@ enum special_options {
 	UdevRules,
 	FreezeReshape,
 	Continue,
+	OffRootOpt,
 };
 
 /* structures read from config file */
-- 
1.7.8.3

[PATCH 3/4] Add --offroot argument to mdmon

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdmon.8 |   12 +++++++++++-
 mdmon.c |    8 ++++++++
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/mdmon.8 b/mdmon.8
index 8c1ce5f..a9178b6 100644
--- a/mdmon.8
+++ b/mdmon.8
@@ -5,7 +5,7 @@ mdmon \- monitor MD external metadata arrays
 
 .SH SYNOPSIS
 
-.BI mdmon " [--all] [--takeover] CONTAINER"
+.BI mdmon " [--all] [--takeover] [--offroot] CONTAINER"
 
 .SH OVERVIEW
 The 2.6.27 kernel brings the ability to support external metadata arrays.
@@ -165,6 +165,16 @@ argument is over-written with the name of the container.  To allow for
 containers with names longer than 5 characters, this argument can be
 arbitrarily extended, e.g. to
 .BR \-\-all-active-arrays .
+.TP
+.BR \-\-offroot
+Set first character of argv[0] to @ to indicate mdmon was launched
+from initrd/initramfs and should not be shutdown by systemd as part of
+the regular shutdown process. This option is normally only used by
+the system's initscripts. Please see here for more details on how
+systemd handled argv[0]:
+.IP
+.B http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons
+.PP
 
 .PP
 Note that
diff --git a/mdmon.c b/mdmon.c
index a65c4a4..2093476 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -272,6 +272,10 @@ void usage(void)
 "  --help        -h   : This message\n"
 "  --all              : All devices\n"
 "  --takeover    -t   : Takeover container\n"
+"  --offroot          : Set first character of argv[0] to @ to indicate the\n"
+"                       application was launched from initrd/initramfs and\n"
+"                       should not be shutdown by systemd as part of the\n"
+"                       regular shutdown process.\n"
 );
 	exit(2);
 }
@@ -291,6 +295,7 @@ int main(int argc, char *argv[])
 		{"all", 0, NULL, 'a'},
 		{"takeover", 0, NULL, 't'},
 		{"help", 0, NULL, 'h'},
+		{"offroot", 0, NULL, OffRootOpt},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -304,6 +309,9 @@ int main(int argc, char *argv[])
 			container_name = optarg;
 			takeover = 1;
 			break;
+		case OffRootOpt:
+			argv[0][0] = '@';
+			break;
 		case 'h':
 		default:
 			usage();
-- 
1.7.8.3

[PATCH 4/4] Spawn mdmon with --offroot if mdadm was launched with --offroot

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdadm.c |    1 +
 mdadm.h |    2 ++
 util.c  |   17 +++++++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/mdadm.c b/mdadm.c
index 78f28d5..1efa3e8 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -181,6 +181,7 @@ int main(int argc, char *argv[])
 		 */
 		case OffRootOpt:
 			argv[0][0] = '@';
+			__offroot = 1;
 			continue;
 
 		case ':':
diff --git a/mdadm.h b/mdadm.h
index fec93aa..d862b3e 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1383,3 +1383,5 @@ static inline int xasprintf(char **strp, const char *fmt, ...) {
 
 #define PROCESS_DELAYED -2
 #define PROCESS_PENDING -3
+
+extern int __offroot;
diff --git a/util.c b/util.c
index 6985a70..4ba44e6 100644
--- a/util.c
+++ b/util.c
@@ -32,6 +32,8 @@
 #include	<dirent.h>
 #include	<signal.h>
 
+int __offroot;
+
 /*
  * following taken from linux/blkpg.h because they aren't
  * anywhere else and it isn't safe to #include linux/ * stuff.
@@ -1622,10 +1624,17 @@ int start_mdmon(int devnum)
 				skipped = 0;
 
 		for (i=0; paths[i]; i++)
-			if (paths[i][0])
-				execl(paths[i], "mdmon",
-				      devnum2devname(devnum),
-				      NULL);
+			if (paths[i][0]) {
+				if (__offroot) {
+					execl(paths[i], "mdmon", "--offroot",
+					      devnum2devname(devnum),
+					      NULL);
+				} else {
+					execl(paths[i], "mdmon",
+					      devnum2devname(devnum),
+					      NULL);
+				}
+			}
 		exit(1);
 	case -1: fprintf(stderr, Name ": cannot run mdmon. "
 			 "Array remains readonly\n");
-- 
1.7.8.3

Re: [PATCH v3 0/4] add --offroot argument to mdadm/mdmon

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On 1/25/2012 10:18 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> Here is v3, also known as the perfect<tm> version of the of the
> systemd/dracut fixes for preboot/offroot/initrd handling in
> mdadm/mdmon. I have changed the argument now to --offroot based on the
> input I received. 
> 
> Cheers,
> Jes
> 
> 
> Jes Sorensen (4):
>   mdmon: Use getopt_long() to parse command line options
>   Add --offroot argument to mdadm
>   Add --offroot argument to mdmon
>   Spawn mdmon with --offroot if mdadm was launched with --offroot
> 
>  ReadMe.c   |    5 +++++
>  mdadm.8.in |   12 ++++++++++++
>  mdadm.c    |   10 ++++++++++
>  mdadm.h    |    3 +++
>  mdmon.8    |   12 +++++++++++-
>  mdmon.c    |   59 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  util.c     |   17 +++++++++++++----
>  7 files changed, 102 insertions(+), 16 deletions(-)
> 

Acked-by: Doug Ledford <dledford@redhat.com>


-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD
	      http://people.redhat.com/dledford

Infiniband specific RPMs available at
	      http://people.redhat.com/dledford/Infiniband


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 898 bytes --]

Re: [PATCH 4/4] Spawn mdmon with --offroot if mdadm was launched with --offroot

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]

On Wed, 25 Jan 2012 16:18:04 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  mdadm.c |    1 +
>  mdadm.h |    2 ++
>  util.c  |   17 +++++++++++++----
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/mdadm.c b/mdadm.c
> index 78f28d5..1efa3e8 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -181,6 +181,7 @@ int main(int argc, char *argv[])
>  		 */
>  		case OffRootOpt:
>  			argv[0][0] = '@';
> +			__offroot = 1;
>  			continue;
>  
>  		case ':':
> diff --git a/mdadm.h b/mdadm.h
> index fec93aa..d862b3e 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1383,3 +1383,5 @@ static inline int xasprintf(char **strp, const char *fmt, ...) {
>  
>  #define PROCESS_DELAYED -2
>  #define PROCESS_PENDING -3
> +
> +extern int __offroot;
> diff --git a/util.c b/util.c
> index 6985a70..4ba44e6 100644
> --- a/util.c
> +++ b/util.c
> @@ -32,6 +32,8 @@
>  #include	<dirent.h>
>  #include	<signal.h>
>  
> +int __offroot;
> +
>  /*
>   * following taken from linux/blkpg.h because they aren't
>   * anywhere else and it isn't safe to #include linux/ * stuff.
> @@ -1622,10 +1624,17 @@ int start_mdmon(int devnum)
>  				skipped = 0;
>  
>  		for (i=0; paths[i]; i++)
> -			if (paths[i][0])
> -				execl(paths[i], "mdmon",
> -				      devnum2devname(devnum),
> -				      NULL);
> +			if (paths[i][0]) {
> +				if (__offroot) {
> +					execl(paths[i], "mdmon", "--offroot",
> +					      devnum2devname(devnum),
> +					      NULL);

I just occurred to me that this could be:

    execl(paths[i], "@mdmon", devnum2devname(devnum));

which seems simpler... but I'll leave it as it is.

All applied - thanks.

NeilBrown


> +				} else {
> +					execl(paths[i], "mdmon",
> +					      devnum2devname(devnum),
> +					      NULL);
> +				}
> +			}
>  		exit(1);
>  	case -1: fprintf(stderr, Name ": cannot run mdmon. "
>  			 "Array remains readonly\n");


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 4/4] Spawn mdmon with --offroot if mdadm was launched with --offroot

[Jes Sorensen]

On 01/30/12 02:10, NeilBrown wrote:
>> @@ -1622,10 +1624,17 @@ int start_mdmon(int devnum)
>> >  				skipped = 0;
>> >  
>> >  		for (i=0; paths[i]; i++)
>> > -			if (paths[i][0])
>> > -				execl(paths[i], "mdmon",
>> > -				      devnum2devname(devnum),
>> > -				      NULL);
>> > +			if (paths[i][0]) {
>> > +				if (__offroot) {
>> > +					execl(paths[i], "mdmon", "--offroot",
>> > +					      devnum2devname(devnum),
>> > +					      NULL);
> I just occurred to me that this could be:
> 
>     execl(paths[i], "@mdmon", devnum2devname(devnum));
> 
> which seems simpler... but I'll leave it as it is.

Hadn't thought of that :)

> All applied - thanks.

Excellent, will you be able to push it out today? As soon as I see it in
your git repo, I will push it into Fedora rawhide so we can get it tested.

Cheers,
Jes

[PATCH 1/4] mdmon: Use getopt_long() to parse command line options

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This changes mdmon over to use getopt_long() for option parsing,
making it easier to add new options. In addition this patch introduces
a short version -t for --takeover and adds -h/--help.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdmon.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index b6ae0e6..a65c4a4 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -265,7 +265,14 @@ static int do_fork(void)
 
 void usage(void)
 {
-	fprintf(stderr, "Usage: mdmon [--all] [--takeover] CONTAINER\n");
+	fprintf(stderr,
+"Usage: mdmon [options] CONTAINER\n"
+"\n"
+"Options are:\n"
+"  --help        -h   : This message\n"
+"  --all              : All devices\n"
+"  --takeover    -t   : Takeover container\n"
+);
 	exit(2);
 }
 
@@ -277,25 +284,47 @@ int main(int argc, char *argv[])
 	int devnum;
 	char *devname;
 	int status = 0;
-	int arg;
+	int opt;
 	int all = 0;
 	int takeover = 0;
-
-	for (arg = 1; arg < argc; arg++) {
-		if (strncmp(argv[arg], "--all",5) == 0 ||
-		    strcmp(argv[arg], "/proc/mdstat") == 0) {
-			container_name = argv[arg];
+	static struct option options[] = {
+		{"all", 0, NULL, 'a'},
+		{"takeover", 0, NULL, 't'},
+		{"help", 0, NULL, 'h'},
+		{NULL, 0, NULL, 0}
+	};
+
+	while ((opt = getopt_long(argc, argv, "th", options, NULL)) != -1) {
+		switch (opt) {
+		case 'a':
+			container_name = argv[optind-1];
 			all = 1;
-		} else if (strcmp(argv[arg], "--takeover") == 0)
+			break;
+		case 't':
+			container_name = optarg;
 			takeover = 1;
-		else if (container_name == NULL)
-			container_name = argv[arg];
-		else
+			break;
+		case 'h':
+		default:
 			usage();
+			break;
+		}
 	}
+
+	if (all == 0 && container_name == NULL) {
+		if (argv[optind])
+			container_name = argv[optind];
+	}
+
 	if (container_name == NULL)
 		usage();
 
+	if (argc - optind > 1)
+		usage();
+
+	if (strcmp(container_name, "/proc/mdstat") == 0)
+		all = 1;
+
 	if (all) {
 		struct mdstat_ent *mdstat, *e;
 		int container_len = strlen(container_name);
-- 
1.7.8.3

[PATCH 1/4] mdmon: Use getopt_long() to parse command line options

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This changes mdmon over to use getopt_long() for option parsing,
making it easier to add new options. In addition this patch introduces
a short version -t for --takeover and adds -h/--help.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdmon.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index b6ae0e6..a65c4a4 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -265,7 +265,14 @@ static int do_fork(void)
 
 void usage(void)
 {
-	fprintf(stderr, "Usage: mdmon [--all] [--takeover] CONTAINER\n");
+	fprintf(stderr,
+"Usage: mdmon [options] CONTAINER\n"
+"\n"
+"Options are:\n"
+"  --help        -h   : This message\n"
+"  --all              : All devices\n"
+"  --takeover    -t   : Takeover container\n"
+);
 	exit(2);
 }
 
@@ -277,25 +284,47 @@ int main(int argc, char *argv[])
 	int devnum;
 	char *devname;
 	int status = 0;
-	int arg;
+	int opt;
 	int all = 0;
 	int takeover = 0;
-
-	for (arg = 1; arg < argc; arg++) {
-		if (strncmp(argv[arg], "--all",5) == 0 ||
-		    strcmp(argv[arg], "/proc/mdstat") == 0) {
-			container_name = argv[arg];
+	static struct option options[] = {
+		{"all", 0, NULL, 'a'},
+		{"takeover", 0, NULL, 't'},
+		{"help", 0, NULL, 'h'},
+		{NULL, 0, NULL, 0}
+	};
+
+	while ((opt = getopt_long(argc, argv, "th", options, NULL)) != -1) {
+		switch (opt) {
+		case 'a':
+			container_name = argv[optind-1];
 			all = 1;
-		} else if (strcmp(argv[arg], "--takeover") == 0)
+			break;
+		case 't':
+			container_name = optarg;
 			takeover = 1;
-		else if (container_name == NULL)
-			container_name = argv[arg];
-		else
+			break;
+		case 'h':
+		default:
 			usage();
+			break;
+		}
 	}
+
+	if (all == 0 && container_name == NULL) {
+		if (argv[optind])
+			container_name = argv[optind];
+	}
+
 	if (container_name == NULL)
 		usage();
 
+	if (argc - optind > 1)
+		usage();
+
+	if (strcmp(container_name, "/proc/mdstat") == 0)
+		all = 1;
+
 	if (all) {
 		struct mdstat_ent *mdstat, *e;
 		int container_len = strlen(container_name);
-- 
1.7.8.3