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?
It seems a lit simpler - does it do enough?
Or is it just a precursor, and then we apply Adam's patch changing the same
code but in a different location?
Slightly confused...
NeilBrown
Attachment:
signature.asc
Description: PGP signature
