[PATCH v2] mdadm: make --update=homehost work again

[PATCH v2] mdadm: make --update=homehost work again

[Justin Maggard]

Commit 1e2b276535cea41c348292a019bdda8a58cb1679 (Report error in --update
string is not recognised) broke homehost updating functionality because it
depended on each string comparison being done even after we already found
a match.  Make it work again by using a goto instead.

---
 super0.c |    3 ++-
 super1.c |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/super0.c b/super0.c
index ecb6b38..f15f1e0 100644
--- a/super0.c
+++ b/super0.c
@@ -554,10 +554,11 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
 	} else if (strcmp(update, "homehost") == 0 &&
 		   homehost) {
 		uuid_set = 0;
-		update = "uuid";
 		info->uuid[0] = sb->set_uuid0;
 		info->uuid[1] = sb->set_uuid1;
+		goto update_uuid;
 	} else if (strcmp(update, "uuid") == 0) {
+ update_uuid:
 		if (!uuid_set && homehost) {
 			char buf[20];
 			char *hash = sha1_buffer(homehost,
diff --git a/super1.c b/super1.c
index 5bb1f01..fc31948 100644
--- a/super1.c
+++ b/super1.c
@@ -987,14 +987,15 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 	} else if (strcmp(update, "homehost") == 0 &&
 		   homehost) {
 		char *c;
-		update = "name";
 		c = strchr(sb->set_name, ':');
 		if (c)
 			strncpy(info->name, c+1, 31 - (c-sb->set_name));
 		else
 			strncpy(info->name, sb->set_name, 32);
 		info->name[32] = 0;
+		goto update_name;
 	} else if (strcmp(update, "name") == 0) {
+ update_name:
 		if (info->name[0] == 0)
 			sprintf(info->name, "%d", info->array.md_minor);
 		memset(sb->set_name, 0, sizeof(sb->set_name));
-- 
1.7.9.5

Re: [PATCH v2] mdadm: make --update=homehost work again

[NeilBrown]

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

On Wed,  6 Feb 2013 12:07:17 -0800 Justin Maggard <jmaggard10@gmail.com>
wrote:

> Commit 1e2b276535cea41c348292a019bdda8a58cb1679 (Report error in --update
> string is not recognised) broke homehost updating functionality because it
> depended on each string comparison being done even after we already found
> a match.  Make it work again by using a goto instead.
> 

Hi Justin,
thanks for the patch.  Unfortunately I really don't like gotos when they can
be easily avoided (though they do have their place sometimes).
So I restructured the code a bit instead - as below.

Thanks,
NeilBrown

From 1fc34af916a7afd0b6a6b386973a6045579ee31a Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Thu, 7 Feb 2013 11:51:21 +1100
Subject: [PATCH] make --update=homehost work again

Commit 1e2b276535cea41c348292a019bdda8a58cb1679 (Report error in --update
string is not recognised) broke homehost updating functionality because it
depended on each string comparison being done even after we already found
a match.  Make it work again by restructuring code.

Reported-by: (and original version by) Justin Maggard <jmaggard10@gmail.com>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/super0.c b/super0.c
index 32a9f7b..1f4dc75 100644
--- a/super0.c
+++ b/super0.c
@@ -435,6 +435,18 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
 	int rv = 0;
 	int uuid[4];
 	mdp_super_t *sb = st->sb;
+
+	if (strcmp(update, "homehost") == 0 &&
+	    homehost) {
+		/* note that 'homehost' is special as it is really
+		 * a "uuid" update.
+		 */
+		uuid_set = 0;
+		update = "uuid";
+		info->uuid[0] = sb->set_uuid0;
+		info->uuid[1] = sb->set_uuid1;
+	}
+
 	if (strcmp(update, "sparc2.2")==0 ) {
 		/* 2.2 sparc put the events in the wrong place
 		 * So we copy the tail of the superblock
@@ -551,12 +563,6 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
 		/* make sure resync happens */
 		sb->state &= ~(1<<MD_SB_CLEAN);
 		sb->recovery_cp = 0;
-	} else if (strcmp(update, "homehost") == 0 &&
-		   homehost) {
-		uuid_set = 0;
-		update = "uuid";
-		info->uuid[0] = sb->set_uuid0;
-		info->uuid[1] = sb->set_uuid1;
 	} else if (strcmp(update, "uuid") == 0) {
 		if (!uuid_set && homehost) {
 			char buf[20];
diff --git a/super1.c b/super1.c
index c240b91..d0f1d5f 100644
--- a/super1.c
+++ b/super1.c
@@ -885,6 +885,21 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 	int rv = 0;
 	struct mdp_superblock_1 *sb = st->sb;
 
+	if (strcmp(update, "homehost") == 0 &&
+	    homehost) {
+		/* Note that 'homehost' is special as it is really
+		 * a "name" update.
+		 */
+		char *c;
+		update = "name";
+		c = strchr(sb->set_name, ':');
+		if (c)
+			strncpy(info->name, c+1, 31 - (c-sb->set_name));
+		else
+			strncpy(info->name, sb->set_name, 32);
+		info->name[32] = 0;
+	}
+
 	if (strcmp(update, "force-one")==0) {
 		/* Not enough devices for a working array,
 		 * so bring this one up-to-date
@@ -1037,16 +1052,6 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 			sb->bblog_shift = 0;
 			sb->bblog_offset = 0;
 		}
-	} else if (strcmp(update, "homehost") == 0 &&
-		   homehost) {
-		char *c;
-		update = "name";
-		c = strchr(sb->set_name, ':');
-		if (c)
-			strncpy(info->name, c+1, 31 - (c-sb->set_name));
-		else
-			strncpy(info->name, sb->set_name, 32);
-		info->name[32] = 0;
 	} else if (strcmp(update, "name") == 0) {
 		if (info->name[0] == 0)
 			sprintf(info->name, "%d", info->array.md_minor);

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

Re: [PATCH v2] mdadm: make --update=homehost work again

[Justin Maggard]

On Wed, Feb 6, 2013 at 4:53 PM, NeilBrown <neilb@suse.de> wrote:
> On Wed,  6 Feb 2013 12:07:17 -0800 Justin Maggard <jmaggard10@gmail.com>
> wrote:
>
>> Commit 1e2b276535cea41c348292a019bdda8a58cb1679 (Report error in --update
>> string is not recognised) broke homehost updating functionality because it
>> depended on each string comparison being done even after we already found
>> a match.  Make it work again by using a goto instead.
>>
>
> Hi Justin,
> thanks for the patch.  Unfortunately I really don't like gotos when they can
> be easily avoided (though they do have their place sometimes).
> So I restructured the code a bit instead - as below.
>

Works for me.  Thanks!

-Justin

Re: Raid 6, 9 1.5T days drives, 2 "fail" one after the other

[NeilBrown]

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

On Thu, 07 Feb 2013 10:39:53 -0500 Dragos Dobrescu <dragos@mpigani.org> wrote:

> Hi,
> I need some help.
> I noticed the server was in recovery mode. It had just dropped a "faulty" drive. I checked the drive and it looked like it was working. When the recovery was done I added the drive back, after recreating the partition. 
> 
> As soon as I did it, mdadm informed me that it redropped it and dropped another drive at the same time. I removed both driver and added a brand new drive (second is on the way) which the system accepted and started recovery. 
> 
> What I don't understand is that I plugged the drives on another computer with sata-USB adapter and performed a full smart checkup which returned successful, minus a few bad sectors and some warnings of pas over heating.
>  What is going on? 
> Thank you for your help.
> 
> Dragos
> 

No one replied?  I felt sure someone else would.

Maybe you  have a problem with your driver card, or with a cable, or
something.

Normally if md stops a driver there will be messages in the kernel log about
access failures.  Do you still have all the logs from when this  happened?
Are there any messages from the kernel?

NeilBrown


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

Re: Raid 6, 9 1.5T days drives, 2 "fail" one after the other

[Phil Turmel]

Hi Neil, Dragos,

On 02/18/2013 07:39 PM, NeilBrown wrote:
> On Thu, 07 Feb 2013 10:39:53 -0500 Dragos Dobrescu
> <dragos@mpigani.org> wrote:
> 
>> Hi, I need some help. I noticed the server was in recovery mode. It
>> had just dropped a "faulty" drive. I checked the drive and it
>> looked like it was working. When the recovery was done I added the
>> drive back, after recreating the partition.
>> 
>> As soon as I did it, mdadm informed me that it redropped it and
>> dropped another drive at the same time. I removed both driver and
>> added a brand new drive (second is on the way) which the system
>> accepted and started recovery.
>> 
>> What I don't understand is that I plugged the drives on another
>> computer with sata-USB adapter and performed a full smart checkup
>> which returned successful, minus a few bad sectors and some
>> warnings of pas over heating. What is going on? Thank you for your
>> help.
>> 
>> Dragos
>> 
> 
> No one replied?  I felt sure someone else would.

No one else saw it.  The message you quoted is not in the archives, and
I never got one directly.

> Maybe you  have a problem with your driver card, or with a cable, or 
> something.

Or normal UREs.  Dragos, please also share your smartctl reports (with -x).

> Normally if md stops a driver there will be messages in the kernel
> log about access failures.  Do you still have all the logs from when
> this  happened? Are there any messages from the kernel?
> 
> NeilBrown

Phil