[PATCH 0/4] Assorted mdadm patches.

[PATCH 0/4] Assorted mdadm patches.

[NeilBrown]

One real bugfix - that a customer might have hit.
Another real bugfix, which proves no-one is using that feature.
And a couple of minor consistency improvements.

Can you guess which is which :-?

NeilBrown


---

NeilBrown (4):
      Error messages should end with a newline character.
      Use correct syntax for passing DEVLINKS to mdadm from udev
      super1: only set clustered flag when bitmap is present
      Don't use exit(ERANGE)


 Build.c                     |    4 ++--
 Grow.c                      |    4 ++--
 Manage.c                    |    2 +-
 mdadm.8.in                  |    2 +-
 mdadm.c                     |    2 +-
 mdopen.c                    |    2 +-
 super1.c                    |    4 ++--
 udev-md-raid-assembly.rules |    2 +-
 8 files changed, 11 insertions(+), 11 deletions(-)

--
Signature

[PATCH 2/4] Use correct syntax for passing DEVLINKS to mdadm from udev

[NeilBrown]

 ${DEVLINKS}
is not valid udev syntax, and is passed through uninterpreted.
 $env{DEVLINKS}
or
  %e{DEVLINKS}
is correct.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 mdadm.8.in                  |    2 +-
 udev-md-raid-assembly.rules |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mdadm.8.in b/mdadm.8.in
index ecfe9da8d474..d8c2dfca6040 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2948,7 +2948,7 @@ This is the only context where the aliases are used.  They are
 usually provided by a
 .I udev
 rules mentioning
-.BR ${DEVLINKS} .
+.BR $env{DEVLINKS} .
 
 .IP +
 Does the device have a valid md superblock?  If a specific metadata
diff --git a/udev-md-raid-assembly.rules b/udev-md-raid-assembly.rules
index 8ca232a44b1f..9f055ed022d5 100644
--- a/udev-md-raid-assembly.rules
+++ b/udev-md-raid-assembly.rules
@@ -30,7 +30,7 @@ LABEL="md_inc"
 
 # remember you can limit what gets auto/incrementally assembled by
 # mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
-ACTION=="add|change", IMPORT{program}="BINDIR/mdadm --incremental --export $devnode --offroot ${DEVLINKS}"
+ACTION=="add|change", IMPORT{program}="BINDIR/mdadm --incremental --export $devnode --offroot $env{DEVLINKS}"
 ACTION=="add|change", ENV{MD_STARTED}=="*unsafe*", ENV{MD_FOREIGN}=="no", ENV{SYSTEMD_WANTS}+="mdadm-last-resort@$env{MD_DEVICE}.timer"
 ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="BINDIR/mdadm -If $name --path $env{ID_PATH}"
 ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="BINDIR/mdadm -If $name"

[PATCH 4/4] Don't use exit(ERANGE)

[NeilBrown]

mdadm uses smaller exit codes like 0,1,2,3,4.
Using ERANGE is inconsistent and not helpful.
So change it to a more consistent number.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 mdadm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mdadm.c b/mdadm.c
index 70b16f2231b7..d80aab361264 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -619,7 +619,7 @@ int main(int argc, char *argv[])
 			c.homecluster = optarg;
 			if (strlen(c.homecluster) > 64) {
 				pr_err("Cluster name too big.\n");
-				exit(ERANGE);
+				exit(2);
 			}
 			continue;
 		case O(CREATE,'x'): /* number of spare (eXtra) disks */

[PATCH 3/4] super1: only set clustered flag when bitmap is present

[NeilBrown]

If no bitmap is present, then the test

	if (__le32_to_cpu(bsb->nodes) > 1)

accesses uninitialised memory.  So move that test inside
a test for a bitmap being present.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 super1.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/super1.c b/super1.c
index 86ec850d03bd..47f90cd75805 100644
--- a/super1.c
+++ b/super1.c
@@ -977,14 +977,14 @@ static void getinfo_super1(struct supertype *st, struct mdinfo *info, char *map)
 	info->array.chunk_size = __le32_to_cpu(sb->chunksize)*512;
 	info->array.state =
 		(__le64_to_cpu(sb->resync_offset) == MaxSector)	? 1 : 0;
