[PATCH v2] md/raid0: Ignore RAID0 layout if the second zone has only one device

[PATCH v2] md/raid0: Ignore RAID0 layout if the second zone has only one device

[Pascal Hambourg]

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.

Reviewed-By: NeilBrown <neilb@suse.de>
Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>
---

Changes since v1:
- add missing Signed-off-by
- add missing subsystem maintainer in recipients

---
  drivers/md/raid0.c | 31 ++++++++++++++++---------------
  1 file changed, 16 insertions(+), 15 deletions(-)

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;

-- 
2.11.0

Re: [PATCH v2] md/raid0: Ignore RAID0 layout if the second zone has only one device

[Song Liu]

On Tue, Apr 12, 2022 at 11:54 PM Pascal Hambourg <pascal@plouf.fr.eu.org> wrote:
>
> 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.
>
> Reviewed-By: NeilBrown <neilb@suse.de>
> Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>

Thanks for the patch and thanks Neil for the review.

Applied to md-next with two minor changes:

s/ENOTSUPP/EOPNOTSUPP/
s/Reviewed-By/Review-by/

Thanks,
Song

> ---
>
> Changes since v1:
> - add missing Signed-off-by
> - add missing subsystem maintainer in recipients
>
> ---
>   drivers/md/raid0.c | 31 ++++++++++++++++---------------
>   1 file changed, 16 insertions(+), 15 deletions(-)
>
> 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;
>
> --
> 2.11.0
>

Re: [PATCH v2] md/raid0: Ignore RAID0 layout if the second zone has only one device

[NeilBrown]

On Fri, 15 Apr 2022, Song Liu wrote:
> On Tue, Apr 12, 2022 at 11:54 PM Pascal Hambourg <pascal@plouf.fr.eu.org> wrote:
> >
> > 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.
> >
> > Reviewed-By: NeilBrown <neilb@suse.de>
> > Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>
> 
> Thanks for the patch and thanks Neil for the review.
> 
> Applied to md-next with two minor changes:
> 
> s/ENOTSUPP/EOPNOTSUPP/

Why did you change the error code that was returned?  If you want to do
that it should be a separate patch with explanation.

> s/Reviewed-By/Review-by/

Do you mean s/Reviewed-By/Reviewed-by/  - i.e. change the B to b ??

Since v5.0 there have only be 5 "Review-by" and over 100,000
"Reviewed-by" (and about 100 "Reviewed-By").

Thanks,
NeilBrown


> 
> Thanks,
> Song
> 
> > ---
> >
> > Changes since v1:
> > - add missing Signed-off-by
> > - add missing subsystem maintainer in recipients
> >
> > ---
> >   drivers/md/raid0.c | 31 ++++++++++++++++---------------
> >   1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > 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;
> >
> > --
> > 2.11.0
> >
> 

Re: [PATCH v2] md/raid0: Ignore RAID0 layout if the second zone has only one device

[Pascal Hambourg]

Le 14/04/2022 à 21:02, Song Liu a écrit :
> On Tue, Apr 12, 2022 at 11:54 PM Pascal Hambourg <pascal@plouf.fr.eu.org> wrote:
>>
>> 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.
>>
>> Reviewed-By: NeilBrown <neilb@suse.de>
>> Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>
> 
> Thanks for the patch and thanks Neil for the review. >
> Applied to md-next with two minor changes:

Do you think this patch qualifies for -stable too ?