[PATCH 0/6] Fixes for installation of CentOS 6 on DDF BIOS RAID

[PATCH 0/6] Fixes for installation of CentOS 6 on DDF BIOS RAID

[mwilck]

I am currently working on RHEL6/CentOS6 installation on a DDF 
BIOS RAID using MD. Currently, RHEL/CentOS will activate dmraid when 
the installer encouters DDF meta data, as most distributions do. 
I got the installation working with MD using an update image, and I
will soon publish a recipe for that.
I hope to do the same for other distributions in the future.

In the process I found some glitches that need the following
series of small patches for mdadm.

Martin Wilck (6):
  DDF: ddf_activate_spare: fix gcc -O2 uninitialized warning
  DDF: export_examine_super_ddf: print MD_DEVICES
  DDF: container_content_ddf: set safe_mode_delay > 0
  DDF: increase default value for safe_mode_delay to 4000ms
  in_initrd: fix gcc compiler error
  mdmon: allow disabling "@dmon" command name at compile time

 Makefile    |    4 +++-
 mdmon.c     |    2 ++
 super-ddf.c |   11 +++++++++--
 util.c      |    4 ++--
 4 files changed, 16 insertions(+), 5 deletions(-)

-- 
1.7.3.4

[PATCH 1/6] DDF: ddf_activate_spare: fix gcc -O2 uninitialized warning

[mwilck]

The warning is spurious, but will break compilation with -O2.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index e3b3dca..95972ad 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4779,7 +4779,7 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
 	struct ddf_super *ddf = a->container->sb;
 	int global_ok = 0;
 	struct mdinfo *rv = NULL;
-	struct mdinfo *di;
+	struct mdinfo *di = NULL;
 	struct metadata_update *mu;
 	struct dl *dl;
 	int i;
-- 
1.7.3.4

Re: [PATCH 1/6] DDF: ddf_activate_spare: fix gcc -O2 uninitialized warning

[NeilBrown]

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

On Fri, 16 Aug 2013 20:21:55 +0200 mwilck@arcor.de wrote:

> The warning is spurious, but will break compilation with -O2.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  super-ddf.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/super-ddf.c b/super-ddf.c
> index e3b3dca..95972ad 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -4779,7 +4779,7 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
>  	struct ddf_super *ddf = a->container->sb;
>  	int global_ok = 0;
>  	struct mdinfo *rv = NULL;
> -	struct mdinfo *di;
> +	struct mdinfo *di = NULL;
>  	struct metadata_update *mu;
>  	struct dl *dl;
>  	int i;

I fixed this differently as below.
Thanks,
NeilBrown

From bb925ff08beda785d42f5ff8a0f082d91c5caad4 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Fri, 16 Aug 2013 20:21:55 +0200
Subject: [PATCH] DDF: ddf_activate_spare: fix gcc -O2 uninitialized warning

