On Fri, 15 Mar 2013 15:48:23 -0500 Jonathan Brassow [off-list ref]
wrote:

DM RAID:  Add message/status support for changing sync action

This patch adds a message interface to dm-raid to allow the user to more
finely control the sync actions being performed by the MD driver.  This
gives the user the ability to initiate "check" and "repair" (i.e. scrubbing).
Two additional fields have been added to the status output to provide more
information about the type of sync action occurring and the results of those
actions, specifically: <sync_action> and <mismatch_cnt>.  This is essentially
the device-mapper way of doing what MD controls through the 'sync_action'
sysfs file and shows through the 'mismatch_cnt' sysfs file.

This is a first patch and I still have two concerns:
1) in 'raid_messages', reap_sync_thread() is not called when "frozen" or
   "idle" is issued like it would be in  md.c:action_store() because it is
   not available to dm-raid.c.  This hasn't prevented the thread from
   shutting down properly in my tests, but there may be something I'm
   not considering.

I think the only real need for the reap_sync_thread() call is to make the
change synchronous.  i.e. after "echo frozen > sync_action" completes, you
know that the array actually is frozen, rather that it should be frozen very
soon.  This can be important for code which wants to set 'frozen', then
manipulate the array in a way that would be prevented if it was still
undergoing a resync/recovery/check/etc.

I'm don't know how much I actually depend on that property, but it sounds
like a good one to have.
I would probably be OK to export reap_sync_thread() so that dm-raid can use
it.

2) (and this is more of an LVM problem)  The "in-sync ratio" field of the
   status output has generally been used to determine the progress of the
   initial synchronization or of rebuilding a device.  "check" and "repair"
   have not been available options until now.  So, if this ratio also
   shows the progress of a "check", older versions of LVM would seem to
   indicate (through the 'sync%' field) that the array is not in-sync, when
   in fact it is.  Thus, this change might be perceived as a break to
   backwards compatability.  (However, the old LVM tools would not be able
   to /issue/ a "check" command and should never run into this.)  I don't
   see this as a problem for this patch, but I thought it should be
   brought up and considered.   Obviously, code in LVM will need to be
   changed to adjust to this - perhaps to show the different types of sync
   actions that might be occurring.

I see your point.  I don't have any opinion on whether it is a real problem
or just a theoretical one though.

In general, the patch looks good - particular as it includes documentation
changes!  When you get a very you are happy with I'll apply it.

Thanks,
NeilBrown