- To: NeilBrown <neilb@xxxxxxx>
- Subject: RE: [PATCH 07/14] FIX: Extend size of raid0 array
- From: "Dorau, Lukasz" <lukasz.dorau@xxxxxxxxx>
- Date: Fri, 20 Apr 2012 08:48:31 +0000
- Accept-language: en-US
- Cc: "linux-raid@xxxxxxxxxxxxxxx" <linux-raid@xxxxxxxxxxxxxxx>, "Williams, Dan J" <dan.j.williams@xxxxxxxxx>, "Ciechanowski, Ed" <ed.ciechanowski@xxxxxxxxx>, "Patelczyk, Maciej" <maciej.patelczyk@xxxxxxxxx>, "Danecki, Jacek" <jacek.danecki@xxxxxxxxx>, "Kwolek, Adam" <adam.kwolek@xxxxxxxxx>
- In-reply-to: <20120420072349.7636d58f@notabene.brown>
- Thread-index: AQHNHEVuIzurNTqaxEiRINTooYM6ipaiO7GggABgbICAAM3oAA==
- Thread-topic: [PATCH 07/14] FIX: Extend size of raid0 array
On Thursday, April 19, 2012 11:24 PM NeilBrown <neilb@xxxxxxx> wrote:
>
> On Thu, 19 Apr 2012 15:09:45 +0000 "Dorau, Lukasz"
> <lukasz.dorau@xxxxxxxxx>
> wrote:
>
> > On Tuesday, April 17, 2012 4:53 AM NeilBrown wrote:
> > >
> > > On Fri, 13 Apr 2012 16:52:01 +0200 Adam Kwolek
> <adam.kwolek@xxxxxxxxx>
> > > wrote:
> > >
> > > > For raid0, takeover operation is required for size change.
> > > > Add takeover to degraded raid4 before size change and back to raid0
> after.
> > > > Array information has to be read again from md after takeover.
> > > >
> > > > Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx>
> > > > ---
> > > > Grow.c | 35 +++++++++++++++++++++++++++++++++++
> > > > 1 files changed, 35 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/Grow.c b/Grow.c
> > > > index 8c7bafc..e8f6554 100644
> > > > --- a/Grow.c
> > > > +++ b/Grow.c
> > > > @@ -1626,6 +1626,7 @@ int Grow_reshape(char *devname, int fd, int
> > > quiet, char *backup_file,
> > > > long long orig_size = get_component_size(fd)/2;
> > > > long long min_csize;
> > > > struct mdinfo *mdi;
> > > > + int raid0_takeover = 0;
> > > >
> > > > if (orig_size == 0)
> > > > orig_size = array.size;
> > > > @@ -1674,6 +1675,28 @@ int Grow_reshape(char *devname, int fd, int
> > > quiet, char *backup_file,
> > > > "2TB per device\n");
> > > > size = min_csize;
> > > > }
> > > > + if (st->ss->external) {
> > > > + if (sra->array.level == 0) {
> > > > + rv = sysfs_set_str(sra, NULL, "level",
> > > > + "raid5");
> > > > + if (!rv) {
> > > > + raid0_takeover = 1;
> > > > + /* get array parametes after takeover
> > > > + * to chane one parameter at time
> > > only
> > > > + */
> > > > + rv = ioctl(fd, GET_ARRAY_INFO,
> > > &array);
> > > > + }
> > > > + }
> > > > + /* make sure mdmon is
> > > > + * aware of the new level */
> > > > + if (!mdmon_running(st->container_dev))
> > > > + start_mdmon(st->container_dev);
> > > > + ping_monitor(container);
> > > > + if (mdmon_running(st->container_dev) &&
> > > > + st->update_tail == NULL)
> > > > + st->update_tail = &st->updates;
> > > > + }
> > > > +
> > > > array.size = size;
> > > > if (array.size != size) {
> > > > /* got truncated to 32bit, write to
> > > > @@ -1686,12 +1709,24 @@ int Grow_reshape(char *devname, int fd,
> int
> > > quiet, char *backup_file,
> > > > rv = -1;
> > > > } else {
> > > > rv = ioctl(fd, SET_ARRAY_INFO, &array);
> > > > +
> > > > /* manage array size when it is managed externally
> > > > */
> > > > if ((rv == 0) && st->ss->external)
> > > > rv = set_array_size(st, sra, sra->text_version);
> > > > }
> > > >
> > > > + if (raid0_takeover) {
> > > > + /* do not recync non-existing parity,
> > > > + * we will drop it anyway
> > > > + */
> > > > + sysfs_set_str(sra, NULL, "sync_action", "idle");
> > >
> > > This is probably ineffective. If the array thinks it needs to resync, then
> > > just setting it to 'idle' will only stop in temporarily. It will immediately
> > > start syncing again.
> > >
> > > However given that it is a RAID5 array which is degraded, it shouldn't try
> to
> > > resync at all.
> > >
> > > So: does it still work if you remove this line? If so, could you send a
> > > patch to remove it?
> > > If not - we should work out why.
> > >
> >
> > It doesn't work without this line.
> > The N-disks raid0 changes to degraded (N+1)-disks raid5 (N of N+1 disks)
> > in /proc/mdstat after size extension, however the metadata on all disks is
> correct.
> >
>
> In that case I suspect that fact that it works with the line is racy and it
> could sometimes fail. Setting "sync_action" to "frozen" would be safer.
> Can you confirm that also works? I'd feel more comfortable about that.
>
You're right. I've checked it on another machine and it sometimes fails.
I've also checked that with "sync_action" set to "frozen" it works.
I'll send the patch soon.
Regards,
Lukasz
--
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
[ATA RAID]
[Linux SCSI Target Infrastructure]
[Managing RAID on Linux]
[Linux IDE]
[Linux SCSI]
[Linux Hams]
[Device-Mapper]
[Kernel]
[Linux Books]
[Linux Admin]
[Linux Net]
[GFS]
[RPM]
[git]
[Photos]
[Yosemite Photos]
[Yosemite News]
[AMD 64]
[Linux Networking]