On Tue, 20 Sep 2011 23:31:03 -0700 Dan Williams <dan.j.williams@xxxxxxxxx>
wrote:
> On Tue, Sep 20, 2011 at 8:31 PM, NeilBrown <neilb@xxxxxxx> wrote:
> > On Mon, 19 Sep 2011 18:52:31 +0200 Lukasz Orlowski
> > <lukasz.orlowski@xxxxxxxxx> 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@xxxxxxxxx>
> >> ---
> >> 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:
signature.asc
Description: PGP signature
