On Sun, Oct 08 2017, John Stoffel wrote:
>>>>>> "NeilBrown" == NeilBrown <neilb@xxxxxxxx> writes:
>
> NeilBrown> On Wed, Oct 04 2017, John Stoffel wrote:
>>> Since Eli had such a horrible experience where he shrunk the
>>> individual component raid device size, instead of growing the overall
>>> raid by adding a device, I came up with this hacky patch to warn you
>>> when you are about to shoot yourself in the foot.
>>>
>>> The idea is it will warn you and exit unless you pass in the --force
>>> (or -f) switch when using the command. For example, on a set of loop
>>> devices:
>>>
>>> # cat /proc/mdstat
>>> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5]
>>> [raid4] [multipath] [faulty]
>>> md99 : active raid6 loop4p1[4] loop3p1[3] loop2p1[2] loop1p1[1]
>>> loop0p1[0]
>>> 606720 blocks super 1.2 level 6, 512k chunk, algorithm 2 [5/5]
>>> [UUUUU]
>>>
>>> # ./mdadm --grow /dev/md99 --size 128
>>> mdadm: Cannot set device size smaller than current component_size of /dev/md99 array. Use -f to force change.
>>>
>>> # ./mdadm --grow /dev/md99 --size 128 -f
>>> mdadm: component size of /dev/md99 has been set to 0K
>>>
>
> NeilBrown> I'm not sure I like this.
> NeilBrown> The reason that mdadm will quietly accept a size change like this is
> NeilBrown> that it is trivial to revert - just set the same to a big number and all
> NeilBrown> your data is still there.
>
> This is wrong, because if you use --grow --size ### with a small
> enough number, it destroys the MD raid superblock.
If that is true, then it is a kernel bug and should be fixed in the kernel.
> So again, I think
> the --force option is *critical* here. Or we need to block the size
> change from going smaller than the superblock size. Here's my test,
> where I just warn if the size is going to be smaller:
>
> # ./mdadm --grow /dev/md99 --size 128
> mdadm: setting raid component device size from 202240 to 128 in array /dev/md99,
> this may need to be reverted if new size is smaller.
> mdadm: component size of /dev/md99 has been set to 0K
>
> # ./mdadm --grow /dev/md99 --size 202240
> mdadm: setting raid component device size from 0 to 202240 in array /dev/md99,
> this may need to be reverted if new size is smaller.
> mdadm: Cannot set device size in this type of array.
>
> # mdadm -E /dev/md99
> mdadm: No md superblock detected on /dev/md99.
>
> So I think this argues for a much stronger check, and/or the --force
> option when shrinking. I'll re-spin my patch series into two chunks,
> one just the message if changing size. The second to require the
> --force option.
Why don't you like my suggestion that you should need to reduce the
--array-size first?
Thanks,
NeilBrown
>
> And I think we need a third option to make sure the size can't be
> smaller than the array superblock size as well. Otherwise a simple
> mistake trashes your array.
>
> My current warning only patch (with whitespace damage...)
>
>> git diff
> diff --git a/Grow.c b/Grow.c
> index 455c5f9..18aea63 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1625,6 +1625,10 @@ int Grow_reshape(char *devname, int fd,
> return 1;
> }
>
> + if (s->size != (unsigned)array.size) {
> + pr_err("setting raid component device size from %u to %llu in array %s,\nthis may need to be reverted if new size is smaller.\n",(unsigned)array.size,s->size,devname);
> + }
> +
> st = super_by_fd(fd, &subarray);
> if (!st) {
> pr_err("Unable to determine metadata format for %s\n", devname);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
