Re: [PATCH] Grow: Do not use grow-continue unit file if reshape is starting

[NeilBrown <neilb@suse.de>]

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

On Mon, 7 Jul 2014 14:18:40 +0000 "Baldysiak, Pawel"
<pawel.baldysiak@intel.com> wrote:

> > On Monday, July 07, 2014 3:06 AM Neil Brown wrote:
> > To: Baldysiak, Pawel
> > Cc: linux-raid@vger.kernel.org; Paszkiewicz, Artur
> > Subject: Re: [PATCH] Grow: Do not use grow-continue unit file if reshape is
> > starting
> > 
> > On Fri, 4 Jul 2014 08:59:00 +0000 "Baldysiak, Pawel"
> > <pawel.baldysiak@intel.com> wrote:
> > 
> > > Mdadm should use mdadm-grow-continue unit file only if reshape is
> > > going to be continued. Otherwise, array specific reshape with IMSM
> > > metadata will fail to start, due to missing information about ongoing
> > > migration - grow-continue will try to start again the reshape process.
> > 
> > I don't think I agree.  I think mdadm should always use mdadm-grow-
> > continue unit file if it is available.
> > 
> > Please explain in more detail what problem you are seeing.
> > 
> > NeilBrown
> > 
> 
> Hi Neil
> 
> If we try to do array-specific reshape (e.g. change from level 0 to 5) 
> mdadm will impose all changes to metadata and start reshape - before running grow-continue.
> After that  grow-continue will see that reshape progress is '0', so will overwrite "restart" by setting it to =0.

I assume you are referring to this code:
	if (st->ss->external && restart && (info->reshape_progress == 0)) {
          .....
		if ((verify_reshape_position(info, reshape.level) >= 0) &&
		    (info->reshape_progress == 0))
			restart = 0;
	}

in reshape_array().

> Then it will try to impose all changes again and fail - with error:
> "Cannot set device shape for /dev/md/vol" - It will be unable to write changes to sysfs,
> due to already started reshape (which freeze at '0' progress).
> 
> We can try to fix it by moving part of code that runs grow-continue in reshape_array earlier,
>  so "start_reshape() " will be executed by grow-continue (I am testing that kind of patch now), 
> or forbid running grow-continue when reshape is about to start (patch is tested, attach in my prev mail).
> 
> What do you think about that?
> Do you have any other idea how to fix this issue?

I suspect that the real problem is in the code above.
Maybe it should read the "sync_action" attribute and if it is "reshape", then
this is obviously a restart and it shouldn't set "restart" to 0.

Can you try something like that?

NeilBrown


> 
> Thanks
> Pawel Baldysiak
> 
> > >
> > > Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
> > > ---
> > >  Grow.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Grow.c b/Grow.c
> > > index a2f4f14..0cd9442 100644
> > > --- a/Grow.c
> > > +++ b/Grow.c
> > > @@ -3272,7 +3272,7 @@ started:
> > >  		return 1;
> > >  	}
> > >
> > > -	if (!forked && !check_env("MDADM_NO_SYSTEMCTL"))
> > > +	if (restart && !forked && !check_env("MDADM_NO_SYSTEMCTL"))
> > >  		if (continue_via_systemd(container ?: sra->sys_name)) {
> > >  			free(fdlist);
> > >  			free(offsets);



-- 

-- 

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