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
>
I'm not sure I like this.
The reason that mdadm will quietly accept a size change like this is
that it is trivial to revert - just set the same to a big number and all
your data is still there.
Eli's problem was that he made a harmless mistake, realized that he had
made a mistake, but didn't address the mistake before continuing!
If you really want to make this a two-step process, an approach that
would be more consistent with other aspects of mdadm is to require that
--array-size be reduced first. i.e. setting --size mustn't reduce the
size of the array.
I think that separating the two steps (resize array, resize component)
gives the user a better picture of what is happening, where as requiring
-f just causes people to use -f more often.
Thanks,
NeilBrown
>
> I suspect I could do a better job of showing the original component
> size, so that you have a chance of recovering even then.
>
> But the patch:
>
> diff --git a/Grow.c b/Grow.c
> index 455c5f9..701590f 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1561,7 +1561,7 @@ static int reshape_container(char *container, char *devname,
> char *backup_file, int verbose,
> int forked, int restart, int freeze_reshape);
>
> -int Grow_reshape(char *devname, int fd,
> +int Grow_reshape(char *devname, int fd, int force,
> struct mddev_dev *devlist,
> unsigned long long data_offset,
> struct context *c, struct shape *s)
> @@ -1574,6 +1574,7 @@ int Grow_reshape(char *devname, int fd,
> * requested everything (if kernel supports freezing - 2.6.30).
> * The steps are:
> * - change size (i.e. component_size)
> + * - when shrinking, you must force the change
> * - change level
> * - change layout/chunksize/ndisks
> *
> @@ -1617,6 +1618,11 @@ int Grow_reshape(char *devname, int fd,
> return 1;
> }
>
> + if ((s->size < (unsigned)array.size) && !force) {
> + pr_err("Cannot set device size smaller than current component_size of %s array. Use -f to force change.\n",devname);
> + return 1;
> + }
> +
> if (s->raiddisks && s->raiddisks < array.raid_disks && array.level > 1 &&
> get_linux_version() < 2006032 &&
> !check_env("MDADM_FORCE_FEWER")) {
> diff --git a/ReadMe.c b/ReadMe.c
> index 50d3807..46988ae 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -203,6 +203,7 @@ struct option long_options[] = {
> {"invalid-backup",0,0,InvalidBackup},
> {"array-size", 1, 0, 'Z'},
> {"continue", 0, 0, Continue},
> + {"force", 0, 0, Force},
>
> /* For Incremental */
> {"rebuild-map", 0, 0, RebuildMapOpt},
> @@ -563,6 +564,7 @@ char Help_grow[] =
> " : This is useful if all devices have been replaced\n"
> " : with larger devices. Value is in Kilobytes, or\n"
> " : the special word 'max' meaning 'as large as possible'.\n"
> +" --force -f : Override normal checks and be more forceful\n"
> " --assume-clean : When increasing the --size, this flag will avoid\n"
> " : a resync of the new space\n"
> " --chunk= -c : Change the chunksize of the array\n"
> diff --git a/mdadm.c b/mdadm.c
> index c3a265b..821658a 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -1617,7 +1617,7 @@ int main(int argc, char *argv[])
> else if (s.size > 0 || s.raiddisks || s.layout_str ||
> s.chunk != 0 || s.level != UnSet ||
> data_offset != INVALID_SECTORS) {
> - rv = Grow_reshape(devlist->devname, mdfd,
> + rv = Grow_reshape(devlist->devname, mdfd, c.force,
> devlist->next,
> data_offset, &c, &s);
> } else if (array_size == 0)
> diff --git a/mdadm.h b/mdadm.h
> index 71b8afb..9e00f05 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1300,7 +1300,7 @@ extern int autodetect(void);
> extern int Grow_Add_device(char *devname, int fd, char *newdev);
> extern int Grow_addbitmap(char *devname, int fd,
> struct context *c, struct shape *s);
> -extern int Grow_reshape(char *devname, int fd,
> +extern int Grow_reshape(char *devname, int fd, int force,
> struct mddev_dev *devlist,
> unsigned long long data_offset,
> struct context *c, struct shape *s);
> --
> 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
