[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[lvm-devel] [LVM2 PATCH] (2/5) Fix lvconvert to pass correct parameter to allocate_extents()



This patch fixes lvconvert_mirrors() to pass correct set of
parameters to allocate_extents().

It used to pass (length of LV * number of mirrors) as 'extents'
parameter of allocate_extents().

Though the parameter was wrong, the allocation function somehow
worked because it also passed segtype "striped" instead of "mirror".

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
Fix 'segtype' and 'extents' passed from lvconvert_mirrors to allocator.
(This is a clean-up. The change won't affect the current behaviour.)

Purpose of the change:
  allocate_extents() takes 'segtype' of the LV to which the extents are
  allocated, and 'extents' in terms of LE counts, not PEs.
  (LEs = PEs for linear and striped, but LEs = PEs / n for mirrored)
  Both lv_extend() and tools/lvcreate.c call allocate_extents()
  with the number of LEs.

  OTOH, lvconvert (to mirror) passes:
     - 'segtype' = "striped".
     - 'extents' = PEs (= LEs x mirrors)

  So, current code calls allocate_extents() as if the request is about
  a big linear LV (of length len * n).
  Then, later codes split the allocated extents up to n LVs of length len.
  This is not straightforward way of using the API, not compatible with
  other callers and could cause issues in future changes in the allocation
  code.

Effect of the change:
  It changes 'lp->segtype' from "striped" to "mirror" in case of
  to-mirror conversion.

  'lp->segtype' in lvconvert_mirrors() is used:
    1. to check the existence of segment type support
    2. as an argument to allocate_extents()
    3. as an argument to create_mirror_layers()
  The 1st item is not be a problem.
  The 3rd item is not a problem, either as the callee doesn't use it.

  The 2nd item needs a closer look.
  allocate_extents() uses segtype to check if it's virtual and if the segtype
  is supported or not, that won't be affected by this change.
  allocate_extents() passes segtype to _alloc_init() and _allocate().
  _alloc_init() uses segtype for area_multiple calculation, which will
  return correct value after the segtype change for given stripes/mirrors.
  _allocate() doesn't use segtype.

Index: LVM2.work/tools/lvconvert.c
===================================================================
--- LVM2.work.orig/tools/lvconvert.c
+++ LVM2.work/tools/lvconvert.c
@@ -208,7 +208,7 @@ static int _read_params(struct lvconvert
 			return 0;
 		}
 
-		if (!(lp->segtype = get_segtype_from_string(cmd, "striped")))
+		if (!(lp->segtype = get_segtype_from_string(cmd, "mirror")))
 			return_0;
 	}
 
@@ -236,7 +236,6 @@ static int lvconvert_mirrors(struct cmd_
 	struct alloc_handle *ah = NULL;
 	struct logical_volume *log_lv;
 	struct list *parallel_areas;
-	struct segment_type *segtype;  /* FIXME: could I just use lp->segtype */
 	float sync_percent;
 	const char *mirrorlog;
 	unsigned corelog = 0;
@@ -339,7 +338,7 @@ static int lvconvert_mirrors(struct cmd_
 		if (!(ah = allocate_extents(lv->vg, NULL, lp->segtype,
 					    1, lp->mirrors - 1,
 					    corelog ? 0 : 1,
-					    lv->le_count * (lp->mirrors - 1),
+					    lv->le_count,
 					    NULL, 0, 0, lp->pvh,
 					    lp->alloc,
 					    parallel_areas)))
@@ -386,9 +385,7 @@ static int lvconvert_mirrors(struct cmd_
 				return 0;
 			}
 
-			segtype = get_segtype_from_string(cmd, "striped");
-
-			if (!(ah = allocate_extents(lv->vg, NULL, segtype, 0,
+			if (!(ah = allocate_extents(lv->vg, NULL, lp->segtype, 0,
 						    0, 1, 0,
 						    NULL, 0, 0, lp->pvh,
 						    lp->alloc,

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]