[PATCH 0/2] Fixes metadata while migrating from Raid 0 to Raid 10

[PATCH 0/2] Fixes metadata while migrating from Raid 0 to Raid 10

[Lukasz Orlowski]

The following series fixes metadata updates handling code after migration from Raid 0 to Raid 10:
Last disk is left as a spare instead of being activated and a missing disk is a
member of an array instead of being removed. The array status is degraded
instead of normal.

---

Lukasz Orlowski (1):
      imsm: Moves metadata update code for spare activation to separate function

Lukasz Orlowski (1):
      imsm: fix: Fixes metadata after migration from Raid 0 to Raid 10


 super-intel.c |  207 +++++++++++++++++++++++++++++++--------------------------
 1 files changed, 114 insertions(+), 93 deletions(-)

-- 
Signature
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 1/2] imsm: Moves metadata update code for spare activation to separate function

[Lukasz Orlowski]

The metadata update code during spare activation is moved to a separate
function for clarity of code, as a prework for the next patch fixing
the bug.

Signed-off-by: Lukasz Orlowski <lukasz.orlowski@intel.com>
---
 super-intel.c |  195 ++++++++++++++++++++++++++++++---------------------------
 1 files changed, 102 insertions(+), 93 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index dfa85aa..c669135 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6982,6 +6982,106 @@ error_disk_add:
 	return ret_val;
 }
 