At this point 'di' and 'rv' both have the same value.  gcc doesn't
realise that and a human reader might not either.
'rv' makes more sense too, so use that.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/super-ddf.c b/super-ddf.c
index e3b3dca..ee6499a 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -4959,7 +4959,7 @@ static struct mdinfo *ddf_activate_spare(struct active_array *a,
 	 * Create a metadata_update record to update the
 	 * phys_refnum and lba_offset values
 	 */
-	vc = find_vdcr(ddf, a->info.container_member, di->disk.raid_disk,
+	vc = find_vdcr(ddf, a->info.container_member, rv->disk.raid_disk,
 		       &n_bvd, &vcl);
 	if (vc == NULL)
 		return NULL;

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

[PATCH 2/6] DDF: export_examine_super_ddf: print MD_DEVICES

[mwilck]

Have mdadm -E --export print the number of RAID devices,
like other meta data formats do. Anaconda (RHEL/CentOS installer)
depends on it.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 95972ad..013bb9d 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1593,6 +1593,8 @@ static void export_examine_super_ddf(struct supertype *st)
 	printf("MD_METADATA=ddf\n");
 	printf("MD_LEVEL=container\n");
 	printf("MD_UUID=%s\n", nbuf+5);
+	printf("MD_DEVICES=%u\n",
+		be16_to_cpu(((struct ddf_super *)st->sb)->phys->used_pdes));
 }
 
 static int copy_metadata_ddf(struct supertype *st, int from, int to)
-- 
1.7.3.4

[PATCH 3/6] DDF: container_content_ddf: set safe_mode_delay > 0

[mwilck]

Set safe_mode_delay to something >0, otherwise all container subarrays
assembled will have safe_mode_delay=0. That will break the assumption that
meta data becomes clean after running mdadm --wait-clean.

Use the same value as in getinfo_super_ddf_bvd. It would be cleaner
to call that directly from container_content_ddf, but I need to check
possible side effects first.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 013bb9d..d8fa654 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3668,6 +3668,7 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 		this->array.md_minor      = -1;
 		this->array.major_version = -1;
 		this->array.minor_version = -2;
+		this->safe_mode_delay = 200;
 		cptr = (__u32 *)(vc->conf.guid + 16);
 		this->array.ctime         = DECADE + __be32_to_cpu(*cptr);
 		this->array.utime	  = DECADE +
-- 
1.7.3.4

[PATCH 4/6] DDF: increase default value for safe_mode_delay to 4000ms

[mwilck]

That is the same value that IMSM uses. The current default of 200ms
seems to have been copied from the native MD meta data. That value
appears to be much too low for DDF, given that writing the DDF meta
data means that easily several MB worth of data need to be written to
disk.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 super-ddf.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index d8fa654..f03dd5b 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -47,6 +47,10 @@ unsigned long crc32(
 #define DDF_NOTFOUND (~0U)
 #define DDF_CONTAINER (DDF_NOTFOUND-1)
 
+/* Default for safe_mode_delay. Same value as for IMSM.
+ */
+static const int DDF_SAFE_MODE_DELAY = 4000;
+
 /* The DDF metadata handling.
  * DDF metadata lives at the end of the device.
  * The last 512 byte block provides an 'anchor' which is used to locate
@@ -1976,7 +1980,7 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	sprintf(info->text_version, "/%s/%d",
 		st->container_devnm,
 		info->container_member);
-	info->safe_mode_delay = 200;
+	info->safe_mode_delay = DDF_SAFE_MODE_DELAY;
 
 	memcpy(info->name, ddf->virt->entries[info->container_member].name, 16);
 	info->name[16]=0;
@@ -3668,7 +3672,7 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
 		this->array.md_minor      = -1;
 		this->array.major_version = -1;
 		this->array.minor_version = -2;
-		this->safe_mode_delay = 200;
+		this->safe_mode_delay = DDF_SAFE_MODE_DELAY;
 		cptr = (__u32 *)(vc->conf.guid + 16);
 		this->array.ctime         = DECADE + __be32_to_cpu(*cptr);
 		this->array.utime	  = DECADE +
-- 
1.7.3.4

[PATCH 5/6] in_initrd: fix gcc compiler error

[mwilck]

On some systems, this code caused a "comparison between signed
and unsigned" error.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 util.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/util.c b/util.c
index 8096e29..491a107 100644
--- a/util.c
+++ b/util.c
@@ -1949,6 +1949,6 @@ int in_initrd(void)
 	/* This is based on similar function in systemd. */
 	struct statfs s;
 	return  statfs("/", &s) >= 0 &&
-		(s.f_type == TMPFS_MAGIC ||
-		 s.f_type == RAMFS_MAGIC);
+		((unsigned long)s.f_type == TMPFS_MAGIC ||
+		 (unsigned long)s.f_type == RAMFS_MAGIC);
 }
-- 
1.7.3.4

Re: [PATCH 5/6] in_initrd: fix gcc compiler error

[NeilBrown]

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

On Fri, 16 Aug 2013 20:21:59 +0200 mwilck@arcor.de wrote:

> On some systems, this code caused a "comparison between signed
> and unsigned" error.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  util.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/util.c b/util.c
> index 8096e29..491a107 100644
> --- a/util.c
> +++ b/util.c
> @@ -1949,6 +1949,6 @@ int in_initrd(void)
>  	/* This is based on similar function in systemd. */
>  	struct statfs s;
>  	return  statfs("/", &s) >= 0 &&
> -		(s.f_type == TMPFS_MAGIC ||
> -		 s.f_type == RAMFS_MAGIC);
> +		((unsigned long)s.f_type == TMPFS_MAGIC ||
> +		 (unsigned long)s.f_type == RAMFS_MAGIC);
>  }

