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

Re: [lvm-devel][PATCH] Add escape sequence for ':' in command's PV list



On Wed, 2009-05-13 at 13:09 +0200, Peter Rajnoha wrote:
> Hi,
> 
> this little patch provides an escape sequence in command's PV list
> where ':' is also used as a delimiter for extent ranges. To escape
> this char, we just double it, so '::' is translated to ':' and ':'
> is extent range delimiter.
> (BZ #491409)
> 
> Peter
> 
> 
> diff --git a/tools/toollib.c b/tools/toollib.c
> index aa33468..b5a43b5 100644
> --- a/tools/toollib.c
> +++ b/tools/toollib.c
> @@ -1095,8 +1095,9 @@ struct dm_list *create_pv_list(struct dm_pool *mem, struct volume_group *vg, int
>  	struct dm_list *r;
>  	struct pv_list *pvl;
>  	struct dm_list tags, arg_pvnames;
> -	const char *pvname = NULL;
> +	char *pvname = NULL;
>  	char *colon, *tagname;
> +	char *ptr, *ptr_end;
>  	int i;
>  
>  	/* Build up list of PVs */
> @@ -1128,9 +1129,21 @@ struct dm_list *create_pv_list(struct dm_pool *mem, struct volume_group *vg, int
>  			continue;
>  		}
>  
> -		pvname = argv[i];
> +		ptr = pvname = argv[i];
> +		ptr_end = strchr(ptr, '\0');
> +		colon = NULL;
>  
> -		if ((colon = strchr(pvname, ':'))) {
> +		while ((ptr = strchr(ptr, ':'))) {
> +			if (ptr[1] == ':') {
> +				memmove(ptr, ptr + 1, ptr_end - ptr);
> +				ptr_end--;
> +			}
> +			else if (!colon)
> +				colon = ptr;
> +			ptr++;
> +		}
> +
> +		if (colon) {
>  			if (!(pvname = dm_pool_strndup(mem, pvname,
>  						    (unsigned) (colon -
>  								pvname)))) {

Reviewed and the code works as advertised, however...

This code should probably go inside lvm-string.c with the other code
that handles special characters.
Also, think about consistency among pvcreate, vgcreate, and lvcreate.

Attached is a patch for a small test I put together that I think should
work once this bug is fixed.  It does not work currently because
arguments to pvcreate and vgcreate are handled differently than lvcreate
(it will pass if you change these args to just $dev1 instead of
$dev1_escaped).  I know, it does not make sense to specify a pe range
for pvcreate or vgcreate.  But shouldn't we be consistent, or at least
very much clarify the arguments are different?  I can just imagine
someone scripting something and then being surprised when they needed to
quote only for lvcreate, pvmove, etc.

To see where it fails look at the verbose output of pvcreate or you can
just read the pvcreate code.


>From 033c09639f6abdbfff12b0b7a2c205e089602a5b Mon Sep 17 00:00:00 2001
From: Dave Wysochanski <dwysocha redhat com>
Date: Wed, 13 May 2009 15:21:20 -0400
Subject: [PATCH] Add pvcreate test for escaped ':'.

Test does not currently work but requires consistent handling of ":"
throughout all lvm commands.  Normally the ":" in a pvname indicates
a PE range, so special handling - escaping is required.

This test should work if we choose a "::" as the escaped ":" in the
device name.

Signed-off-by: Dave Wysochanski <dwysocha redhat com>
---
 test/t-pvcreate-names.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
 create mode 100755 test/t-pvcreate-names.sh

diff --git a/test/t-pvcreate-names.sh b/test/t-pvcreate-names.sh
new file mode 100755
index 0000000..dc8546e
--- /dev/null
+++ b/test/t-pvcreate-names.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+# Copyright (C) 2008 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+. ./test-utils.sh
+
+# NOTE: be careful of the filter with the pv name - see regex
+aux prepare_devs 2 128 "colon:pv"
+
+# pvcreate correctly handles escaped ':'
+# to make the escaped name, split $dev1 into 2 sections, before/after colon
+dev1_escaped=`echo $dev1 | awk '{ print substr($1,1,index($1,":")-1)"::"substr($1,index($1,":")+1) }'`
+pvcreate -vvvv $dev1_escaped
+vgcreate $vg $dev1_escaped
+lvcreate -n $lv1 -L 12M $vg $dev1_escaped
+
-- 
1.6.0.6


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