[PATCH 0/2] add support for launching mdmon via systemd

[PATCH 0/2] add support for launching mdmon via systemd

[Jes.Sorensen]

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

Hi,

It's been an ongoing issue for a while trying to figure out how to
handle launching mdmon from the initrd in Fedora and avoid it getting
killed by systemd due to it ending up in the wrong cgroup (udev).

We tried simply attaching mdmon to the cgroup of systemd but it didn't
really work, and in the end it seems to be more in line with the
systemd mentality to have systemd spawn it. With these patches and a
few pending changes to dracut, I am finally able to boot and reboot
systems with 1 or 2 IMSM arrays.

The patch for systemctl is implemented as a compile time option rather
than a command line option, since I think it is something that the
distro will decide. However we can make it a command line option if
preferred.

I also have matching systemd .service files for this, which I am happy
to contribute, but I am not sure if they are useful to other distros
or not? We haven't had a history of including .service files with
mdadm.

Cheers,
Jes


Jes Sorensen (2):
  Add support for launching mdmon via systemctl instead of fork/exec
  execl() only returns in case of error

 Makefile |  6 ++++++
 mdmon.c  |  4 +++-
 util.c   | 15 ++++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)

-- 
1.7.11.7

[PATCH 1/2] Add support for launching mdmon via systemctl instead of fork/exec

[Jes.Sorensen]

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

This allows for having mdmon launched via systemctl which avoids
problems with it getting killed by systemd due to it ending up in the
parent's cgroup (udev).

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Makefile |  6 ++++++
 mdmon.c  |  4 +++-
 util.c   | 14 ++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b9787d6..b786aa9 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,12 @@ else
  DEFAULT_METADATA=1.2
 endif
 
+# Use systemctl to launch mdmon
+#MDMON_SYSTEMCTL=yes
+ifdef MDMON_SYSTEMCTL
+ CPPFLAGS += -DMDMON_SYSTEMCTL=1
+endif
+
 PKG_CONFIG ?= pkg-config
 
 SYSCONFDIR = /etc
diff --git a/mdmon.c b/mdmon.c
index 5d5ae94..7459ae2 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -188,7 +188,9 @@ static void try_kill_monitor(pid_t pid, char *devname, int sock)
 	 * might be "@dmon"
 	 */
 	if (n < 0 || !(strstr(buf, "mdmon") ||
-		       strstr(buf, "@dmon")))
+		       strstr(buf, "@dmon") ||
+		       strstr(buf, "/usr/sbin/mdmon") ||
+		       strstr(buf, "@usr/sbin/mdmon")))
 		return;
 
 	kill(pid, SIGTERM);
diff --git a/util.c b/util.c
index 6c10365..6b28f4e 100644
--- a/util.c
+++ b/util.c
@@ -1641,12 +1641,14 @@ int start_mdmon(int devnum)
 	pid_t pid;
 	int status;
 	char pathbuf[1024];
+#ifndef MDMON_SYSTEMCTL
 	char *paths[4] = {
 		pathbuf,
 		"/sbin/mdmon",
 		"mdmon",
 		NULL
 	};
+#endif
 
 	if (check_env("MDADM_NO_MDMON"))
 		return 0;
@@ -1674,6 +1676,17 @@ int start_mdmon(int devnum)
 			else
 				skipped = 0;
 
+#ifdef MDMON_SYSTEMCTL
+		if (__offroot) {
+			snprintf(pathbuf, 40, "mdmon-offroot@%s.service",
+				 devnum2devname(devnum));
+			execl("/usr/bin/systemctl", "systemctl", "start", pathbuf, NULL);
+		} else {
+			snprintf(pathbuf, 30, "mdmon@%s.service",
+				 devnum2devname(devnum));
+			execl("/usr/bin/systemctl", "systemctl", "start", pathbuf, NULL);
+		}
+#else
 		for (i = 0; paths[i]; i++)
 			if (paths[i][0]) {
 				if (__offroot) {
@@ -1686,6 +1699,7 @@ int start_mdmon(int devnum)
 					      NULL);
 				}
 			}
+#endif
 		exit(1);
 	case -1: pr_err("cannot run mdmon. "
 			 "Array remains readonly\n");
-- 
1.7.11.7

[PATCH 2/2] execl() only returns in case of error

[Jes.Sorensen]

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

In case exec fails, fall through and print warning rather than just
calling exit(1) silently.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 util.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util.c b/util.c
index 6b28f4e..6f7468c 100644
--- a/util.c
+++ b/util.c
@@ -1700,7 +1700,6 @@ int start_mdmon(int devnum)
 				}
 			}
 #endif
-		exit(1);
 	case -1: pr_err("cannot run mdmon. "
 			 "Array remains readonly\n");
 		return -1;
-- 
1.7.11.7

Re: [PATCH 0/2] add support for launching mdmon via systemd

[NeilBrown]

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

On Fri, 18 Jan 2013 09:43:10 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> It's been an ongoing issue for a while trying to figure out how to
> handle launching mdmon from the initrd in Fedora and avoid it getting
> killed by systemd due to it ending up in the wrong cgroup (udev).
> 
> We tried simply attaching mdmon to the cgroup of systemd but it didn't
> really work, and in the end it seems to be more in line with the
> systemd mentality to have systemd spawn it. With these patches and a
> few pending changes to dracut, I am finally able to boot and reboot
> systems with 1 or 2 IMSM arrays.
> 
> The patch for systemctl is implemented as a compile time option rather
> than a command line option, since I think it is something that the
> distro will decide. However we can make it a command line option if
> preferred.
> 
> I also have matching systemd .service files for this, which I am happy
> to contribute, but I am not sure if they are useful to other distros
> or not? We haven't had a history of including .service files with
> mdadm.

Thanks.
I'll have a proper look next week, however:
 - I would rather it was a run-time option, not compile time.
   I believe in giving choice to the admin.
 - I would like to include the .service files.  I don't know if we can get
   all distros to use the same one, but we certainly won't if we don't try.

NeilBrown

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

Re: [PATCH 0/2] add support for launching mdmon via systemd

[Jes Sorensen]

NeilBrown <neilb@suse.de> writes:
> On Fri, 18 Jan 2013 09:43:10 +0100 Jes.Sorensen@redhat.com wrote:
>
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> Hi,
>> 
>> It's been an ongoing issue for a while trying to figure out how to
>> handle launching mdmon from the initrd in Fedora and avoid it getting
>> killed by systemd due to it ending up in the wrong cgroup (udev).
>> 
>> We tried simply attaching mdmon to the cgroup of systemd but it didn't
>> really work, and in the end it seems to be more in line with the
>> systemd mentality to have systemd spawn it. With these patches and a
>> few pending changes to dracut, I am finally able to boot and reboot
>> systems with 1 or 2 IMSM arrays.
>> 
>> The patch for systemctl is implemented as a compile time option rather
>> than a command line option, since I think it is something that the
>> distro will decide. However we can make it a command line option if
>> preferred.
>> 
>> I also have matching systemd .service files for this, which I am happy
>> to contribute, but I am not sure if they are useful to other distros
>> or not? We haven't had a history of including .service files with
>> mdadm.
>
> Thanks.
> I'll have a proper look next week, however:
>  - I would rather it was a run-time option, not compile time.
>    I believe in giving choice to the admin.
>  - I would like to include the .service files.  I don't know if we can get
>    all distros to use the same one, but we certainly won't if we don't try.

Ok, I'll adapt it as a runtime option and also include the .service
files with the patch.

Cheers,
Jes