On Mon, 5 Dec 2011 18:19:09 -0800 "Williams, Dan J"
<dan.j.williams@xxxxxxxxx> wrote:
> On Mon, Dec 5, 2011 at 5:10 PM, NeilBrown <neilb@xxxxxxx> wrote:
> > On Thu, 1 Dec 2011 14:23:16 +0000 "Dorau, Lukasz" <lukasz.dorau@xxxxxxxxx>
> > 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...
>
> Lukasz is right. We want to find out if starting the array with the
> current list of disks in the container would regress the state of the
> array recorded in the metadata. map[0] should always record the best
> possible state of all the slots in the array if any of those have gone
> missing we don't want incremental assembly to proceed.
>
> Sorry for the noise, my point was 'correct' in isolation but it missed
> that the context is looking for the most optimistic view of the disk.
Great - thanks for clearing that up.
NeilBrown
Attachment:
signature.asc
Description: PGP signature
