[linux-lvm] [PATCH LVM2] (0/3) 'cling' allocation policy

[linux-lvm] [PATCH LVM2] (0/3) 'cling' allocation policy

[Jun'ichi Nomura]

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

Hi,

These series of patches implement 'cling' allocation policy.
Patches are applicable to LVM2 2.02.10.

In current implementation, 'normal' policy allows the allocation
like this (output of lvs -a -o+devices):
  LV             Devices
  lv0            lv0_mimage_0(0),lv0_mimage_1(0)
  [lv0_mimage_0] /dev/sdb(0)
  [lv0_mimage_0] /dev/sda(2)
  [lv0_mimage_1] /dev/sda(0)
  [lv0_mimage_1] /dev/sdb(2)
  [lv0_mlog]     /dev/sdc(0)
  dummy1         /dev/sda(1)
  dummy2         /dev/sdb(1)
  dummy3         /dev/sdc(1)

You see /dev/sda is used for both lv0_mimage_0 and lv0_mimage_1.
If this disk breaks, you lose the whole LV.

In this case, what I think is better is something like this:
  LV             Devices
  lv0            lv0_mimage_0(0),lv0_mimage_1(0)
  [lv0_mimage_0] /dev/sdb(0)
  [lv0_mimage_0] /dev/sdb(2)
  [lv0_mimage_1] /dev/sda(0)
  [lv0_mimage_1] /dev/sda(2)
  [lv0_mlog]     /dev/sdc(0)
  dummy1         /dev/sda(1)
  dummy2         /dev/sdb(1)
  dummy3         /dev/sdc(1)

With these patches, 'normal' allocation tries to do the latter first.

Attached is a VG metadata as a testcase.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America

[-- Attachment #2: testvg0_alloc-cling.vg --]
[-- Type: text/plain, Size: 3171 bytes --]

# Generated by LVM2: Sat Oct  7 01:56:23 2006

contents = "Text Format Volume Group"
version = 1

description = "Created *before* executing 'lvextend -l+1 testvg0/lv0'"

creation_host = "tetsuo.lab"	# Linux tetsuo.lab 2.6.9-42.ELsmp #1 SMP Wed Jul 12 23:27:17 EDT 2006 i686
creation_time = 1160200583	# Sat Oct  7 01:56:23 2006

testvg0 {
	id = "1q8uy1-HgpO-JpQK-Dvn4-SYlQ-r452-YQa0P0"
	seqno = 7
	status = ["RESIZEABLE", "READ", "WRITE"]
	extent_size = 8192		# 4 Megabytes
	max_lv = 0
	max_pv = 0

	physical_volumes {

		pv0 {
			id = "EkAheU-E42S-nhmb-h3LB-CGEG-4NvX-f0PbFP"
			device = "/dev/dm-0"	# Hint only

			status = ["ALLOCATABLE"]
			pe_start = 384
			pe_count = 4	# 16 Megabytes
		}

		pv1 {
			id = "Ee7qjC-qtNm-TBd4-ql19-aRZg-mCXk-CCs5Uk"
			device = "/dev/dm-1"	# Hint only

			status = ["ALLOCATABLE"]
			pe_start = 384
			pe_count = 4	# 16 Megabytes
		}

		pv2 {
			id = "3wvgkO-7p1Z-yoHy-stW4-ZY3p-VHg1-DQe9d7"
			device = "/dev/dm-2"	# Hint only

			status = ["ALLOCATABLE"]
			pe_start = 384
			pe_count = 4	# 16 Megabytes
		}
	}

	logical_volumes {

		lv0 {
			id = "RIWMkB-slcN-4ZtA-iVrd-HIZO-PEO2-Z3ommp"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "mirror"
				mirror_count = 2
				mirror_log = "lv0_mlog"
				region_size = 1024

				mirrors = [
					"lv0_mimage_0", 0,
					"lv0_mimage_1", 0
				]
			}
		}

		dummy1 {
			id = "TlfCSG-kzkN-fv65-xd5g-Hpb0-I3dG-k7UVAJ"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv0", 1
				]
			}
		}

		dummy2 {
			id = "hWyYcp-FD0J-JiFy-dziI-g0FZ-Begs-aYEbJ2"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv1", 1
				]
			}
		}

		dummy3 {
			id = "NhIJ26-07w0-OA6d-ZE14-gegx-nVW8-YRM5vq"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv2", 1
				]
			}
		}

		lv0_mlog {
			id = "OP7pVI-Col0-vKbT-Zwar-M7CC-dZiP-WkV740"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv2", 0
				]
			}
		}

		lv0_mimage_0 {
			id = "2TWuNa-37S2-1m4O-SC02-uDEq-5l1O-AT3CcI"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv1", 0
				]
			}
		}

		lv0_mimage_1 {
			id = "S2EKHn-CroW-aFS7-ptF5-mYUo-DuYt-NmT1YD"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv0", 0
				]
			}
		}
	}
}

