>>>>> "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. 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.
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