That is rather horrible.  It seems that some architectures like this field to
be signed and other unsigned.
But aren't TMPFS_MAGIC and  RAMFS_MAGIC simple constants and so signed by
default?
What are that defined to on the system that produces the error?

Maybe we need to cast both LHS and RHS to "unsigned long"....

NeilBrown

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

[PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[mwilck]

mdmon sets its command name to "@dmon" on distributions using systemd
to tell systemd not to kill it during shutdown.

Some older tools (e.g. the anaconda loader on RHEL6 during installation)
look specifically for "mdmon" and won't work with "@dmon". They can't be
fixed any more. So, this patch introduces the macro SYSTEMD to disable
changing the command name. Default is to assume that mdmon is started
by systemd.

Signed-off-by: Martin Wilck <mwilck@arcor.de>
---
 Makefile |    4 +++-
 mdmon.c  |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index e8da3a5..5970b90 100644
--- a/Makefile
+++ b/Makefile
@@ -77,11 +77,13 @@ MAP_PATH = $(MAP_DIR)/$(MAP_FILE)
 MDMON_DIR = $(RUN_DIR)
 # place for autoreplace cookies
 FAILED_SLOTS_DIR = $(RUN_DIR)/failed-slots
+# set SYSTEMD to 1 for distributions where mdmon is started by systemd
+SYSTEMD = 1
 SYSTEMD_DIR=/lib/systemd/system
 DIRFLAGS = -DMAP_DIR=\"$(MAP_DIR)\" -DMAP_FILE=\"$(MAP_FILE)\"
 DIRFLAGS += -DMDMON_DIR=\"$(MDMON_DIR)\"
 DIRFLAGS += -DFAILED_SLOTS_DIR=\"$(FAILED_SLOTS_DIR)\"
-CFLAGS = $(CWFLAGS) $(CXFLAGS) -DSendmail=\""$(MAILCMD)"\" $(CONFFILEFLAGS) $(DIRFLAGS)
+CFLAGS = $(CWFLAGS) $(CXFLAGS) -DSendmail=\""$(MAILCMD)"\" $(CONFFILEFLAGS) $(DIRFLAGS) -DSYSTEMD=$(SYSTEMD)
 
 VERSION = $(shell [ -d .git ] && git describe HEAD | sed 's/mdadm-//')
 VERS_DATE = $(shell [ -d .git ] && date --date="`git log -n1 --format=format:%cd --date=short`" '+%0dth %B %Y' | sed -e 's/1th/1st/' -e 's/2th/2nd/' -e 's/11st/11th/' -e 's/12nd/12th/')
diff --git a/mdmon.c b/mdmon.c
index f0b0623..132b469 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -298,6 +298,7 @@ int main(int argc, char *argv[])
 		{NULL, 0, NULL, 0}
 	};
 
+#if (SYSTEMD == 1)
 	if (in_initrd()) {
 		/*
 		 * set first char of argv[0] to @. This is used by
@@ -306,6 +307,7 @@ int main(int argc, char *argv[])
 		 */
 		argv[0][0] = '@';
 	}
