Re: [PATCH 5/9] imsm: fix reserved sectors for spares

[NeilBrown <neilb@suse.de>]

On Fri, 26 Aug 2011 12:51:18 -0700 "Williams, Dan J"
<dan.j.williams@intel.com> wrote:

> On Thu, Aug 25, 2011 at 7:14 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > Different OROMs reserve different amounts of space for the migration area.
> > When activating a spare minimize the reserved space otherwise a valid spare
> > can be prevented from joining an array with a migration area smaller than
> > IMSM_RESERVED_SECTORS.
> >
> > This may result in an array that cannot be reshaped, but that is less
> > surprising than not being able to rebuild a degraded array.
> > imsm_reserved_sectors() already reports the minimal value which adds to
> > the confusion when trying rebuild an array because mdadm -E indicates
> > that the device has enough space.
> >
> > Cc: Anna Czarnowska <anna.czarnowska@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  super-intel.c |   11 ++++++++++-
> >  1 files changed, 10 insertions(+), 1 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index 0347183..193e0d0 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -833,7 +833,16 @@ static struct extent *get_extents(struct intel_super *super, struct dl *dl)
> >        struct extent *rv, *e;
> >        int i;
> >        int memberships = count_memberships(dl, super);
> > -       __u32 reservation = MPB_SECTOR_CNT + IMSM_RESERVED_SECTORS;
> > +       __u32 reservation;
> > +
> > +       /* trim the reserved area for spares, so they can join any array
> > +        * regardless of whether the OROM has assigned sectors from the
> > +        * IMSM_RESERVED_SECTORS region
> > +        */
> > +       if (dl->index == -1)
> > +               reservation = MPB_SECTOR_CNT;
> 
> Anna rightly points out that this could probably be safely made
> bigger.  As it stands this applies to too broad an array of disks.  I
> think a happy medium (until we can nail down the forward compatibility
> of older oroms, v8 in this case) is to detect the case where the disk
> is being activated for rebuild and if it is at least as large as one
> of the existing members truncate the reserved region to the same size
> as the other member.  That way we are at least compatible with
> whatever agent created the array in the first instance.
> 

I think you are saying that I can go ahead and apply this patch, but that it
might get improved upon in the future .... I hope that is right ?

> But this also comes back to the problem of duplicating the array
> configuration.  It becomes difficult to make things the same size
> unless the orom version (reserved region layout) is specified.

Yes, that is occasionally a serious problem.  As metadata becomes more
flexible it become harder to reproduce exact configuration.

Maybe I need a --no-default option to --create to ensure no defaults are used.
Then some sort of generic 
    --config-param foo=bar
which creates a dictionary of foo=bar assignments which are used by the
metadata to fill in any detail that they would normally fill from defaults.

Sounds a bit heavy-weight though...

NeilBrown

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html