Re: [PATCH 1/3] Create: Allow to create two volumes of different sizes within one container

[NeilBrown <neilb@suse.de>]

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

On Tue, 20 Sep 2011 23:31:03 -0700 Dan Williams <dan.j.williams@intel.com>
wrote:

> On Tue, Sep 20, 2011 at 8:31 PM, NeilBrown <neilb@suse.de> wrote:
> > On Mon, 19 Sep 2011 18:52:31 +0200 Lukasz Orlowski
> > <lukasz.orlowski@intel.com> wrote:
> >
> >> Allows to create RAID 5 volume on 3 disks and then RAID 1 volume on 2
> >> disks withing the same container.
> >>
> >> Signed-off-by: Lukasz Orlowski <lukasz.orlowski@intel.com>
> >> ---
> >>  super-intel.c |    6 ++++++
> >>  1 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/super-intel.c b/super-intel.c
> >> index a78d723..616853b 100644
> >> --- a/super-intel.c
> >> +++ b/super-intel.c
> >> @@ -5041,6 +5041,12 @@ 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");
> >>
> >
> > This patch doesn't make sense.
> >
> > Firstly the description seems backwards.  The purpose of this patch seems to
> > disallow the creation of two volumes with different numbers of devices, but
> > the description seems to say that it allows it.  But that is a small point.
> >
> > ->num_disks is the number of devices in the container including spares.  This
> > patch would allow the first array in a container to have fewer devices than
> > the container with the rest being spares.  However the second array would have
> > to have the same number of devices as the container - even if this is more
> > than the first array.
> >
> > Presumably what you really want to do is:
> >  if num_raid_devs > 0, then find the relevant imsm_map, and then check if
> >     map->num_members == raiddisks
> >  and fail if they are not equal.
> >
> 
> Yeah, the changelog is backwards, and this breaks tests/08imsm-overlap
> because it does not honor IMSM_NO_PLATFORM.  How about the attached?

Better (though text/plain attachments are preferred over
application/octet-stream....)

However there still seems to be confusion over num_disks versus num_members.

You compare num_disks to raiddisks but my understanding is that num_disks can
include spares while raiddisks definitely doesn't.  Am I confused or are you?

NeilBrown


> 
> Lukasz, please copy me on patch submissions.
> 
> Thanks,
> Dan


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