+#endif
 
 	while ((opt = getopt_long(argc, argv, "thaF", options, NULL)) != -1) {
 		switch (opt) {
-- 
1.7.3.4

Re: [PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[NeilBrown]

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

On Fri, 16 Aug 2013 20:22:00 +0200 mwilck@arcor.de wrote:

> mdmon sets its command name to "@dmon" on distributions using systemd
> to tell systemd not to kill it during shutdown.
> 
> Some older tools (e.g. the anaconda loader on RHEL6 during installation)
> look specifically for "mdmon" and won't work with "@dmon". They can't be
> fixed any more. So, this patch introduces the macro SYSTEMD to disable
> changing the command name. Default is to assume that mdmon is started
> by systemd.
> 
> Signed-off-by: Martin Wilck <mwilck@arcor.de>
> ---
>  Makefile |    4 +++-
>  mdmon.c  |    2 ++
>  2 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e8da3a5..5970b90 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -77,11 +77,13 @@ MAP_PATH = $(MAP_DIR)/$(MAP_FILE)
>  MDMON_DIR = $(RUN_DIR)
>  # place for autoreplace cookies
>  FAILED_SLOTS_DIR = $(RUN_DIR)/failed-slots
> +# set SYSTEMD to 1 for distributions where mdmon is started by systemd
> +SYSTEMD = 1
>  SYSTEMD_DIR=/lib/systemd/system
>  DIRFLAGS = -DMAP_DIR=\"$(MAP_DIR)\" -DMAP_FILE=\"$(MAP_FILE)\"
>  DIRFLAGS += -DMDMON_DIR=\"$(MDMON_DIR)\"
>  DIRFLAGS += -DFAILED_SLOTS_DIR=\"$(FAILED_SLOTS_DIR)\"
> -CFLAGS = $(CWFLAGS) $(CXFLAGS) -DSendmail=\""$(MAILCMD)"\" $(CONFFILEFLAGS) $(DIRFLAGS)
> +CFLAGS = $(CWFLAGS) $(CXFLAGS) -DSendmail=\""$(MAILCMD)"\" $(CONFFILEFLAGS) $(DIRFLAGS) -DSYSTEMD=$(SYSTEMD)
>  
>  VERSION = $(shell [ -d .git ] && git describe HEAD | sed 's/mdadm-//')
>  VERS_DATE = $(shell [ -d .git ] && date --date="`git log -n1 --format=format:%cd --date=short`" '+%0dth %B %Y' | sed -e 's/1th/1st/' -e 's/2th/2nd/' -e 's/11st/11th/' -e 's/12nd/12th/')
> diff --git a/mdmon.c b/mdmon.c
> index f0b0623..132b469 100644
> --- a/mdmon.c
> +++ b/mdmon.c
> @@ -298,6 +298,7 @@ int main(int argc, char *argv[])
>  		{NULL, 0, NULL, 0}
>  	};
>  
> +#if (SYSTEMD == 1)
>  	if (in_initrd()) {
>  		/*
>  		 * set first char of argv[0] to @. This is used by
> @@ -306,6 +307,7 @@ int main(int argc, char *argv[])
>  		 */
>  		argv[0][0] = '@';
>  	}
> +#endif
>  
>  	while ((opt = getopt_long(argc, argv, "thaF", options, NULL)) != -1) {
>  		switch (opt) {

Can you help me understand?  What is anaconda even looking for mdmon?
Does it need exactly "mdmon" or would "@mdmon" be OK?

Does "started by systemd" mean that "systemctl start mdmon@mdXXX.service" was
used to start it?  In that case can't we put something in
systemd/mdmon@.service to make the right thing happen?

I'm going to hold off on this one for the moment.

All others applied.

Thanks,
NeilBrown

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

Re: [PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[Martin Wilck]

On 08/28/2013 07:15 AM, NeilBrown wrote:

> Can you help me understand?  What is anaconda even looking for mdmon?
> Does it need exactly "mdmon" or would "@mdmon" be OK?

To be precise, it's not anaconda but "loader", the C program that starts
anaconda and cleans up after it. It looks exactly for "mdmon". See
https://git.fedorahosted.org/cgit/anaconda.git/tree/loader/shutdown.c?h=rhel6-branch#n59

> Does "started by systemd" mean that "systemctl start mdmon@mdXXX.service" was
> used to start it?  In that case can't we put something in
> systemd/mdmon@.service to make the right thing happen?

Sorry for having explained unclearly. This is *not* a systemd
environment. There is no regular "init" process running AFAICT, "loader"
takes that role as far as necessary in the installation environment.

> 
> I'm going to hold off on this one for the moment.

If this isn't applied, in the RHEL/CentOS 6 installation environment,
"loader" will kill mdmon, and a subsequent "mdadm --wait-clean" will
hang forever.

Martin

> 
> All others applied.
> 
> Thanks,
> NeilBrown

Re: [PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[NeilBrown]

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

On Sun, 01 Sep 2013 19:20:13 +0200 Martin Wilck <mwilck@arcor.de> wrote:

> On 08/28/2013 07:15 AM, NeilBrown wrote:
> 
> > Can you help me understand?  What is anaconda even looking for mdmon?
> > Does it need exactly "mdmon" or would "@mdmon" be OK?
> 
> To be precise, it's not anaconda but "loader", the C program that starts
> anaconda and cleans up after it. It looks exactly for "mdmon". See
> https://git.fedorahosted.org/cgit/anaconda.git/tree/loader/shutdown.c?h=rhel6-branch#n59
> 
> > Does "started by systemd" mean that "systemctl start mdmon@mdXXX.service" was
> > used to start it?  In that case can't we put something in
> > systemd/mdmon@.service to make the right thing happen?
> 
> Sorry for having explained unclearly. This is *not* a systemd
> environment. There is no regular "init" process running AFAICT, "loader"
> takes that role as far as necessary in the installation environment.
> 
> > 
> > I'm going to hold off on this one for the moment.
> 
> If this isn't applied, in the RHEL/CentOS 6 installation environment,
> "loader" will kill mdmon, and a subsequent "mdadm --wait-clean" will
> hang forever.
> 

Thanks for the details Martin.

I've taken a different approach which should solve the same problem.

If you can confirm that this still works I would appreciate it.

Thanks,
NeilBrown


From 2f1bcf43d93a991845b2ebee76850bd4dc5bc7fc Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Mon, 2 Sep 2013 11:02:09 +1000
Subject: [PATCH] Make sure "mdmon" doesn't get called "@dmon".

The Anaconda installer (via its "loader" program) will try to kill
many processes at shutdown, but not "mdmon".

However when mdadm runs mdmon in the Anaconda environment, mdmon
sets argv[0][0] to '@' resulting in "@dmon" which confuses
"loader".

So change mdadm to set argv[0] to a path so that mdmon becomes e.g.
  "@usr/sbin/mdmon"
which "loader" will recognise as being "mdmon".

Reported-by: Martin Wilck <mwilck@arcor.de>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/util.c b/util.c
index 491a107..c12fb21 100644
--- a/util.c
+++ b/util.c
@@ -1691,7 +1691,7 @@ int start_mdmon(char *devnm)
 	char *paths[4] = {
 		pathbuf,
 		"/sbin/mdmon",
-		"mdmon",
+		"./mdmon",
 		NULL
 	};
 
@@ -1759,7 +1759,7 @@ int start_mdmon(char *devnm)
 
 		for (i = 0; paths[i]; i++)
 			if (paths[i][0]) {
-				execl(paths[i], "mdmon",
+				execl(paths[i], paths[i],
 				      devnm, NULL);
 			}
 		exit(1);

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

Re: [PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[Martin Wilck]

On 09/02/2013 03:07 AM, NeilBrown wrote:
> Thanks for the details Martin.
> 
> I've taken a different approach which should solve the same problem.
> 
> If you can confirm that this still works I would appreciate it.

Confirmed. Thanks a lot. CentOS6 installation works with upstream mdadm
(git 4e5e54c).

Martin

Re: [PATCH 6/6] mdmon: allow disabling "@dmon" command name at compile time

[NeilBrown]

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

On Mon, 02 Sep 2013 20:47:07 +0200 Martin Wilck <mwilck@arcor.de> wrote:

> On 09/02/2013 03:07 AM, NeilBrown wrote:
> > Thanks for the details Martin.
> > 
> > I've taken a different approach which should solve the same problem.
> > 
> > If you can confirm that this still works I would appreciate it.
> 
> Confirmed. Thanks a lot. CentOS6 installation works with upstream mdadm
> (git 4e5e54c).
> 

Excellent - thanks.

NeilBrown

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