On Thu, 6 Jan 2011 08:43:35 -0600 Jonathan Brassow [off-list ref]
wrote:

On Jan 6, 2011, at 4:46 AM, NeilBrown wrote:

quoted

Patches 1 and 2 aren't really needed - I feel inclined not to remove  
that
stuff until all the dust settles..  So I'll put them at the bottom  
of my
queue and bring them forward in a couple of releases if that seems
appropriate.

No problem.

quoted

Patch 3 I'll push out through my tree shortly.

thanks

quoted

The rest I am happy with going out through 'dm' to -next and beyond  
whenever
you like.

However I will take this opportunity to suggest that the raid  
parameters
might need a bit of review before they are finalised.

The possible parameters are as follows:
<chunk_size>		Chunk size in sectors.
[[no]sync]		Force/Prevent RAID initialization
[rebuild <idx>]	Rebuild the drive indicated by the index
[daemon_sleep <ms>]	Time between bitmap daemon work to clear bits
[min_recovery_rate <kB/sec/disk>]	Throttle RAID initialization
[max_recovery_rate <kB/sec/disk>]	Throttle RAID initialization
[max_write_behind <value>]		See '-write-behind=' (man mdadm)
[stripe_cache <sectors>]		Stripe cache size for higher RAIDs


'rebuild idx' only allows one device to be rebuilt at a time.  With  
RAID6 it
is possible that two devices can be ready for rebuild, in which case  
we would
want to rebuild both of them.

In which case, you can specify the parameter twice:
"... rebuild 0 rebuild 1..."
No problem there, I think.

Yes, I see.  I'm use to seeing positional parameter in dm tables and didn't
process properly that these are key/value parameters.

quoted

Also if you stop an array during a rebuild you really want to start  
again
where you were up to.

This is kept in the superblock (forthcoming patches).  In the case  
where metadata devices are not specified (or can't be specified, as in  
this initial set of patches), the user has two choices: 1) allow the  
array to completely resync, or 2) specify [no]sync.

OK... though 'resync' and 'rebuild' are very different things and should not
be confused.  So the [no]sync option should not affect rebuild of new devices.

quoted

Most general solution would be to combine the rebuild information  
with the
list of component devices - either a sector number or something to  
indicate
that the rebuild it complete (which could be just a v.big sector  
number).

Is there something more to this, or is the above comment suitable for  
this also?

No, I think you've address it all.

quoted

I'd much prefer 'daemon_sleep' to be in seconds, we decimals  
supported if
needed.

I had this in seconds originally, but device-mapper tables already  
have a precedent for specifying things in ms.  We don't want to have  
each target pick their own units.  (A similar thing can be said for  
values given in sectors.  We may have wanted kiB or B, but sectors is  
the standard.)

Fair enough.

quoted

Also:

3:	<#raid_devs> <meta_dev1> <dev1> .. <meta_devN> <devN>

This doesn't allow offsets into the various devices like dm-stripe,
dm-linear, dm-raid1 all do.

This is a conscious choice.  The offset was discarded because it is  
unneeded.

Famous last words!!!

When reshaping a RAID5/6 to change the chunk size or layout, or when
converting an N drive RAID5 to N+1 drive RAID6, it makes it a lot easier if
you can can change the data offset at the same time.  Then you don't have to
keep backing up data.  md cannot do this yet but I hope to add the code soon
and because 'data_offset' is a distinct value it is conceptually easy to do.
If 'data_offset' is assumed to be zero, then it isn't.

But I don't know to what extent you are planning on supporting reshaping of
arrays....

Thanks,

NeilBrown

  brassow