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

On Tue, 20 Sep 2011 23:31:03 -0700 Dan Williams [off-list ref]
wrote:

On Tue, Sep 20, 2011 at 8:31 PM, NeilBrown [off-list ref] wrote:

quoted

On Mon, 19 Sep 2011 18:52:31 +0200 Lukasz Orlowski
[off-list ref] wrote:

quoted

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 <redacted>
---
 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