-	if (__le32_to_cpu(bsb->nodes) > 1)
-		info->array.state |= (1 << MD_SB_CLUSTERED);
 
 	super_offset = __le64_to_cpu(sb->super_offset);
 	info->data_offset = __le64_to_cpu(sb->data_offset);
 	info->component_size = __le64_to_cpu(sb->size);
 	if (sb->feature_map & __le32_to_cpu(MD_FEATURE_BITMAP_OFFSET)) {
 		info->bitmap_offset = (int32_t)__le32_to_cpu(sb->bitmap_offset);
+		if (__le32_to_cpu(bsb->nodes) > 1)
+			info->array.state |= (1 << MD_SB_CLUSTERED);
 	} else if (sb->feature_map & __le32_to_cpu(MD_FEATURE_PPL)) {
 		info->ppl_offset = __le16_to_cpu(sb->ppl.offset);
 		info->ppl_size = __le16_to_cpu(sb->ppl.size);

[PATCH 1/4] Error messages should end with a newline character.

[NeilBrown]

Add "\n" to the end of error messages which don't already
have one.  Also spell "opened" correctly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 Build.c  |    4 ++--
 Grow.c   |    4 ++--
 Manage.c |    2 +-
 mdopen.c |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Build.c b/Build.c
index 70ba06843796..962c2e3776b9 100644
--- a/Build.c
+++ b/Build.c
@@ -181,7 +181,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
 			int major = BITMAP_MAJOR_HI;
 #if 0
 			if (s->bitmap_chunk == UnSet) {
-				pr_err("%s cannot be openned.", s->bitmap_file);
+				pr_err("%s cannot be opened.\n", s->bitmap_file);
 				goto abort;
 			}
 #endif
@@ -193,7 +193,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
 			}
 			bitmap_fd = open(s->bitmap_file, O_RDWR);
 			if (bitmap_fd < 0) {
-				pr_err("%s cannot be openned.", s->bitmap_file);
+				pr_err("%s cannot be opened.\n", s->bitmap_file);
 				goto abort;
 			}
 		}
diff --git a/Grow.c b/Grow.c
index 4ecb1d8449ae..9305db6b6df4 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3183,7 +3183,7 @@ static int reshape_array(char *container, int fd, char *devname,
 
 		if (info2) {
 			if (sysfs_init(info2, fd, st->devnm)) {
-				pr_err("unable to initialize sysfs for %s",
+				pr_err("unable to initialize sysfs for %s\n",
 				       st->devnm);
 				free(info2);
 				goto release;
@@ -5127,7 +5127,7 @@ int Grow_continue_command(char *devname, int fd,
 		}
 
 		if (sysfs_init(content, fd2, mdstat->devnm)) {
-			pr_err("Unable to initialize sysfs for %s, Grow cannot continue",
+			pr_err("Unable to initialize sysfs for %s, Grow cannot continue.\n",
 			       mdstat->devnm);
 			ret_val = 1;
 			close(fd2);
diff --git a/Manage.c b/Manage.c
index 04b9398c2e4f..6b6112f7abc9 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1423,7 +1423,7 @@ int Manage_subdevs(char *devname, int fd,
 			}
 			add_devlist = conf_get_devs();
 			if (add_devlist == NULL) {
-				pr_err("no devices to scan for missing members.");
+				pr_err("no devices to scan for missing members.\n");
 				continue;
 			}
 			for (dp = &add_devlist; *dp; dp = & (*dp)->next)
diff --git a/mdopen.c b/mdopen.c
index c4f1c12c2dcd..3c0052f2db23 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -198,7 +198,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 			return -1;
 		}
 		if (cname[0] == 0) {
-			pr_err("%s is an invalid name for an md device (empty!).", dev);
+			pr_err("%s is an invalid name for an md device (empty!).\n", dev);
 			return -1;
 		}
 		if (num < 0) {

Re: [PATCH 0/4] Assorted mdadm patches.

[Jes Sorensen]

On 08/04/2017 01:30 AM, NeilBrown wrote:
> One real bugfix - that a customer might have hit.
> Another real bugfix, which proves no-one is using that feature.
> And a couple of minor consistency improvements.
> 
> Can you guess which is which :-?

Clearly patch 1 is the customer reported bug :)

I applied all of them. I am not wildly in love with patch 4, I kinda 
would like to switch to using standard error codes, but mixing them is 
inconsistent, so lets at least be consistent for now.

All applied!

Thanks,
Jes

Re: [PATCH 0/4] Assorted mdadm patches.

[NeilBrown]

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

On Wed, Aug 16 2017, Jes Sorensen wrote:

> On 08/04/2017 01:30 AM, NeilBrown wrote:
>> One real bugfix - that a customer might have hit.
>> Another real bugfix, which proves no-one is using that feature.
>> And a couple of minor consistency improvements.
>> 
>> Can you guess which is which :-?
>
> Clearly patch 1 is the customer reported bug :)
>
> I applied all of them. I am not wildly in love with patch 4, I kinda 
> would like to switch to using standard error codes, but mixing them is 
> inconsistent, so lets at least be consistent for now.

If we were to switch to standard error codes, we should probably aim to
use the EX_* codes from sysexits.h.  No errno values.

>
> All applied!

Thanks,
NeilBrown

>
> Thanks,
> Jes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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