On Fri, 08 Apr 2022, Pascal Hambourg wrote:
> Le 07/04/2022 à 00:55, NeilBrown wrote:
> > On Wed, 06 Apr 2022, Pascal Hambourg wrote:
> >> On 05/04/2022, NeilBrown wrote:
> >>> On Wed, 06 Apr 2022, Pascal Hambourg wrote:
> >>>>
> >>>> This is a question about original/alternate layout enforcement for RAID0
> >>>> arrays with members of different sizes introduced by commits
> >>>> c84a1372df929033cb1a0441fb57bd3932f39ac9 ("md/raid0: avoid RAID0 data
> >>>> corruption due to layout confusion.)" and
> >>>> 33f2c35a54dfd75ad0e7e86918dcbe4de799a56c ("md: add feature flag
> >>>> MD_FEATURE_RAID0_LAYOUT").
> >>>>
> >>>> The layout is irrelevant if all members have the same size so the array
> >>>> has only one zone. But isn't it also irrelevant if the array has two
> >>>> zones and the second zone has only one device, for example if the array
> >>>> has two members of different sizes ?
> >>>
> >>> Yes.
> >>
> >> So wouldn't it make sense to allow assembly even when the layout is
> >> undefined, like what is done when the array has only one zone ?
> >>
> > Yes.
> >
> > P.S. maybe you would like to try making the code change yourself, and
> > posting the patch.
>
> Sure. In order to check the number of devices in the second zone I had
> to move the layout check after all zones are set up. Is this fine ?
Thanks!
Reviewed-By: NeilBrown <neilb@xxxxxxx>
It might be nice to resend with a proper commit message.
Borrowing your words and teaking them a little, it could read:
The RAID0 layout is irrelevant if all members have the same size so the
array has only one zone. It is *also*( irrelevant if the array has two
zones and the second zone has only one device, for example if the array
has two members of different sizes ?
So in that case it makes sense to allow assembly even when the layout is
undefined, like what is done when the array has only one zone
Thanks,
NeilBrown
>
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index b21e101183f4..7623811cc11c 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -128,21 +128,6 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
> pr_debug("md/raid0:%s: FINAL %d zones\n",
> mdname(mddev), conf->nr_strip_zones);
>
> - if (conf->nr_strip_zones == 1) {
> - conf->layout = RAID0_ORIG_LAYOUT;
> - } else if (mddev->layout == RAID0_ORIG_LAYOUT ||
> - mddev->layout == RAID0_ALT_MULTIZONE_LAYOUT) {
> - conf->layout = mddev->layout;
> - } else if (default_layout == RAID0_ORIG_LAYOUT ||
> - default_layout == RAID0_ALT_MULTIZONE_LAYOUT) {
> - conf->layout = default_layout;
> - } else {
> - pr_err("md/raid0:%s: cannot assemble multi-zone RAID0 with default_layout setting\n",
> - mdname(mddev));
> - pr_err("md/raid0: please set raid0.default_layout to 1 or 2\n");
> - err = -ENOTSUPP;
> - goto abort;
> - }
> /*
> * now since we have the hard sector sizes, we can make sure
> * chunk size is a multiple of that sector size
> @@ -273,6 +258,22 @@ static int create_strip_zones(struct mddev *mddev, struct r0conf **private_conf)
> (unsigned long long)smallest->sectors);
> }
>
> + if (conf->nr_strip_zones == 1 || conf->strip_zone[1].nb_dev == 1) {
> + conf->layout = RAID0_ORIG_LAYOUT;
> + } else if (mddev->layout == RAID0_ORIG_LAYOUT ||
> + mddev->layout == RAID0_ALT_MULTIZONE_LAYOUT) {
> + conf->layout = mddev->layout;
> + } else if (default_layout == RAID0_ORIG_LAYOUT ||
> + default_layout == RAID0_ALT_MULTIZONE_LAYOUT) {
> + conf->layout = default_layout;
> + } else {
> + pr_err("md/raid0:%s: cannot assemble multi-zone RAID0 with default_layout setting\n",
> + mdname(mddev));
> + pr_err("md/raid0: please set raid0.default_layout to 1 or 2\n");
> + err = -ENOTSUPP;
> + goto abort;
> + }
> +
> pr_debug("md/raid0:%s: done.\n", mdname(mddev));
> *private_conf = conf;
>
>
