Re: [PATCH 01/10] mdadm/Grow: Update new level when starting reshape

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Mon, 2 Sep 2024 11:50:13 +0200
Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote:

> Hi Xiao,
> Thanks for patches.
> 
> On Wed, 28 Aug 2024 10:11:41 +0800
> Xiao Ni <xni@redhat.com> wrote:
> 
> > Reshape needs to specify a backup file when it can't update data offset
> > of member disks. For this situation, first, it starts reshape and then
> > it kicks off mdadm-grow-continue service which does backup job and
> > monitors the reshape process. The service is a new process, so it needs
> > to read superblock from member disks to get information.  
> 
> Looks like kernel is fine with reset the same level so I don't see a risk in
> this change for other scenarios but please mention that.
> 

Sorry, I didn't notice that it is new field. We should not update it if it
doesn't exist. Perhaps, we should print message that kernel patch is needed? 

> > 
> > But in the first step, it doesn't update new level in superblock. So
> > it can't change level after reshape finishes, because the new level is
> > not right. So records the new level in the first step.  
> 
> > 
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >  Grow.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Grow.c b/Grow.c
> > index 5810b128aa99..97e48d86a33f 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -2946,6 +2946,9 @@ static int impose_reshape(struct mdinfo *sra,
> >  		if (!err && sysfs_set_num(sra, NULL, "layout",
> >  					  reshape->after.layout) < 0)
> >  			err = errno;
> > +		if (!err && sysfs_set_num(sra, NULL, "new_level",
> > +					info->new_level) < 0)
> > +			err = errno;  
> 
> Please add empty line before and after and please merge if statement to one
> line (we support up to 100).
> 
> 
> >  		if (!err && subarray_set_num(container, sra, "raid_disks",
> >  					     reshape->after.data_disks +
> >  					     reshape->parity) < 0)  
> 
> 
> Thanks,
> Mariusz
>