+static int apply_update_activate_spare(struct imsm_update_activate_spare *u,
+		struct intel_super *super, struct active_array *active_array) {
+	struct imsm_super *mpb = super->anchor;
+	struct imsm_dev *dev = get_imsm_dev(super, u->array);
+	struct imsm_map *map = get_imsm_map(dev, 0);
+	struct imsm_map *migr_map;
+	struct active_array *a;
+	struct imsm_disk *disk;
+	__u8 to_state;
+	struct dl *dl;
+	unsigned int found;
+	int failed;
+	int victim = get_imsm_disk_idx(dev, u->slot, -1);
+	int i;
+
+		for (dl = super->disks; dl; dl = dl->next)
+			if (dl == u->dl)
+				break;
+
+		if (!dl) {
+			fprintf(stderr, "error: imsm_activate_spare passed "
+					"an unknown disk (index: %d)\n",
+					u->dl->index);
+			return 0;
+		}
+
+		/* count failures (excluding rebuilds and the victim)
+		 * to determine map[0] state
+		 */
+		failed = 0;
+		for (i = 0; i < map->num_members; i++) {
+			if (i == u->slot)
+				continue;
+			disk = get_imsm_disk(super,
+					get_imsm_disk_idx(dev, i, -1));
+			if (!disk || is_failed(disk))
+				failed++;
+		}
+
+		/* adding a pristine spare, assign a new index */
+		if (dl->index < 0) {
+			dl->index = super->anchor->num_disks;
+			super->anchor->num_disks++;
+		}
+		disk = &dl->disk;
+		disk->status |= CONFIGURED_DISK;
+		disk->status &= ~SPARE_DISK;
+
+		/* mark rebuild */
+		to_state = imsm_check_degraded(super, dev, failed);
+		map->map_state = IMSM_T_STATE_DEGRADED;
+		migrate(dev, super, to_state, MIGR_REBUILD);
+		migr_map = get_imsm_map(dev, 1);
+		set_imsm_ord_tbl_ent(map, u->slot, dl->index);
+		set_imsm_ord_tbl_ent(migr_map, u->slot,
+				dl->index | IMSM_ORD_REBUILD);
+
+		/* update the family_num to mark a new container
+		 * generation, being careful to record the existing
+		 * family_num in orig_family_num to clean up after
+		 * earlier mdadm versions that neglected to set it.
+		 */
+		if (mpb->orig_family_num == 0)
+			mpb->orig_family_num = mpb->family_num;
+		mpb->family_num += super->random;
+
+		/* count arrays using the victim in the metadata */
+		found = 0;
+		for (a = active_array; a ; a = a->next) {
+			dev = get_imsm_dev(super, a->info.container_member);
+			map = get_imsm_map(dev, 0);
+
+			if (get_imsm_disk_slot(map, victim) >= 0)
+				found++;
+		}
+
+		/* delete the victim if it is no longer being
+		 * utilized anywhere
+		 */
+		if (!found) {
+			struct dl **dlp;
+
+			/* We know that 'manager' isn't touching anything,
+			 * so it is safe to delete
+			 */
+			for (dlp = &super->disks; *dlp; dlp = &(*dlp)->next)
+				if ((*dlp)->index == victim)
+					break;
+
+			/* victim may be on the missing list */
+			if (!*dlp)
+				for (dlp = &super->missing; *dlp;
+						dlp = &(*dlp)->next)
+					if ((*dlp)->index == victim)
+						break;
+			imsm_delete(super, dlp, victim);
+		}
+
+	return 1;
+}
 
 static int apply_reshape_container_disks_update(struct imsm_update_reshape *u,
 						struct intel_super *super,
@@ -7276,99 +7376,8 @@ static void imsm_process_update(struct supertype *st,
 	}
 	case update_activate_spare: {
 		struct imsm_update_activate_spare *u = (void *) update->buf; 
-		struct imsm_dev *dev = get_imsm_dev(super, u->array);
-		struct imsm_map *map = get_imsm_map(dev, 0);
-		struct imsm_map *migr_map;
-		struct active_array *a;
-		struct imsm_disk *disk;
-		__u8 to_state;
-		struct dl *dl;
-		unsigned int found;
-		int failed;
-		int victim = get_imsm_disk_idx(dev, u->slot, -1);
-		int i;
-
-		for (dl = super->disks; dl; dl = dl->next)
-			if (dl == u->dl)
-				break;
-
-		if (!dl) {
-			fprintf(stderr, "error: imsm_activate_spare passed "
-				"an unknown disk (index: %d)\n",
-				u->dl->index);
-			return;
-		}
-
-		super->updates_pending++;
-		/* count failures (excluding rebuilds and the victim)
-		 * to determine map[0] state
-		 */
-		failed = 0;
-		for (i = 0; i < map->num_members; i++) {
-			if (i == u->slot)
-				continue;
-			disk = get_imsm_disk(super,
-					     get_imsm_disk_idx(dev, i, -1));
-			if (!disk || is_failed(disk))
-				failed++;
-		}
-
-		/* adding a pristine spare, assign a new index */
-		if (dl->index < 0) {
-			dl->index = super->anchor->num_disks;
-			super->anchor->num_disks++;
-		}
-		disk = &dl->disk;
-		disk->status |= CONFIGURED_DISK;
-		disk->status &= ~SPARE_DISK;
-
-		/* mark rebuild */
-		to_state = imsm_check_degraded(super, dev, failed);
-		map->map_state = IMSM_T_STATE_DEGRADED;
-		migrate(dev, super, to_state, MIGR_REBUILD);
-		migr_map = get_imsm_map(dev, 1);
-		set_imsm_ord_tbl_ent(map, u->slot, dl->index);
-		set_imsm_ord_tbl_ent(migr_map, u->slot, dl->index | IMSM_ORD_REBUILD);
-
-		/* update the family_num to mark a new container
-		 * generation, being careful to record the existing
-		 * family_num in orig_family_num to clean up after
-		 * earlier mdadm versions that neglected to set it.
-		 */
-		if (mpb->orig_family_num == 0)
-			mpb->orig_family_num = mpb->family_num;
-		mpb->family_num += super->random;
-
-		/* count arrays using the victim in the metadata */
-		found = 0;
-		for (a = st->arrays; a ; a = a->next) {
-			dev = get_imsm_dev(super, a->info.container_member);
-			map = get_imsm_map(dev, 0);
-
-			if (get_imsm_disk_slot(map, victim) >= 0)
-				found++;
-		}
-
-		/* delete the victim if it is no longer being
-		 * utilized anywhere
-		 */
-		if (!found) {
-			struct dl **dlp;
-
-			/* We know that 'manager' isn't touching anything,
-			 * so it is safe to delete
-			 */
-			for (dlp = &super->disks; *dlp; dlp = &(*dlp)->next)
-				if ((*dlp)->index == victim)
-					break;
-
-			/* victim may be on the missing list */
-			if (!*dlp)
-				for (dlp = &super->missing; *dlp; dlp = &(*dlp)->next)
-					if ((*dlp)->index == victim)
-						break;
-			imsm_delete(super, dlp, victim);
-		}
+		if (apply_update_activate_spare(u, super, st->arrays))
+			super->updates_pending++;
 		break;
 	}
 	case update_create_array: {

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 2/2] imsm: fix: Fixes metadata after migration from Raid 0 to Raid 10

[Lukasz Orlowski]

From: root <root@gklab-128-192.igk.intel.com>

After migration from Raid 0 to Raid 10, the metadata is incorrect,
leaving one mirror disk marked as spare and one missing disk as a member
of the array.
The reason is that the metadata update code for spare activation
procedure takes into account one spare disk only, not checking
the following ones.

Signed-off-by: Lukasz Orlowski <lukasz.orlowski@intel.com>
---
 super-intel.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index c669135..dbac50a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6994,8 +6994,15 @@ static int apply_update_activate_spare(struct imsm_update_activate_spare *u,
 	struct dl *dl;
 	unsigned int found;
 	int failed;
-	int victim = get_imsm_disk_idx(dev, u->slot, -1);
+	int victim;
 	int i;
+	int second_map_created = 0;
+
+	for (; u; u = u->next) {
+		victim = get_imsm_disk_idx(dev, u->slot, -1);
+
+		if (victim < 0)
+			return 0;
 
 		for (dl = super->disks; dl; dl = dl->next)
 			if (dl == u->dl)
@@ -7032,8 +7039,12 @@ static int apply_update_activate_spare(struct imsm_update_activate_spare *u,
 
 		/* mark rebuild */
 		to_state = imsm_check_degraded(super, dev, failed);
-		map->map_state = IMSM_T_STATE_DEGRADED;
-		migrate(dev, super, to_state, MIGR_REBUILD);
+		if (!second_map_created) {
+			second_map_created = 1;
+			map->map_state = IMSM_T_STATE_DEGRADED;
+			migrate(dev, super, to_state, MIGR_REBUILD);
+		} else
+			map->map_state = to_state;
 		migr_map = get_imsm_map(dev, 1);
 		set_imsm_ord_tbl_ent(map, u->slot, dl->index);
 		set_imsm_ord_tbl_ent(migr_map, u->slot,
@@ -7079,6 +7090,7 @@ static int apply_update_activate_spare(struct imsm_update_activate_spare *u,
 						break;
 			imsm_delete(super, dlp, victim);
 		}
+	}
 
 	return 1;
 }

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 0/2] Fixes metadata while migrating from Raid 0 to Raid 10

[NeilBrown]

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

On Fri, 21 Oct 2011 09:47:29 +0200 Lukasz Orlowski
<lukasz.orlowski@intel.com> wrote:

> The following series fixes metadata updates handling code after migration from Raid 0 to Raid 10:
> Last disk is left as a spare instead of being activated and a missing disk is a
> member of an array instead of being removed. The array status is degraded
> instead of normal.
> 
> ---
> 
> Lukasz Orlowski (1):
>       imsm: Moves metadata update code for spare activation to separate function
> 
> Lukasz Orlowski (1):
>       imsm: fix: Fixes metadata after migration from Raid 0 to Raid 10
> 
> 
>  super-intel.c |  207 +++++++++++++++++++++++++++++++--------------------------
>  1 files changed, 114 insertions(+), 93 deletions(-)
> 

Thanks.
I've applied this with some re-indenting (which suspect you only left that
way to it was easier to see exactly where the changes were).

Thanks,
NeilBrown

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