Re: [PATCH] mdadm: Check bitmap first when reshape raid1 to raid0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Original Message -----
> From: "Neil Brown" <neilb@xxxxxxx>
> To: "Xiao Ni" <xni@xxxxxxxxxx>
> Cc: "Jes Sorensen" <jes.sorensen@xxxxxxxxxx>, linux-raid@xxxxxxxxxxxxxxx
> Sent: Monday, October 26, 2015 4:21:36 AM
> Subject: Re: Fwd: [PATCH] mdadm: Check bitmap first when reshape raid1 to raid0
> 
> Xiao Ni <xni@xxxxxxxxxx> writes:
> 
> > Hi Neil
> >
> > I encountered one problem. When reshape one raid1 with bitmap to raid0,
> > it'll
> > lose legs.
> >
> > I sent the patch by git-send-email, but I can't see the mail in linux-raid.
> > So
> > I forward it again. And add signed-off-by line.
> >
> > Best Regards
> > Xiao
> >
> > ----- Forwarded Message -----
> > From: "Xiao Ni" <xni@xxxxxxxxxx>
> > To: xni@xxxxxxxxxx
> > Sent: Sunday, October 25, 2015 1:36:02 PM
> > Subject: [PATCH] mdadm: Check bitmap first when reshape raid1 to raid0
> >
> > One raid1 with bitmap is composed by 4 disks. It'll fail when rashape to
> > raid0
> > and lose 3 disks. It should check bitmap first when reshape raid1 to raid0.
> >
> > Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
> > ---
> >  Grow.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 80d7b22..5e9b0bb 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -1898,6 +1898,12 @@ size_change_error:
> >  	     array.layout == ((1 << 8) + 2) && !(array.raid_disks & 1)) ||
> >  	    (s->level == 0 && array.level == 1 && sra)) {
> >  		int err;
> > +
> > +		if (array.state & (1<<MD_SB_BITMAP_PRESENT)) {
> > +			cont_err("Bitmap must be removed before level can be changed\n");
> > +			rv = 1;
> > +			goto release;
> > +		}
> >  		err = remove_disks_for_takeover(st, sra, array.layout);
> >  		if (err) {
> >  			dprintf("Array cannot be reshaped\n");
> > @@ -2706,9 +2712,6 @@ static int impose_level(int fd, int level, char
> > *devname, int verbose)
> >  			err = errno;
> >  			pr_err("%s: could not set level to %s\n",
> >  				devname, c);
> > -			if (err == EBUSY &&
> > -			    (array.state & (1<<MD_SB_BITMAP_PRESENT)))
> > -				cont_err("Bitmap must be removed before level can be changed\n");
> >  			return err;
> >  		}
> >  		if (verbose >= 0)
> > --
> 
> You appear to have identified a problem that needs fixing, but the
> patch is fairly obviously wrong.
> 
> You've removed a test and error message that could apply to any level
> change, and added a test and error message that only applies to some
> specific level changes.
> Why does that error message no longer apply to all the other possible
> level changes?
> 
> The test that you have added is mostly OK ... but you missed something.
> Look around and similar nearby code.
> The test you removed certainly should stay.
> 
> NeilBrown
> 

Thanks for printing out the mistake. I checked the nearby code and found 
I missed close the cfd. Is this I missed? But I'm wondering that it doesn't 
close it in the following code. Now it's just closed when it's failed to
reshape raid1/raid10 to raid0.

 Grow.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Grow.c b/Grow.c
index 80d7b22..d711884 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1898,6 +1898,15 @@ size_change_error:
             array.layout == ((1 << 8) + 2) && !(array.raid_disks & 1)) ||
            (s->level == 0 && array.level == 1 && sra)) {
                int err;
+               
+               if (array.state & (1<<MD_SB_BITMAP_PRESENT)) {
+                       dprintf("Bitmap must be removed before level can be changed\n");
+                       if (cfd > -1)
+                               close(cfd);
+                       rv = 1;
+                       goto release;
+               }
+
                err = remove_disks_for_takeover(st, sra, array.layout);

Best Regards
Xiao

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



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux