Require number of stripes to be greater than parity devices in higher RAID.

Also, add some comments to code that I recently added that may be unclear
otherwise.

4 files changed, 30 insertions, 3 deletions

diff --git a/WHATS_NEW b/WHATS_NEW
index 1d8bd526..dc62fcbd 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.93 - 
 ====================================
+  Require number of stripes to be greater than parity devices in higher RAID.
   Add LVMetaD systemd units.
   Fix allocation code to allow replacement of single RAID 4/5/6 device.
   Check all tags and LV names are in a valid form in vg_validate.
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index a743c075..fef26451 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -752,6 +752,22 @@ static struct alloc_handle *_alloc_init(struct cmd_context *cmd,
 		area_count = stripes;
 
 	size = sizeof(*ah);
+
+	/*
+	 * It is a requirement that RAID 4/5/6 are created with a number of
+	 * stripes that is greater than the number of parity devices.  (e.g
+	 * RAID4/5 must have at least 2 stripes and RAID6 must have at least
+	 * 3.)  It is also a constraint that, when replacing individual devices
+	 * in a RAID 4/5/6 array, no more devices can be replaced than
+	 * there are parity devices.  (Otherwise, there would not be enough
+	 * redundancy to maintain the array.)  Understanding these two
+	 * constraints allows us to infer whether the caller of this function
+	 * is intending to allocate an entire array or just replacement
+	 * component devices.  In the former case, we must account for the
+	 * necessary parity_count.  In the later case, we do not need to
+	 * account for the extra parity devices because the array already
+	 * exists and they only want replacement drives.
+	 */
 	parity_count = (area_count <= segtype->parity_devs) ? 0 :
 		segtype->parity_devs;
 	alloc_count = area_count + parity_count;
diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c
index e60af176..a4a6a40d 100644
--- a/lib/metadata/raid_manip.c
+++ b/lib/metadata/raid_manip.c
@@ -519,9 +519,18 @@ static int _alloc_image_components(struct logical_volume *lv,
 	else if (!(segtype = get_segtype_from_string(lv->vg->cmd, "raid1")))
 		return_0;
 
+	/*
+	 * The number of extents is based on the RAID type.  For RAID1,
+	 * each of the rimages is the same size - 'le_count'.  However
+	 * for RAID 4/5/6, the stripes add together (NOT including the parity
+	 * devices) to equal 'le_count'.  Thus, when we are allocating
+	 * individual devies, we must specify how large the individual device
+	 * is along with the number we want ('count').
+	 */
 	extents = (segtype->parity_devs) ?
 		(lv->le_count / (seg->area_count - segtype->parity_devs)) :
 		lv->le_count;
+
 	if (!(ah = allocate_extents(lv->vg, NULL, segtype, 0, count, count,
 				    region_size, extents, pvs,
 				    lv->alloc, parallel_areas)))
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 7d3a5b0f..ebc1a0ec 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -485,9 +485,10 @@ static int _read_raid_params(struct lvcreate_params *lp,
 	 *
 	 * For RAID 4/5/6, these values must be set.
 	 */
-	if (!segtype_is_mirrored(lp->segtype) && (lp->stripes < 2)) {
-		log_error("Number of stripes to %s not specified",
-			  lp->segtype->name);
+	if (!segtype_is_mirrored(lp->segtype) &&
+	    (lp->stripes <= lp->segtype->parity_devs)) {
+		log_error("Number of stripes must be at least %d for %s",
+			  lp->segtype->parity_devs + 1, lp->segtype->name);
 		return 0;
 	}