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

Re: [lvm-devel] [LVM2 PATCH] Fix lvcreate's checking of the number of PVs



Jun'ichi Nomura wrote:
> While not explained in man page, lvcreate can take tags of PVs
> as a specification of allocatable PVs.
> e.g. if you have 2 MD RAID1 disks and other disks in your vg,
>       you can do:
>        pvchange --addtag raid1 /dev/md[01]
>        lvcreate -L100M -i2 vg @raid1
>       instead of 'lvcreate -L100M -i2 vg /dev/md0 /dev/md1'
> 
> However, lvcreate checks the number of PVs based on the number of
> arguments, so it fails even if the number of PVs for the tag is many
> enough.
> 
> The same check is done later in the allocation code.
> So it's safe to remove the checks here.
> 
> A patch to fix this and a reproducer script is attached.

After the fix, 'argc' is no longer used and should be removed
in both _read_stripe_params() and _read_mirror_params().

This is the updated version of the patch.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
lvcreate can take tags instead of PVs as allocatable PV specificaction.
 e.g. 'lvcreate -L2G vg @fast' will try to allocate from PVs tagged as 'fast'.

However, lvcreate checks the number of PVs by the number of arguments,
which is not correct if more than 1 PVs have the same tag.

It's checked later whether the number of allocatable PVs is enough
for parallel areas of striped/mirrored volume.
So it's safe to remove the tests here.

Index: LVM2.work/tools/lvcreate.c
===================================================================
--- LVM2.work.orig/tools/lvcreate.c
+++ LVM2.work/tools/lvcreate.c
@@ -180,10 +180,8 @@ static int _read_size_params(struct lvcr
  * up to the power of 2) */
 static int _read_stripe_params(struct lvcreate_params *lp,
 			       struct cmd_context *cmd,
-			       int *pargc)
+			       int *pargc __attribute((unused)))
 {
-	int argc = *pargc;
-
 	if (arg_count(cmd, stripesize_ARG)) {
 		if (arg_sign_value(cmd, stripesize_ARG, 0) == SIGN_MINUS) {
 			log_error("Negative stripesize is invalid");
@@ -211,12 +209,6 @@ static int _read_stripe_params(struct lv
 			  display_size(cmd, (uint64_t) lp->stripe_size));
 	}
 
-	if (argc && (unsigned) argc < lp->stripes) {
-		log_error("Too few physical volumes on "
-			  "command line for %d-way striping", lp->stripes);
-		return 0;
-	}
-
 	if (lp->stripes < 1 || lp->stripes > MAX_STRIPES) {
 		log_error("Number of stripes (%d) must be between %d and %d",
 			  lp->stripes, 1, MAX_STRIPES);
@@ -236,19 +228,12 @@ static int _read_stripe_params(struct lv
 
 static int _read_mirror_params(struct lvcreate_params *lp,
 			       struct cmd_context *cmd,
-			       int *pargc)
+			       int *pargc __attribute((unused)))
 {
-	int argc = *pargc;
 	int region_size;
 	int pagesize = lvm_getpagesize();
 	const char *mirrorlog;
 
-	if (argc && (unsigned) argc < lp->mirrors) {
-		log_error("Too few physical volumes on "
-			  "command line for %d-way mirroring", lp->mirrors);
-		return 0;
-	}
-
 	if (arg_count(cmd, regionsize_ARG)) {
 		if (arg_sign_value(cmd, regionsize_ARG, 0) == SIGN_MINUS) {
 			log_error("Negative regionsize is invalid");

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