[linux-lvm] [PATCH LVM2] (1/3) remove unnecessary parameter from _check_contiguous

[Jun'ichi Nomura]

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

This patch removes 'struct physical_volume' from _check_contiguous.
The pv can be obtained later by pva->map->pv.

The patch doesn't fix any bug but is a preparation for later patch.

diffstat:
 lib/metadata/lv_manip.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America


[-- Attachment #2: 04-remove-unnecessary-arg-from-check_contiguous.patch --]
[-- Type: text/x-patch, Size: 1686 bytes --]

diff -X dontdiff -urp LVM2.03.regionsizefix/lib/metadata/lv_manip.c LVM2.04.shrink-checkcontig-arg/lib/metadata/lv_manip.c
--- LVM2.03.regionsizefix/lib/metadata/lv_manip.c	2006-10-05 20:21:27.000000000 -0400
+++ LVM2.04.shrink-checkcontig-arg/lib/metadata/lv_manip.c	2006-10-07 00:46:29.000000000 -0400
@@ -690,8 +690,7 @@ static int _comp_area(const void *l, con
 /*
  * Is pva contiguous to any existing areas or on the same PV?
  */
-static int _check_contiguous(struct lv_segment *prev_lvseg,
-			     struct physical_volume *pv, struct pv_area *pva,
+static int _check_contiguous(struct lv_segment *prev_lvseg, struct pv_area *pva,
 			     struct pv_area **areas, uint32_t areas_size)
 {
 	struct pv_segment *prev_pvseg;
@@ -703,7 +702,7 @@ static int _check_contiguous(struct lv_s
 			lastseg = list_item(list_last(&seg_lv(prev_lvseg, s)->segments), struct lv_segment);
 			/* FIXME For more areas supply flattened prev_lvseg to ensure consistency */
 			if (lastseg->area_count == 1 &&
-			    _check_contiguous(lastseg, pv, pva, &areas[s], 1))
+			    _check_contiguous(lastseg, pva, &areas[s], 1))
 				return 1;
 			continue;
 		}
@@ -711,7 +710,7 @@ static int _check_contiguous(struct lv_s
 		if (!(prev_pvseg = seg_pvseg(prev_lvseg, s)))
 			continue; /* FIXME Broken */
 
-		if ((prev_pvseg->pv != pv))
+		if ((prev_pvseg->pv != pva->map->pv))
 			continue;
 
 		if (prev_pvseg->pe + prev_pvseg->len == pva->start) {
@@ -907,7 +906,6 @@ static int _find_parallel_space(struct a
 				if (contiguous) {
 					if (prev_lvseg &&
 					    _check_contiguous(prev_lvseg,
-							      pvm->pv,
 							      pva, areas,
 							      areas_size)) {
 						contiguous_count++;

[linux-lvm] [PATCH LVM2] (2/3) use _for_each_pv() from _check_contiguous()

[Jun'ichi Nomura]

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

This patch rewrites _check_contiguous() to use _for_each_pv().

_for_each_pv() is a generic iterator function to execute given
function on each PV constituting the specified range of LV.
Since _check_contiguous() tries to find contiguous PV segment from
the end of the existing LV segment, I gave the last one LE as
a iteration range of _for_each_pv().

To pass necessary parameters to _for_each_pv callback (_match_pv()),
this patch adds new struct pv_match_handle.

The patch doesn't fix any bug but is a preparation for later patch.
Also, the use like this is what _for_each_pv() is intended for, I think.

diffstat:
 lv_manip.c |   89 +++++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 23 deletions(-)

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America


[-- Attachment #2: 05-use-for_each_pv-from-check_contiguous.patch --]
[-- Type: text/x-patch, Size: 3632 bytes --]

diff -X ../dontdiff -urp LVM2.04-shrink-checkcontig-arg/lib/metadata/lv_manip.c LVM2.05.use_for_each_pv/lib/metadata/lv_manip.c
--- LVM2.04-shrink-checkcontig-arg/lib/metadata/lv_manip.c	2006-10-07 02:46:21.000000000 -0400
+++ LVM2.05.use_for_each_pv/lib/metadata/lv_manip.c	2006-10-07 02:49:12.000000000 -0400
@@ -24,6 +24,11 @@
 #include "display.h"
 #include "segtype.h"
 
+static int _for_each_pv(struct cmd_context *cmd, struct logical_volume *lv,
+			uint32_t le, uint32_t len, uint32_t *max_seg_len,
+			int (*fn)(struct cmd_context *cmd, struct pv_segment *peg, void *data),
+			void *data);
+
 /*
  * PVs used by a segment of an LV
  */
@@ -680,42 +685,80 @@ static int _comp_area(const void *l, con
 	return 0;
 }
 
+struct pv_match_handle {
+	/* function to check if pv_segment fits for the pv_area */
+	int (*cond)(struct pv_segment *, struct pv_area *);
+
+	struct pv_area *pva; /* pv_area to match */
+	int index; /* index of pv in flattened lv segment */
+	int matched; /* 1 if matching pv_segment is found */
+};
+
 /*
- * Is pva contiguous to any existing areas or on the same PV?
+ * (_for_each_pv callback)
+ * Check if the pv_segment fits for the pv_match_handle
  */
-static int _check_contiguous(struct lv_segment *prev_lvseg, struct pv_area *pva,
-			     struct pv_area **areas, uint32_t areas_size)
+static int _match_pv(struct cmd_context *cmd, struct pv_segment *peg,
+		     void *data)
 {
-	struct pv_segment *prev_pvseg;
-	struct lv_segment *lastseg;
-	uint32_t s;
+	struct pv_match_handle *h = (struct pv_match_handle *)data;
 
-	for (s = 0; s < prev_lvseg->area_count && s < areas_size; s++) {
-		if (seg_type(prev_lvseg, s) == AREA_LV) {
-			lastseg = list_item(list_last(&seg_lv(prev_lvseg, s)->segments), struct lv_segment);
-			/* FIXME For more areas supply flattened prev_lvseg to ensure consistency */
-			if (lastseg->area_count == 1 &&
-			    _check_contiguous(lastseg, pva, &areas[s], 1))
-				return 1;
-			continue;
-		}
+	h->index++;
+	if (h->cond && h->cond(peg, h->pva)) {
+		h->matched = 1;
+		return 0; /* to exit from for_each_pv loop */
+	}
 
-		if (!(prev_pvseg = seg_pvseg(prev_lvseg, s)))
-			continue; /* FIXME Broken */
+	return 1;
+}
 
-		if ((prev_pvseg->pv != pva->map->pv))
-			continue;
+static int _find_and_set_pv_area(struct lv_segment *prev_lvseg,
+				 struct pv_area *pva,
+				 struct pv_area **areas, uint32_t areas_size,
+				 int (*cond)(struct pv_segment *,
+					     struct pv_area *))
+{
+	struct pv_match_handle pmh;
+
+	pmh.cond = cond;
+	pmh.pva = pva;
+	pmh.index = -1;
+	pmh.matched = 0;
+
+	/* search through all PVs in the last extent of the LV */
+	_for_each_pv(NULL, prev_lvseg->lv,
+		     prev_lvseg->le + prev_lvseg->len - 1, 1,
+		     NULL, _match_pv, &pmh);
 
-		if (prev_pvseg->pe + prev_pvseg->len == pva->start) {
-			areas[s] = pva;
-			return 1;
-		}
+	if (pmh.matched && pmh.index >= 0 && pmh.index < areas_size) {
+		areas[pmh.index] = pva;
+		return 1;
 	}
 
 	return 0;
 }
 
 /*
+ * Is pva contiguous to any existing areas or on the same PV?
+ */
+static int is_contiguous(struct pv_segment *prev, struct pv_area *pva)
+{
+	if ((prev->pv == pva->map->pv) &&
+	    (prev->pe + prev->len == pva->start))
+		return 1;
+
+	return 0;
+}
+
+static int _check_contiguous(struct lv_segment *prev_lvseg, struct pv_area *pva,
+			     struct pv_area **areas, uint32_t areas_size)
+{
+	return _find_and_set_pv_area(prev_lvseg, pva, areas, areas_size,
+				     is_contiguous);
+}
+
+
+/*
  * Choose sets of parallel areas to use, respecting any constraints.
  */
 static int _find_parallel_space(struct alloc_handle *ah, alloc_policy_t alloc,

[linux-lvm] [PATCH LVM2] (3/3) add 'cling' allocation policy

[Jun'ichi Nomura]

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

This patch adds 'cling' allocation policy.

Currently there are 3 policies: 'contiguous', 'normal', 'anywhere'.
LVM2 tries to apply from left to right until it succeeds allocation
or it failed to allocate with specified policy.

With this patch, policies are applied in the order:
'contiguous', 'cling', 'normal', 'anywhere'.

From man page:
 The contiguous policy requires  that
 new extents are adjacent to existing extents. If there are sufficient
 free extents to satisfy an allocation request but  normal doesn't use them,
 anywhere will - even if that reduces performance by placing two stripes
 on the same physical volume.

'cling' policy requires that new extents are on the same physical
volume with the existing extents.

diffstat:
 include/metadata.h      |    1
 lib/display/display.c   |    1
 lib/metadata/lv_manip.c |   49 ++++++++++++++++++++++++++++++++++++++++++++---- lib/metadata/metadata.h |    1
 lib/report/report.c     |    2 +
 man/lvm.8               |    4 ++-
 6 files changed, 53 insertions(+), 5 deletions(-)

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America


[-- Attachment #2: 06-add-cling-allocation-policy.patch --]
[-- Type: text/x-patch, Size: 6158 bytes --]

diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/include/metadata.h LVM2.06.alloc-cling/include/metadata.h
--- LVM2.05.use_for_each_pv/include/metadata.h	2006-10-05 19:45:15.000000000 -0400
+++ LVM2.06.alloc-cling/include/metadata.h	2006-10-07 02:49:55.000000000 -0400
@@ -82,6 +82,7 @@ typedef enum {
 	ALLOC_INVALID = 0,
 	ALLOC_INHERIT,
 	ALLOC_CONTIGUOUS,
+	ALLOC_CLING,
 	ALLOC_NORMAL,
 	ALLOC_ANYWHERE
 } alloc_policy_t;
diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/lib/display/display.c LVM2.06.alloc-cling/lib/display/display.c
--- LVM2.05.use_for_each_pv/lib/display/display.c	2006-07-29 00:46:08.000000000 -0400
+++ LVM2.06.alloc-cling/lib/display/display.c	2006-10-07 02:49:55.000000000 -0400
@@ -30,6 +30,7 @@ static struct {
 } _policies[] = {
 	{
 	ALLOC_CONTIGUOUS, "contiguous"}, {
+	ALLOC_CLING, "cling"}, {
 	ALLOC_NORMAL, "normal"}, {
 	ALLOC_ANYWHERE, "anywhere"}, {
 	ALLOC_INHERIT, "inherit"}
diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/lib/metadata/lv_manip.c LVM2.06.alloc-cling/lib/metadata/lv_manip.c
--- LVM2.05.use_for_each_pv/lib/metadata/lv_manip.c	2006-10-07 02:49:12.000000000 -0400
+++ LVM2.06.alloc-cling/lib/metadata/lv_manip.c	2006-10-07 02:51:14.000000000 -0400
@@ -757,6 +757,23 @@ static int _check_contiguous(struct lv_s
 				     is_contiguous);
 }
 
+/*
+ * Is pva on the same PV with any existing areas?
+ */
+static int is_same_pv(struct pv_segment *prev, struct pv_area *pva)
+{
+	if (prev->pv == pva->map->pv)
+		return 1;
+
+	return 0;
+}
+
+static int _check_same_pv(struct lv_segment *prev_lvseg, struct pv_area *pva,
+			  struct pv_area **areas, uint32_t areas_size)
+{
+	return _find_and_set_pv_area(prev_lvseg, pva, areas, areas_size,
+				     is_same_pv);
+}
 
 /*
  * Choose sets of parallel areas to use, respecting any constraints.
@@ -771,9 +788,9 @@ static int _find_parallel_space(struct a
 	struct pv_area *pva;
 	struct pv_list *pvl;
 	unsigned already_found_one = 0;
-	unsigned contiguous = 0, contiguous_count = 0;
+	unsigned contiguous = 0, cling = 0, prefered_count = 0;
 	unsigned ix;
-	unsigned ix_offset = 0;	/* Offset for non-contiguous allocations */
+	unsigned ix_offset = 0;	/* Offset for non-prioritized allocations */
 	uint32_t max_parallel;	/* Maximum extents to allocate */
 	uint32_t next_le;
 	struct seg_pvs *spvs;
@@ -786,6 +803,9 @@ static int _find_parallel_space(struct a
 	if ((alloc == ALLOC_CONTIGUOUS) && prev_lvseg) {
 		contiguous = 1;
 		ix_offset = prev_lvseg->area_count;
+	} else if ((alloc == ALLOC_CLING) && prev_lvseg) {
+		cling = 1;
+		ix_offset = prev_lvseg->area_count;
 	}
 
 	/* FIXME This algorithm needs a lot of cleaning up! */
@@ -846,7 +866,16 @@ static int _find_parallel_space(struct a
 					    _check_contiguous(prev_lvseg,
 							      pva, areas,
 							      areas_size)) {
-						contiguous_count++;
+						prefered_count++;
+						goto next_pv;
+					}
+					continue;
+				} else if (cling) {
+					if (prev_lvseg &&
+					    _check_same_pv(prev_lvseg,
+							   pva, areas,
+							   areas_size)) {
+						prefered_count++;
 						goto next_pv;
 					}
 					continue;
@@ -875,7 +904,7 @@ static int _find_parallel_space(struct a
 				break;
 		}
 
-		if (contiguous && (contiguous_count < ix_offset))
+		if ((contiguous || cling) && (prefered_count < ix_offset))
 			break;
 
 		/* Only allocate log_area the first time around */
@@ -985,6 +1014,18 @@ static int _allocate(struct alloc_handle
 		goto finished;
 
 	old_allocated = allocated;
+	if (!_find_parallel_space(ah, ALLOC_CLING, pvms, areas,
+				  areas_size, can_split,
+				  prev_lvseg, &allocated, new_extents)) {
+		stack;
+		goto out;
+	}
+
+	if ((allocated == new_extents) || (ah->alloc == ALLOC_CLING) ||
+	    (!can_split && (allocated != old_allocated)))
+		goto finished;
+
+	old_allocated = allocated;
 	if (!_find_parallel_space(ah, ALLOC_NORMAL, pvms, areas,
 				  areas_size, can_split,
 				  prev_lvseg, &allocated, new_extents)) {
diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/lib/metadata/metadata.h LVM2.06.alloc-cling/lib/metadata/metadata.h
--- LVM2.05.use_for_each_pv/lib/metadata/metadata.h	2006-10-05 19:45:15.000000000 -0400
+++ LVM2.06.alloc-cling/lib/metadata/metadata.h	2006-10-07 02:49:55.000000000 -0400
@@ -82,6 +82,7 @@ typedef enum {
 	ALLOC_INVALID = 0,
 	ALLOC_INHERIT,
 	ALLOC_CONTIGUOUS,
+	ALLOC_CLING,
 	ALLOC_NORMAL,
 	ALLOC_ANYWHERE
 } alloc_policy_t;
diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/lib/report/report.c LVM2.06.alloc-cling/lib/report/report.c
--- LVM2.05.use_for_each_pv/lib/report/report.c	2006-10-05 19:45:16.000000000 -0400
+++ LVM2.06.alloc-cling/lib/report/report.c	2006-10-07 02:49:55.000000000 -0400
@@ -104,6 +104,8 @@ static char _alloc_policy_char(alloc_pol
 	switch (alloc) {
 	case ALLOC_CONTIGUOUS:
 		return 'c';
+	case ALLOC_CLING:
+		return 'C';
 	case ALLOC_NORMAL:
 		return 'n';
 	case ALLOC_ANYWHERE:
diff -X ../dontdiff -urp LVM2.05.use_for_each_pv/man/lvm.8 LVM2.06.alloc-cling/man/lvm.8
--- LVM2.05.use_for_each_pv/man/lvm.8	2006-08-23 04:18:58.000000000 -0400
+++ LVM2.06.alloc-cling/man/lvm.8	2006-10-07 02:49:55.000000000 -0400
@@ -136,7 +136,7 @@ Characters allowed in tags are: A-Z a-z 
 Delete the tag \fBtag\fP from a PV, VG or LV, if it's present.
 .TP
 \fB--alloc AllocationPolicy\fP
-The allocation policy to use: \fBcontiguous\fP, \fBnormal\fP, \fBanywhere\fP or \fBinherit\fP.
+The allocation policy to use: \fBcontiguous\fP, \fBcling\fP, \fBnormal\fP, \fBanywhere\fP or \fBinherit\fP.
 When a command needs to allocate physical extents from the volume group,
 the allocation policy controls how they are chosen.  
 Each volume group and logical volume has an allocation policy.
@@ -151,6 +151,8 @@ existing extents. If there are sufficien
 an allocation request but \fBnormal\fP doesn't use them,
 \fBanywhere\fP will - even if that reduces performance by
 placing two stripes on the same physical volume.
+The \fBcling\fP policy requires that new extents are on the same physical
+volume with existing extents.
 .IP
 N.B. The policies described above are not implemented fully yet.
 In particular, \fBcontiguous\fP does not place new extents adjacent to existing

[linux-lvm] Re: [PATCH LVM2] (0/3) 'cling' allocation policy

[Alasdair G Kergon]

On Fri, Oct 06, 2006 at 05:54:20PM -0400, Jun'ichi Nomura wrote:
>   [lv0_mimage_0] /dev/sdb(0)
>   [lv0_mimage_1] /dev/sdb(2)

> You see /dev/sda is used for both lv0_mimage_0 and lv0_mimage_1.
> If this disk breaks, you lose the whole LV.
 
You still have a full copy of the data on sdb.
It's just that the current implementation isn't clever enough to
piece it back together yet.

Alasdair
-- 
agk@redhat.com

[linux-lvm] Re: [PATCH LVM2] (2/3) use _for_each_pv() from _check_contiguous()

[Alasdair G Kergon]

On Fri, Oct 06, 2006 at 05:56:06PM -0400, Jun'ichi Nomura wrote:
> This patch rewrites _check_contiguous() to use _for_each_pv().
 
I think the existing FIXME got dropped and I suspect there are problems
with multiple levels of recursion.  (h->index++ ?)
 
I'm checking in something based on your patch, but I expect there'll still be
things to fix up.

Alasdair
-- 
agk@redhat.com

[linux-lvm] Re: [PATCH LVM2] (3/3) add 'cling' allocation policy

[Alasdair G Kergon]

On Fri, Oct 06, 2006 at 05:56:54PM -0400, Jun'ichi Nomura wrote:
> This patch adds 'cling' allocation policy.

Applied.
Needed to reset preferred_count at the top of the outer loop.
(Was no need previously as contiguous had exactly one possible
match so was pointless going round the loop again so it didn't.)

> +				} else if (cling) {
> +					if (prev_lvseg &&
> +					    _check_same_pv(prev_lvseg,
> +							   pva, areas,
> +							   areas_size)) {
> +						prefered_count++;
>  						goto next_pv;
>  					}
>  					continue;

goto next_pv here - pointless repeatedly testing the same PV - if
it didn't match once it won't again

Alasdair
-- 
agk@redhat.com

[linux-lvm] Re: [PATCH LVM2] (2/3) use _for_each_pv() from _check_contiguous()

[Jun'ichi Nomura]

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

Alasdair G Kergon wrote:
>> This patch rewrites _check_contiguous() to use _for_each_pv().
>  
> I think the existing FIXME got dropped and I suspect there are problems
> with multiple levels of recursion.  (h->index++ ?)

I think the FIXME was intended for the case of stacked LV.
The index here is used to flatten the stacked LV to linear array
of PVs.
To complete it, we need to use the index in other places, too.

In _allocate(),
         /* Upper bound if none of the PVs in prev_lvseg is in pvms */
         /* FIXME Work size out properly */
         if (prev_lvseg)
                 areas_size += prev_lvseg->area_count;

and in _find_parallel_space(),
         if (prev_lvseg) {
                 ix_offset = prev_lvseg->area_count;

we can count the number of PVs constituting the last extent of
the prev_lvseg correctly by _for_each_pv().

What do you think about the attached patch?

It adds index calculation in _for_each_pv() (I replaced
'top_level_area_index' because it seemed aiming for similar
purpose) and fixes the 2 places above.
If you pass a pointer to counter, _for_each_pv() increments
the counter for each AREA_PV and passes the current number to
call back function. Upon return from the top level _for_each_pv(),
the counter should have the number of PVs in flattend LV.
As long as we use the same set of parameters for _for_each_pv(),
the counter value can be used as consistent index.

I also attached hypothetical metadata of stacked LV
(RAID0+1 and RAID10).

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America

[-- Attachment #2: 07-area_count-calculation-fix.patch --]
[-- Type: text/x-patch, Size: 4327 bytes --]

diff -X dontdiff -urp LVM2/lib/metadata/lv_manip.c LVM2.07.calcfix/lib/metadata/lv_manip.c
--- LVM2/lib/metadata/lv_manip.c	2006-10-09 21:06:56.000000000 -0400
+++ LVM2.07.calcfix/lib/metadata/lv_manip.c	2006-10-09 21:24:59.000000000 -0400
@@ -680,7 +680,7 @@ static int _alloc_parallel_area(struct a
 static int _for_each_pv(struct cmd_context *cmd, struct logical_volume *lv,
 			uint32_t le, uint32_t len, uint32_t *max_seg_len,
 			uint32_t first_area, uint32_t max_areas,
-			int top_level_area_index,
+			uint32_t *flattend_area_index,
 			int only_single_area_segments,
 			int (*fn)(struct cmd_context *cmd,
 				  struct pv_segment *peg, uint32_t s,
@@ -720,13 +720,19 @@ static int _for_each_pv(struct cmd_conte
 					       area_len, max_seg_len,
 					       only_single_area_segments ? 0 : 0,
 					       only_single_area_segments ? 1 : 0,
-					       top_level_area_index != -1 ? top_level_area_index : s,
+					       flattend_area_index,
 					       only_single_area_segments, fn,
 					       data)))
 				stack;
-		} else if (seg_type(seg, s) == AREA_PV)
-			if (!(r = fn(cmd, seg_pvseg(seg, s), top_level_area_index != -1 ? top_level_area_index : s, data)))
+		} else if (seg_type(seg, s) == AREA_PV) {
+			if (fn && !(r = fn(cmd, seg_pvseg(seg, s),
+					   flattend_area_index != NULL ?
+						*flattend_area_index : s,
+					   data)))
 				stack;
+			if (flattend_area_index)
+				(*flattend_area_index)++;
+		}
 		if (r != 1)
 			return r;
 	}
@@ -734,7 +740,7 @@ static int _for_each_pv(struct cmd_conte
 	/* FIXME only_single_area_segments used as workaround to skip log LV - needs new param? */
 	if (!only_single_area_segments && seg_is_mirrored(seg) && seg->log_lv) {
 		if (!(r = _for_each_pv(cmd, seg->log_lv, 0, MIRROR_LOG_SIZE,
-				       NULL, 0, 0, 0, only_single_area_segments,
+				       NULL, 0, 0, NULL, only_single_area_segments,
 				       fn, data)))
 			stack;
 		if (r != 1)
@@ -832,7 +838,7 @@ static int _check_cling(struct cmd_conte
 	/* FIXME Cope with stacks by flattening */
 	if (!(r = _for_each_pv(cmd, prev_lvseg->lv,
 			       prev_lvseg->le + prev_lvseg->len - 1, 1, NULL,
-			       0, 0, -1, 1,
+			       0, 0, NULL, 1,
 			       _is_condition, &pvmatch)))
 		stack;
 
@@ -860,7 +866,7 @@ static int _check_contiguous(struct cmd_
 	/* FIXME Cope with stacks by flattening */
 	if (!(r = _for_each_pv(cmd, prev_lvseg->lv,
 			       prev_lvseg->le + prev_lvseg->len - 1, 1, NULL,
-			       0, 0, -1, 1,
+			       0, 0, NULL, 1,
 			       _is_condition, &pvmatch)))
 		stack;
 
@@ -885,7 +891,7 @@ static int _find_parallel_space(struct a
 	unsigned already_found_one = 0;
 	unsigned contiguous = 0, cling = 0, preferred_count = 0;
 	unsigned ix;
-	unsigned ix_offset = 0;	/* Offset for non-preferred allocations */
+	uint32_t ix_offset = 0;	/* Offset for non-preferred allocations */
 	uint32_t max_parallel;	/* Maximum extents to allocate */
 	uint32_t next_le;
 	struct seg_pvs *spvs;
@@ -896,7 +902,10 @@ static int _find_parallel_space(struct a
 
 	/* Are there any preceding segments we must follow on from? */
 	if (prev_lvseg) {
-		ix_offset = prev_lvseg->area_count;
+		if (!_for_each_pv(NULL, prev_lvseg->lv,
+				  prev_lvseg->le + prev_lvseg->len - 1, 1, NULL,
+				  0, 0, &ix_offset, 1, NULL, NULL))
+			return 0;
 		if ((alloc == ALLOC_CONTIGUOUS))
 			contiguous = 1;
 		else if ((alloc == ALLOC_CLING))
@@ -1092,9 +1101,14 @@ static int _allocate(struct alloc_handle
 	}
 
 	/* Upper bound if none of the PVs in prev_lvseg is in pvms */
-	/* FIXME Work size out properly */
-	if (prev_lvseg)
-		areas_size += prev_lvseg->area_count;
+	if (prev_lvseg) {
+		uint32_t prev_lvseg_areas;
+		if (!_for_each_pv(NULL, prev_lvseg->lv,
+				  prev_lvseg->le + prev_lvseg->len - 1, 1, NULL,
+				  0, 0, &prev_lvseg_areas, 1, NULL, NULL))
+			return 0;
+		areas_size += prev_lvseg_areas;
+	}
 
 	/* Allocate an array of pv_areas to hold the largest space on each PV */
 	if (!(areas = dm_malloc(sizeof(*areas) * areas_size))) {
@@ -1621,7 +1635,7 @@ struct list *build_parallel_areas_from_l
 		/* Find next segment end */
 		/* FIXME Unnecessary nesting! */
 		if (!_for_each_pv(cmd, lv, current_le, spvs->len, &spvs->len,
-				  0, 0, -1, 0, _add_pvs, (void *) spvs)) {
+				  0, 0, NULL, 0, _add_pvs, (void *) spvs)) {
 			stack;
 			return NULL;
 		}

[-- Attachment #3: mirror-stripe.vg --]
[-- Type: text/plain, Size: 4720 bytes --]

# Generated by LVM2: Mon Oct  9 20:30:07 2006

contents = "Text Format Volume Group"
version = 1

description = "Created *before* executing 'lvextend -l+1 mvg2/lvol2'"

creation_host = "tetsuo.lab"	# Linux tetsuo.lab 2.6.9-42.ELsmp #1 SMP Wed Jul 12 23:27:17 EDT 2006 i686
creation_time = 1160440207	# Mon Oct  9 20:30:07 2006

mvg2 {
	id = "AxZG1h-O6iz-YrBC-McFa-N252-NWZL-eLX1qy"
	seqno = 198
	status = ["RESIZEABLE", "READ", "WRITE"]
	extent_size = 8192		# 4 Megabytes
	max_lv = 0
	max_pv = 0

	physical_volumes {

		pv0 {
			id = "9EMez9-jHA3-FKBa-Ubtx-CkuB-WRlK-ZAIKiu"
			device = "/dev/sdc13"	# Hint only

			status = ["ALLOCATABLE"]
			dev_size = 3916160	# 1.86737 Gigabytes
			pe_start = 384
			pe_count = 478	# 1.86719 Gigabytes
		}

		pv1 {
			id = "CyCMk3-IJEW-v7FB-laN3-ZQAp-2Lia-MiA3H3"
			device = "/dev/sdc14"	# Hint only

			status = ["ALLOCATABLE"]
			dev_size = 3916160	# 1.86737 Gigabytes
			pe_start = 384
			pe_count = 478	# 1.86719 Gigabytes
		}
	}

	logical_volumes {

		lvol2 {
			id = "rqnWd1-o0ub-bQIv-x2Rw-o9Z6-KeSo-OAVWnf"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "mirror"
				mirror_count = 2
				region_size = 1024

				mirrors = [
					"lvol2_mimage_0", 0,
					"lvol2_mimage_1", 0
				]
			}
		}

		lvol5 {
			id = "1ADDD2-16NG-HttH-0RN0-3ymb-P9Eg-TJBRjl"
			status = ["READ", "WRITE", "VISIBLE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "striped"
				stripe_count = 2
				stripe_size = 128	# 64 Kilobytes

				stripes = [
					"lvol3", 0,
					"lvol4", 0
				]
			}
		}

		lvol0 {
			id = "1ADDD2-16NG-HttH-0RN0-3ymb-P9Eg-TJBRjs"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "striped"
				stripe_count = 2
				stripe_size = 128	# 64 Kilobytes

				stripes = [
					"pv0", 0,
					"pv1", 0
				]
			}
		}

		lvol1 {
			id = "1ADDD2-16NG-HttH-0RN0-3ymb-P9Eg-TJBRjk"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "striped"
				stripe_count = 2
				stripe_size = 128	# 64 Kilobytes

				stripes = [
					"pv0", 1,
					"pv1", 1
				]
			}
		}

		lvol2_mimage_0 {
			id = "6TZMZW-n0cV-UNT8-w2lk-xMNH-3Ejm-quLPlS"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"lvol0", 0
				]
			}
		}

		lvol2_mimage_1 {
			id = "mBeAfD-xomg-HvnK-ifJY-tzCf-yAcs-IqNKdr"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 2	# 8 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"lvol1", 0
				]
			}
		}

		lvol3 {
			id = "rqnWd1-o0ub-bQIv-x2Rw-o9Z6-KeSo-OAVWng"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "mirror"
				mirror_count = 2
				region_size = 1024

				mirrors = [
					"lvol3_mimage_0", 0,
					"lvol3_mimage_1", 0
				]
			}
		}

		lvol3_mimage_0 {
			id = "6TZMZW-n0cV-UNT8-w2lk-xMNH-3Ejm-quLPlT"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv0", 2
				]
			}
		}

		lvol3_mimage_1 {
			id = "mBeAfD-xomg-HvnK-ifJY-tzCf-yAcs-IqNKds"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv1", 2
				]
			}
		}

		lvol4 {
			id = "rqnWd1-o0ub-bQIv-x2Rw-o9Z6-KeSo-OAVWnh"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "mirror"
				mirror_count = 2
				region_size = 1024

				mirrors = [
					"lvol4_mimage_0", 0,
					"lvol4_mimage_1", 0
				]
			}
		}

		lvol4_mimage_0 {
			id = "6TZMZW-n0cV-UNT8-w2lk-xMNH-3Ejm-quLPlU"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv0", 3
				]
			}
		}

		lvol4_mimage_1 {
			id = "mBeAfD-xomg-HvnK-ifJY-tzCf-yAcs-IqNKdt"
			status = ["READ", "WRITE"]
			segment_count = 1

			segment1 {
				start_extent = 0
				extent_count = 1	# 4 Megabytes

				type = "striped"
				stripe_count = 1	# linear

				stripes = [
					"pv1", 3
				]
			}
		}
	}
}