Re: [PATCH 07/14] FIX: Extend size of raid0 array

[NeilBrown <neilb@suse.de>]

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

On Thu, 19 Apr 2012 15:09:45 +0000 "Dorau, Lukasz" <lukasz.dorau@intel.com>
wrote:

> On Tuesday, April 17, 2012 4:53 AM NeilBrown wrote:
> > 
> > On Fri, 13 Apr 2012 16:52:01 +0200 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> > 
> > > For raid0, takeover operation is required for size change.
> > > Add takeover to degraded raid4 before size change and back to raid0 after.
> > > Array information has to be read again from md after takeover.
> > >
> > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > > ---
> > >  Grow.c |   35 +++++++++++++++++++++++++++++++++++
> > >  1 files changed, 35 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/Grow.c b/Grow.c
> > > index 8c7bafc..e8f6554 100644
> > > --- a/Grow.c
> > > +++ b/Grow.c
> > > @@ -1626,6 +1626,7 @@ int Grow_reshape(char *devname, int fd, int
> > quiet, char *backup_file,
> > >  		long long orig_size = get_component_size(fd)/2;
> > >  		long long min_csize;
> > >  		struct mdinfo *mdi;
> > > +		int raid0_takeover = 0;
> > >
> > >  		if (orig_size == 0)
> > >  			orig_size = array.size;
> > > @@ -1674,6 +1675,28 @@ int Grow_reshape(char *devname, int fd, int
> > quiet, char *backup_file,
> > >  				"2TB per device\n");
> > >  			size = min_csize;
> > >  		}
> > > +		if (st->ss->external) {
> > > +			if (sra->array.level == 0) {
> > > +				rv = sysfs_set_str(sra, NULL, "level",
> > > +						   "raid5");
> > > +				if (!rv) {
> > > +					raid0_takeover = 1;
> > > +					/* get array parametes after takeover
> > > +					 * to chane one parameter at time
> > only
> > > +					 */
> > > +					rv = ioctl(fd, GET_ARRAY_INFO,
> > &array);
> > > +				}
> > > +			}
> > > +			/* make sure mdmon is
> > > +			 * aware of the new level */
> > > +			if (!mdmon_running(st->container_dev))
> > > +				start_mdmon(st->container_dev);
> > > +			ping_monitor(container);
> > > +			if (mdmon_running(st->container_dev) &&
> > > +					st->update_tail == NULL)
> > > +				st->update_tail = &st->updates;
> > > +		}
> > > +
> > >  		array.size = size;
> > >  		if (array.size != size) {
> > >  			/* got truncated to 32bit, write to
> > > @@ -1686,12 +1709,24 @@ int Grow_reshape(char *devname, int fd, int
> > quiet, char *backup_file,
> > >  				rv = -1;
> > >  		} else {
> > >  			rv = ioctl(fd, SET_ARRAY_INFO, &array);
> > > +
> > >  			/* manage array size when it is managed externally
> > >  			 */
> > >  			if ((rv == 0) && st->ss->external)
> > >  				rv = set_array_size(st, sra, sra->text_version);
> > >  		}
> > >
> > > +		if (raid0_takeover) {
> > > +			/* do not recync non-existing parity,
> > > +			 * we will drop it anyway
> > > +			 */
> > > +			sysfs_set_str(sra, NULL, "sync_action", "idle");
> > 
> > This is probably ineffective.  If the array thinks it needs to resync, then
> > just setting it to 'idle' will only stop in temporarily.  It will immediately
> > start syncing again.
> > 
> > However given that it is a RAID5 array which is degraded, it shouldn't try to
> > resync at all.
> > 
> > So:  does it still work if you remove this line?  If so, could you send a
> > patch to remove it?
> > If not - we should work out why.
> > 
> 
> It doesn't work without this line.
> The N-disks raid0 changes to degraded (N+1)-disks raid5 (N of N+1 disks) 
> in /proc/mdstat after size extension, however the metadata on all disks is correct.
> 

In that case I suspect that fact that it works with the line is racy and it
could sometimes fail.  Setting "sync_action" to "frozen" would be safer.
Can you confirm that also works?  I'd feel more comfortable about that.

NeilBrown


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