This is the mail archive of the
lvm2-cvs@sourceware.org
mailing list for the LVM2 project.
LVM2 lib/metadata/metadata-exported.h lib/meta ...
- From: wysochanski at sourceware dot org
- To: lvm-devel at redhat dot com, lvm2-cvs at sourceware dot org
- Date: 15 Jan 2008 22:56:31 -0000
- Subject: LVM2 lib/metadata/metadata-exported.h lib/meta ...
CVSROOT: /cvs/lvm2
Module name: LVM2
Changes by: wysochanski@sourceware.org 2008-01-15 22:56:30
Modified files:
lib/metadata : metadata-exported.h metadata.c
tools : toollib.c toollib.h vgcreate.c vgrename.c
vgsplit.c
Added files:
test : t-vgrename-usage.sh
Log message:
Move more parameter validation into the library.
Update vgrename to call validate_vg_rename_params().
Fix vgcreate and vgsplit default arguments by adding defaults parameter to
fill_vg_create_params().
Add t-vgrename-usage.sh test.
Bugzilla: bz251992
---
tools/toollib.c | 32 ++++++++------------------------
tools/toollib.h | 5 ++---
tools/vgcreate.c | 35 +++++++++++++++++++++--------------
tools/vgrename.c | 35 ++++++-----------------------------
tools/vgsplit.c | 21 ++++++++++++++-------
5 files changed, 51 insertions(+), 77 deletions(-)
Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/lib/metadata/metadata-exported.h.diff?cvsroot=lvm2&r1=1.32&r2=1.33
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/lib/metadata/metadata.c.diff?cvsroot=lvm2&r1=1.147&r2=1.148
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/test/t-vgrename-usage.sh.diff?cvsroot=lvm2&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/toollib.c.diff?cvsroot=lvm2&r1=1.124&r2=1.125
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/toollib.h.diff?cvsroot=lvm2&r1=1.55&r2=1.56
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/vgcreate.c.diff?cvsroot=lvm2&r1=1.54&r2=1.55
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/vgrename.c.diff?cvsroot=lvm2&r1=1.48&r2=1.49
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/vgsplit.c.diff?cvsroot=lvm2&r1=1.44&r2=1.45
--- LVM2/lib/metadata/metadata-exported.h 2008/01/14 21:07:58 1.32
+++ LVM2/lib/metadata/metadata-exported.h 2008/01/15 22:56:30 1.33
@@ -540,4 +540,7 @@
int validate_vg_create_params(struct cmd_context *cmd,
struct vgcreate_params *vp);
+int validate_vg_rename_params(struct cmd_context *cmd,
+ const char *vg_name_old,
+ const char *vg_name_new);
#endif
--- LVM2/lib/metadata/metadata.c 2008/01/14 21:07:58 1.147
+++ LVM2/lib/metadata/metadata.c 2008/01/15 22:56:30 1.148
@@ -18,6 +18,7 @@
#include "metadata.h"
#include "toolcontext.h"
#include "lvm-string.h"
+#include "lvm-file.h"
#include "lvmcache.h"
#include "memlock.h"
#include "str_list.h"
@@ -227,6 +228,54 @@
return 0;
}
+static int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name)
+{
+ char vg_path[PATH_MAX];
+
+ if (!validate_name(vg_name))
+ return 0;
+
+ snprintf(vg_path, PATH_MAX, "%s%s", cmd->dev_dir, vg_name);
+ if (path_exists(vg_path)) {
+ log_error("%s: already exists in filesystem", vg_path);
+ return 0;
+ }
+
+ return 1;
+}
+
+
+int validate_vg_rename_params(struct cmd_context *cmd,
+ const char *vg_name_old,
+ const char *vg_name_new)
+{
+ unsigned length;
+ char *dev_dir;
+
+ dev_dir = cmd->dev_dir;
+ length = strlen(dev_dir);
+
+ /* Check sanity of new name */
+ if (strlen(vg_name_new) > NAME_LEN - length - 2) {
+ log_error("New volume group path exceeds maximum length "
+ "of %d!", NAME_LEN - length - 2);
+ return 0;
+ }
+
+ if (!validate_new_vg_name(cmd, vg_name_new)) {
+ log_error("New volume group name \"%s\" is invalid",
+ vg_name_new);
+ return 0;
+ }
+
+ if (!strcmp(vg_name_old, vg_name_new)) {
+ log_error("Old and new volume group names must differ");
+ return 0;
+ }
+
+ return 1;
+}
+
int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
const char *new_name)
{
/cvs/lvm2/LVM2/test/t-vgrename-usage.sh,v --> standard output
revision 1.1
--- LVM2/test/t-vgrename-usage.sh
+++ - 2008-01-15 22:56:31.402467000 +0000
@@ -0,0 +1,46 @@
+#!/bin/sh
+# Copyright (C) 2007 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_description='Exercise some vgrename diagnostics'
+privileges_required_=1
+
+. ./test-lib.sh
+
+cleanup_()
+{
+ test -n "$d1" && losetup -d "$d1"
+ test -n "$d2" && losetup -d "$d2"
+ test -n "$d3" && losetup -d "$d3"
+ test -n "$d4" && losetup -d "$d4"
+ rm -f "$f1" "$f2" "$f3" "$f4"
+}
+
+test_expect_success \
+ 'set up temp files, loopback devices, PVs, vgnames' \
+ 'f1=$(pwd)/1 && d1=$(loop_setup_ "$f1") &&
+ f2=$(pwd)/2 && d2=$(loop_setup_ "$f2") &&
+ f3=$(pwd)/3 && d3=$(loop_setup_ "$f3") &&
+ f4=$(pwd)/4 && d4=$(loop_setup_ "$f4") &&
+ vg1=$(this_test_)-1-$$ &&
+ vg2=$(this_test_)-2-$$ &&
+ pvcreate $d1 $d2 $d3 $d4'
+
+test_expect_success \
+ 'vgrename normal operation - rename vg1 to vg2' \
+ 'vgcreate $vg1 $d1 $d2 &&
+ vgrename $vg1 $vg2 &&
+ check_vg_field_ $vg2 vg_name $vg2 &&
+ vgremove $vg2'
+
+test_done
+# Local Variables:
+# indent-tabs-mode: nil
+# End:
--- LVM2/tools/toollib.c 2008/01/14 21:07:58 1.124
+++ LVM2/tools/toollib.c 2008/01/15 22:56:30 1.125
@@ -1218,23 +1218,6 @@
return 1;
}
-int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name)
-{
- char vg_path[PATH_MAX];
-
- if (!validate_name(vg_name))
- return 0;
-
- snprintf(vg_path, PATH_MAX, "%s%s", cmd->dev_dir, vg_name);
- if (path_exists(vg_path)) {
- log_error("%s: already exists in filesystem", vg_path);
- return 0;
- }
-
- return 1;
-}
-
-
/*
* Set members of struct vgcreate_params from cmdline.
* Do preliminary validation with arg_*() interface.
@@ -1243,23 +1226,27 @@
* validate_vgcreate_params() will be moved into the LVM library.
*/
int fill_vg_create_params(struct cmd_context *cmd,
- char *vg_name, struct vgcreate_params *vp)
+ char *vg_name, struct vgcreate_params *vp_new,
+ struct vgcreate_params *vp_def)
{
- vp->vg_name = skip_dev_dir(cmd, vg_name, NULL);
- vp->max_lv = arg_uint_value(cmd, maxlogicalvolumes_ARG, 0);
- vp->max_pv = arg_uint_value(cmd, maxphysicalvolumes_ARG, 0);
- vp->alloc = arg_uint_value(cmd, alloc_ARG, ALLOC_NORMAL);
+ vp_new->vg_name = skip_dev_dir(cmd, vg_name, NULL);
+ vp_new->max_lv = arg_uint_value(cmd, maxlogicalvolumes_ARG,
+ vp_def->max_lv);
+ vp_new->max_pv = arg_uint_value(cmd, maxphysicalvolumes_ARG,
+ vp_def->max_pv);
+ vp_new->alloc = arg_uint_value(cmd, alloc_ARG, vp_def->alloc);
/* Units of 512-byte sectors */
- vp->extent_size =
- arg_uint_value(cmd, physicalextentsize_ARG, DEFAULT_PE_SIZE);
+ vp_new->extent_size =
+ arg_uint_value(cmd, physicalextentsize_ARG, vp_def->extent_size);
if (arg_count(cmd, clustered_ARG))
- vp->clustered =
- !strcmp(arg_str_value(cmd, clustered_ARG, "n"), "y");
+ vp_new->clustered =
+ !strcmp(arg_str_value(cmd, clustered_ARG,
+ vp_def->clustered ? "y":"n"), "y");
else
/* Default depends on current locking type */
- vp->clustered = locking_is_clustered();
+ vp_new->clustered = locking_is_clustered();
if (arg_sign_value(cmd, physicalextentsize_ARG, 0) == SIGN_MINUS) {
log_error("Physical extent size may not be negative");
@@ -1278,4 +1265,3 @@
return 0;
}
-
--- LVM2/tools/toollib.h 2008/01/14 21:07:58 1.55
+++ LVM2/tools/toollib.h 2008/01/15 22:56:30 1.56
@@ -96,8 +96,7 @@
int apply_lvname_restrictions(const char *name);
-int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name);
-
int fill_vg_create_params(struct cmd_context *cmd,
- char *vg_name, struct vgcreate_params *vp);
+ char *vg_name, struct vgcreate_params *vp_new,
+ struct vgcreate_params *vp_def);
#endif
--- LVM2/tools/vgcreate.c 2008/01/14 21:07:58 1.54
+++ LVM2/tools/vgcreate.c 2008/01/15 22:56:30 1.55
@@ -17,7 +17,8 @@
int vgcreate(struct cmd_context *cmd, int argc, char **argv)
{
- struct vgcreate_params vp;
+ struct vgcreate_params vp_new;
+ struct vgcreate_params vp_def;
struct volume_group *vg;
const char *tag;
@@ -32,23 +33,29 @@
return EINVALID_CMD_LINE;
}
- if (fill_vg_create_params(cmd, argv[0], &vp))
+ vp_def.vg_name = NULL;
+ vp_def.extent_size = DEFAULT_PE_SIZE;
+ vp_def.max_pv = 0;
+ vp_def.max_lv = 0;
+ vp_def.alloc = ALLOC_NORMAL;
+ vp_def.clustered = 0;
+ if (fill_vg_create_params(cmd, argv[0], &vp_new, &vp_def))
return EINVALID_CMD_LINE;
- if (validate_vg_create_params(cmd, &vp))
+ if (validate_vg_create_params(cmd, &vp_new))
return EINVALID_CMD_LINE;
/* Create the new VG */
- if (!(vg = vg_create(cmd, vp.vg_name, vp.extent_size, vp.max_pv,
- vp.max_lv, vp.alloc,
+ if (!(vg = vg_create(cmd, vp_new.vg_name, vp_new.extent_size,
+ vp_new.max_pv, vp_new.max_lv, vp_new.alloc,
argc - 1, argv + 1)))
return ECMD_FAILED;
- if (vp.max_lv != vg->max_lv)
+ if (vp_new.max_lv != vg->max_lv)
log_warn("WARNING: Setting maxlogicalvolumes to %d "
"(0 means unlimited)", vg->max_lv);
- if (vp.max_pv != vg->max_pv)
+ if (vp_new.max_pv != vg->max_pv)
log_warn("WARNING: Setting maxphysicalvolumes to %d "
"(0 means unlimited)", vg->max_pv);
@@ -65,13 +72,13 @@
if (!str_list_add(cmd->mem, &vg->tags, tag)) {
log_error("Failed to add tag %s to volume group %s",
- tag, vp.vg_name);
+ tag, vp_new.vg_name);
return ECMD_FAILED;
}
}
/* FIXME: move this inside vg_create? */
- if (vp.clustered)
+ if (vp_new.clustered)
vg->status |= CLUSTERED;
else
vg->status &= ~CLUSTERED;
@@ -81,26 +88,26 @@
return ECMD_FAILED;
}
- if (!lock_vol(cmd, vp.vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) {
- log_error("Can't get lock for %s", vp.vg_name);
+ if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ log_error("Can't get lock for %s", vp_new.vg_name);
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
}
if (!archive(vg)) {
- unlock_vg(cmd, vp.vg_name);
+ unlock_vg(cmd, vp_new.vg_name);
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
}
/* Store VG on disk(s) */
if (!vg_write(vg) || !vg_commit(vg)) {
- unlock_vg(cmd, vp.vg_name);
+ unlock_vg(cmd, vp_new.vg_name);
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
}
- unlock_vg(cmd, vp.vg_name);
+ unlock_vg(cmd, vp_new.vg_name);
unlock_vg(cmd, VG_ORPHANS);
backup(vg);
--- LVM2/tools/vgrename.c 2007/11/02 20:40:05 1.48
+++ LVM2/tools/vgrename.c 2008/01/15 22:56:30 1.49
@@ -19,7 +19,6 @@
const char *new_vg_path)
{
char *dev_dir;
- unsigned length;
struct id id;
int consistent = 1;
int match = 0;
@@ -35,25 +34,9 @@
vg_name_new = skip_dev_dir(cmd, new_vg_path, NULL);
dev_dir = cmd->dev_dir;
- length = strlen(dev_dir);
- /* Check sanity of new name */
- if (strlen(vg_name_new) > NAME_LEN - length - 2) {
- log_error("New volume group path exceeds maximum length "
- "of %d!", NAME_LEN - length - 2);
+ if (!validate_vg_rename_params(cmd, vg_name_old, vg_name_new))
return 0;
- }
-
- if (!validate_new_vg_name(cmd, vg_name_new)) {
- log_error("New volume group name \"%s\" is invalid",
- vg_name_new);
- return 0;
- }
-
- if (!strcmp(vg_name_old, vg_name_new)) {
- log_error("Old and new volume group names must differ");
- return 0;
- }
log_verbose("Checking for existing volume group \"%s\"", vg_name_old);
--- LVM2/tools/vgsplit.c 2008/01/14 21:07:58 1.44
+++ LVM2/tools/vgsplit.c 2008/01/15 22:56:30 1.45
@@ -212,7 +212,8 @@
int vgsplit(struct cmd_context *cmd, int argc, char **argv)
{
- struct vgcreate_params vp;
+ struct vgcreate_params vp_new;
+ struct vgcreate_params vp_def;
char *vg_name_from, *vg_name_to;
struct volume_group *vg_to, *vg_from;
int opt;
@@ -260,18 +261,24 @@
/* FIXME: need some common logic */
cmd->fmt = vg_from->fid->fmt;
- /* FIXME: original code took defaults from vg_from */
- if (fill_vg_create_params(cmd, vg_name_to, &vp))
+ vp_def.vg_name = NULL;
+ vp_def.extent_size = vg_from->extent_size;
+ vp_def.max_pv = vg_from->max_pv;
+ vp_def.max_lv = vg_from->max_lv;
+ vp_def.alloc = vg_from->alloc;
+ vp_def.clustered = 0;
+
+ if (fill_vg_create_params(cmd, vg_name_to, &vp_new, &vp_def))
return EINVALID_CMD_LINE;
- if (validate_vg_create_params(cmd, &vp))
+ if (validate_vg_create_params(cmd, &vp_new))
return EINVALID_CMD_LINE;
/* Create new VG structure */
/* FIXME: allow user input same params as to vgcreate tool */
- if (!(vg_to = vg_create(cmd, vg_name_to, vp.extent_size,
- vp.max_pv, vp.max_lv,
- vp.alloc, 0, NULL)))
+ if (!(vg_to = vg_create(cmd, vg_name_to, vp_new.extent_size,
+ vp_new.max_pv, vp_new.max_lv,
+ vp_new.alloc, 0, NULL)))
goto error;
if (vg_from->status & CLUSTERED)