Re: Grow set size issue

[Tomasz Majchrzak <tomasz.majchrzak@intel.com>]

On Thu, Mar 30, 2017 at 10:10:32AM -0400, Jes Sorensen wrote:
> On 03/29/2017 05:34 PM, NeilBrown wrote:
> >On Thu, Mar 30 2017, jes.sorensen@gmail.com wrote:
> >
> >>Hi Neil,
> >>
> >>In the below patch you changed the error handling, to make the kernel
> >>not setting the size of the device being an error. However we still have
> >>the code in place to handle the error, except it never triggers.
> >
> >So we do.  I should have removed all of that.
> >I should have just reverted
> >Commit: 65a9798b58b4 ("FIX: Detect error and rollback metadata")
> >
> >
> >>
> >>Question is do you remember the reason for this change? Old kernels not
> >>allowing it, are there any legitimate reasons for the kernel to refuse
> >>the size change?
> >
> >I needed to go further back to remind myself why we do these size change
> >at all.
> >The command being run here is "mdadm --grow /dev/mdX --size=foo"
> >which has a primary purpose of changing the component_size of the array.
> >What can happen is that someone makes all the components bigger
> >(E.g. with LVM) and then uses this command to set --size=max, and it
> >doesn't work.  That is because md doesn't know the devices are bigger.
> >
> >You can tell md that devices have changes size by writing to the "size"
> >attribute.
> >mdadm doesn't have an option for doing that per-device - you need to
> >poke into sysfs.
> >
> >To make it a bit easier, when you use "--grow --size=foo", mdadm will
> >always write "foo" to the "size" attribute of each device, just incase
> >that will be helpful. In the case where the device is now bigger, it
> >will.
> >
> >In the case where the size of the array is being reduced, it is not
> >permitted to change the "size" of each device until the "component_size"
> >of the array has changed, so these attempts to change "size" will fail.
> >But that isn't a problem.
> >
> >In short, the attempt to change "size" here is a convenience, and
> >optimization.  It doesn't matter if it fails.
> >
> >So please just revert all bits of the above commit that are still
> >present.
> 
> Hi Neil,
> 
> Thanks for the explanation, I'll take the big hammer to the
> leftovers and get rid of them.
> 
> Jes

Hi,

The commit 758b327cf5a ("Grow: Remove unnecessary optimization") breaks grow
operation for external metadata.

mdadm --create /dev/md/imsm0 --metadata=imsm --raid-devices=4 /dev/nvme[0-3]n1 --run
mdadm --create /dev/md/r5 --level=5 --chunk 64 --size=1G --raid-devices=3 /dev/nvme[0-2]n1 --run --assume-clean

MDADM_EXPERIMENTAL=1 mdadm --grow /dev/md/r5 --size=2G
mdadm: Cannot set device size for /dev/md/r5: No space left on device

From update_size in MD driver:

	if (avail < num_sectors)
		-ENOSPC;

In the above case 'avail' is set to old value (1GiB) and new size requested
('num_sectors') is 2GiB.

Shall we revert the recent change then?

Regards,

Tomek