Re: [PATCH] imsm: fix: correct checking newly missing disks

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1783 bytes --]

On Thu, 1 Dec 2011 14:23:16 +0000 "Dorau, Lukasz" <lukasz.dorau@intel.com>
wrote:
> > > diff --git a/super-intel.c b/super-intel.c
> > > index 4ebee78..511a32a 100644
> > > --- a/super-intel.c
> > > +++ b/super-intel.c
> > > @@ -2529,13 +2529,13 @@ static void getinfo_super_imsm(struct supertype
> > *st, struct mdinfo *info, char *
> > >
> > >                failed = imsm_count_failed(super, dev);
> > >                state = imsm_check_degraded(super, dev, failed);
> > > -               map = get_imsm_map(dev, dev->vol.migr_state);
> > > +               map = get_imsm_map(dev, 0);
> > >
> > >                /* any newly missing disks?
> > >                 * (catches single-degraded vs double-degraded)
> > >                 */
> > >                for (j = 0; j < map->num_members; j++) {
> > > -                       __u32 ord = get_imsm_ord_tbl_ent(dev, i, -1);
> > > +                       __u32 ord = get_imsm_ord_tbl_ent(dev, i, 0);
> > 
> > This looks wrong.  I noticed this when looking over Przemyslaw's patch [1].
> > 
> > map[0] always contains the destination state of the migration so the
> > most reliable source for looking for out of sync disks is map[1].
> > 
> 
> I am convinced that the patch is good. 
> We are looking for information what was the state of array during migration (before it was stopped), so we have to use map[0].
> map[1] contains information about the state of array before migration, which we do not need.
> 
> Regards,
> Lukasz

Hi,
 do we have agreement on this?  Dan - do you stand by your original concern
 or have you seen the light :-)

The patch is in, but I'd like to be sure it is right and to be honest I
haven't followed the dance of the maps too closely...

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]