[PATCH] DM RAID: Clean-up handling of 'rebuild' device requests

[PATCH] DM RAID: Clean-up handling of 'rebuild' device requests

[Jonathan Brassow]

dm-raid: Change the way the 'rebuild' parameter is processed.

1) We already allowed the rebuild flag to clear the superblock area for a
   device.  There is no reason to shame the user if they didn't clear the
   superblock area ahead of time.  Change the message to simply print-out
   that it is clearing the superblock for the purpose of a rebuild.
2) Set the 'FirstUse' flag in 'super_load' vs 'super_init_validation'.  This
   allows the devices that had 'rebuild' set to be processed like a new device.
3) Since the rebuild devices are processed like new devices, the MD_CHANGE_DEVS
   flag also gets set on the MD device.  This was a necessary missing action
   that forces the new superblock to be written when the array is instantiated.

Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>

Index: linux-upstream/drivers/md/dm-raid.c
===================================================================
--- linux-upstream.orig/drivers/md/dm-raid.c
+++ linux-upstream/drivers/md/dm-raid.c
@@ -667,7 +667,14 @@ static int super_load(struct md_rdev *rd
 		return ret;
 
 	sb = page_address(rdev->sb_page);
-	if (sb->magic != cpu_to_le32(DM_RAID_MAGIC)) {
+
+	/*
+	 * Two cases that we want to write new superblocks and rebuild:
+	 * 1) New device (no matching magic number)
+	 * 2) Device specified for rebuild (!In_sync w/ offset == 0)
+	 */
+	if ((sb->magic != cpu_to_le32(DM_RAID_MAGIC)) ||
+	    (!test_bit(In_sync, &rdev->flags) && !rdev->recovery_offset)) {
 		super_sync(rdev->mddev, rdev);
 
 		set_bit(FirstUse, &rdev->flags);
@@ -743,12 +750,9 @@ static int super_init_validation(struct 
 	 *    recovery_cp must be MaxSector.
 	 */
 	rdev_for_each(r, t, mddev) {
-		if (!test_bit(In_sync, &r->flags)) {
-			if (!test_bit(FirstUse, &r->flags))
-				DMERR("Superblock area of "
-				      "rebuild device %d should have been "
-				      "cleared.", r->raid_disk);
-			set_bit(FirstUse, &r->flags);
+		if (!test_bit(In_sync, &r->flags))
+			DMINFO("Device %d specified for rebuild: "
+			       "Clearing superblock", r->raid_disk);
 			rebuilds++;
 		} else if (test_bit(FirstUse, &r->flags))
 			new_devs++;

Re: [PATCH] DM RAID: Clean-up handling of 'rebuild' device requests

[Jonathan Brassow]

On Nov 28, 2011, at 2:01 PM, Jonathan Brassow wrote:

> dm-raid: Change the way the 'rebuild' parameter is processed.
> 	rdev_for_each(r, t, mddev) {
> -		if (!test_bit(In_sync, &r->flags)) {
> -			if (!test_bit(FirstUse, &r->flags))
> -				DMERR("Superblock area of "
> -				      "rebuild device %d should have been "
> -				      "cleared.", r->raid_disk);
> -			set_bit(FirstUse, &r->flags);
> +		if (!test_bit(In_sync, &r->flags))

Missing '{' - will resend.

 brassow

[PATCH - v2] DM RAID: 'rebuild' parameter code should be setting MD_CHANGE_DEVS

[Jonathan Brassow]

Changes from previous:
- Add missing '{'
- Change comments to emphasize bug fix

 brassow

dm-raid: 'rebuild' parameter code should be setting MD_CHANGE_DEVS

The 'rebuild' parameter is used to rebuild individual devices in an array.
(Resynchronize a RAID1 device or recalculate a parity device in higher RAID,
for example.)  The MD_CHANGE_DEVS flag must be set when this parameter is
given in order to write out the superblocks and make the change take immediate
effect.

So, we need MD_CHANGE_DEVS to be set and the 'FirstUse' flag was being set as
a special case for rebuilds in super_init_validation.  The code that handles
new devices in super_load already sets MD_CHANGE_DEVS and 'FirstUse'.  Adding
a condition for rebuilds in super_load takes care of both flags without a need
for the special case in 'super_init_validation'.

Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>

Index: linux-upstream/drivers/md/dm-raid.c
===================================================================
--- linux-upstream.orig/drivers/md/dm-raid.c
+++ linux-upstream/drivers/md/dm-raid.c
@@ -667,7 +667,14 @@ static int super_load(struct md_rdev *rd
 		return ret;
 
 	sb = page_address(rdev->sb_page);
-	if (sb->magic != cpu_to_le32(DM_RAID_MAGIC)) {
+
+	/*
+	 * Two cases that we want to write new superblocks and rebuild:
+	 * 1) New device (no matching magic number)
+	 * 2) Device specified for rebuild (!In_sync w/ offset == 0)
+	 */
+	if ((sb->magic != cpu_to_le32(DM_RAID_MAGIC)) ||
+	    (!test_bit(In_sync, &rdev->flags) && !rdev->recovery_offset)) {
 		super_sync(rdev->mddev, rdev);
 
 		set_bit(FirstUse, &rdev->flags);
@@ -744,11 +751,8 @@ static int super_init_validation(struct 
 	 */
 	rdev_for_each(r, t, mddev) {
 		if (!test_bit(In_sync, &r->flags)) {
-			if (!test_bit(FirstUse, &r->flags))
-				DMERR("Superblock area of "
-				      "rebuild device %d should have been "
-				      "cleared.", r->raid_disk);
-			set_bit(FirstUse, &r->flags);
+			DMINFO("Device %d specified for rebuild: "
+			       "Clearing superblock", r->raid_disk);
 			rebuilds++;
 		} else if (test_bit(FirstUse, &r->flags))
 			new_devs++;