DM RAID: do not print rebuilds in table line

Commit Message

Jonthan Brassow May 23, 2011, 7:54 p.m. UTC

Patch name: dm-raid-do-not-print-rebuilds-in-table-line.patch

DM RAID table status should not include rebuild parameters.

When a DM RAID device is constructed, a user may supply a 'rebuild <idx>'
pair of parameters to indicate that they want a specific device in the
array rebuilt.  This action should clear the 'recovery_offset' for the
device so that any subsequent reloading of the device will pick up where
it left off.  The rebuild option is unnecessary and, in fact, unwanted
unless it is the intent to begin the rebuild process again.  Therefore,
we should never include the rebuild parameters when printing the DM
table via the status function.

Signed-off-by: Jonathan Brassow <[e-mail]>


--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Alasdair G Kergon May 25, 2011, 2:05 p.m. UTC | #1

On Mon, May 23, 2011 at 02:54:51PM -0500, Jon Brassow wrote:
> DM RAID table status should not include rebuild parameters.

Please could you give an example of a sequence of actual 'dmsetup table' +
'dmsetup status' lines after this change?  (Including reloads during and after
rebuild.)

Please update the documentation file too to mention the asymmetry, if some 
parameter is now input-only.

+ Performing a 'dmsetup table' should display the CTR table used to
+ construct the mapping (with possible reordering of optional
+ parameters).

BTW LVM2 is likely to rely on the order being preserved i.e. it must supply
them to the kernel in the order it is known to print them.  (Remember that the
'table' line gets compared as an unparsed string in userspace.)  Also update it
to say what it does rather than what it 'should' do:)

(We'll increment the API version too of course in the patch.)

Thanks,
Alasdair

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Patch

Index: linux-2.6/drivers/md/dm-raid.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-raid.c
+++ linux-2.6/drivers/md/dm-raid.c
@@ -672,14 +672,11 @@  static int raid_status(struct dm_target 
 		break;
 	case STATUSTYPE_TABLE:
 		/* The string you would use to construct this array */
-		for (i = 0; i < rs->md.raid_disks; i++) {
-			if (rs->dev[i].data_dev &&
-			    !test_bit(In_sync, &rs->dev[i].rdev.flags))
-				raid_param_cnt += 2; /* for rebuilds */
+		for (i = 0; i < rs->md.raid_disks; i++)
 			if (rs->dev[i].data_dev &&
 			    test_bit(WriteMostly, &rs->dev[i].rdev.flags))
 				raid_param_cnt += 2;
-		}
+
 		raid_param_cnt += (hweight64(rs->print_flags) * 2);
 		if (rs->print_flags & (DMPF_SYNC | DMPF_NOSYNC))
 			raid_param_cnt--;
@@ -692,14 +689,10 @@  static int raid_status(struct dm_target 
 			DMEMIT(" sync");
 		if (rs->print_flags & DMPF_NOSYNC)
 			DMEMIT(" nosync");
-		for (i = 0; i < rs->md.raid_disks; i++) {
-			if (rs->dev[i].data_dev &&
-			    !test_bit(In_sync, &rs->dev[i].rdev.flags))
-				DMEMIT(" rebuild %u", i);
+		for (i = 0; i < rs->md.raid_disks; i++)
 			if (rs->dev[i].data_dev &&
 			    test_bit(WriteMostly, &rs->dev[i].rdev.flags))
 				DMEMIT(" write_mostly %u", i);
-		}
 
 		if (rs->print_flags & DMPF_DAEMON_SLEEP)
 			DMEMIT(" daemon_sleep %lu",