From: Williams, Dan J [mailto:dan.j.williams@xxxxxxxxx] Sent: Thursday, December 15, 2011 6:16 AM To: NeilBrown Cc: Labun, Marcin; Ciechanowski, Ed; linux-raid@xxxxxxxxxxxxxxx; Kwolek, Adam Subject: Re: [PATCH 3/3] imsm: FIX: UT '08imsm-overlap' fails On Dec 14, 2011 8:03 PM, "NeilBrown" <neilb@xxxxxxx> wrote: > > On Wed, 14 Dec 2011 10:21:07 -0800 "Williams, Dan J" > <dan.j.williams@xxxxxxxxx> wrote: > > > On Wed, Dec 14, 2011 at 7:07 AM, Adam Kwolek <adam.kwolek@xxxxxxxxx> wrote: > > > Problem was introduced by patch (2011-09-19): > > > Create: Allow to create two volumes of different sizes within one container > > > > > > To resolve problem check requested disks number not against all disks > > > recorded in metadata, but against disks number set in array map structure. > > > > > > Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx> > > > --- > > > > > > super-intel.c | 24 ++++++++++++++++++++---- > > > 1 files changed, 20 insertions(+), 4 deletions(-) > > > > > > diff --git a/super-intel.c b/super-intel.c > > > index 5e1d278..3c10d29 100644 > > > --- a/super-intel.c > > > +++ b/super-intel.c > > > @@ -5318,10 +5318,26 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level, > > > > > > mpb = super->anchor; > > > > > > - if (mpb->num_raid_devs > 0 && mpb->num_disks != raiddisks) { > > > - fprintf(stderr, Name ": the option-rom requires all " > > > - "member disks to be a member of all volumes.\n"); > > > - return 0; > > > + /* check if currently verified volume spans the same disks number > > > + * as all other arrays that belongs to metadata. > > > + * Do not allow to create volume with different disks number > > > + * than curently is used. > > > + */ > > > + for (i = 0; i < mpb->num_raid_devs; i++) { > > > + /* There is any already existing array in metadata > > > + */ > > > + struct imsm_dev *dev = get_imsm_dev(super, i); > > > + struct imsm_map *map = NULL; > > > + if (dev) > > > + map = get_imsm_map(dev, MAP_0); > > > + if (map) { > > > + if (raiddisks != map->num_members) { > > > + fprintf(stderr, Name ": the option-rom requires" > > > + " all member disks to be a member of " > > > + "all volumes.\n"); > > > + return 0; > > > + } > > > + } > > > > I'd prefer to move this check back to the other 'orom' checks in > > validate_geometry_imsm_volume(), and make it dependent on the presence > > of the orom. > > > > diff --git a/super-intel.c b/super-intel.c > > index 07d47b5..9885b98 100644 > > --- a/super-intel.c > > +++ b/super-intel.c > > @@ -5122,12 +5122,6 @@ static int validate_geometry_imsm_volume(struct > > supertype *st, int level, > > if (!super) > > return 0; > > > > - if (mpb->num_raid_devs > 0 && mpb->num_disks != raiddisks) { > > - fprintf(stderr, Name ": the option-rom requires all " > > - "member disks to be a member of all volumes.\n"); > > - return 0; > > - } > > - > > if (!validate_geometry_imsm_orom(super, level, layout, raiddisks, > > chunk, verbose)) { > > fprintf(stderr, Name ": RAID gemetry validation failed. " > > "Cannot proceed with the action(s).\n"); > > @@ -5206,6 +5200,11 @@ static int validate_geometry_imsm_volume(struct > > supertype *st, int level, > > fprintf(stderr, Name ": The option-rom requires all member" > > " disks to be a member of all volumes\n"); > > return 0; > > + } else if (super->orom && mpb->num_raid_devs > 0 && > > + mpb->num_disks != raiddisks) { > > + fprintf(stderr, Name ": The option-rom requires all member" > > + " disks to be a member of all volumes\n"); > > + return 0; > > } > > > > /* retrieve the largest free space block */ > > > Hi Dan, > I think you are presenting this as an alternate - is that correct? Yes. > It seems a lit simpler - does it do enough? I believe so, I'd like Adam to confirm. I'm not sure if checking mpb->num_disks != raiddisks is enough /or it is rather too much/. In disk list could be different number of members than in map (e.g. rebuild when disks list is longer). It was UT08* failure cause and I thing using maps length is better. I'm not sure, if making it dependent on the presence of the orom makes us happy from compatibility point of view. We shouldn't make metadata OROM incompatible. Every array can be mounted on OROM platform later, what scenario for such case? BR Adam (forgive brevity replying from phone) -- Dan -- 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
