From 1ce224d13fc198c0996e3f8c40c95d57f16445ed Mon Sep 17 00:00:00 2001 From: Dave Wysochanski Date: Tue, 22 Jan 2008 02:48:53 +0000 Subject: [PATCH] Fix vgsplit - print error if vgcreate option given w/existing vg destination Fix vgsplit - reject split if metadata types or clustered attributes differ Fix vgsplit - remove physicalextentsize option Add vgsplit test cases --- lib/metadata/metadata.c | 14 +++++++ man/vgsplit.8 | 13 +++---- test/t-vgsplit-operation.sh | 73 ++++++++++++++++++++++++++----------- tools/commands.h | 5 +-- tools/vgsplit.c | 17 ++++++++- 5 files changed, 90 insertions(+), 32 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 80239e12a..46933859e 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -1124,6 +1124,20 @@ int vgs_are_compatible(struct cmd_context *cmd, return 0; } + /* Metadata types must be the same */ + if (vg_to->fid->fmt != vg_from->fid->fmt) { + log_error("Metadata types differ for \"%s\" and \"%s\"", + vg_to->name, vg_from->name); + return 0; + } + + /* Clustering attribute must be the same */ + if ((vg_to->status & CLUSTERED) != (vg_from->status & CLUSTERED)) { + log_error("Clustered attribute differs for \"%s\" and \"%s\"", + vg_to->name, vg_from->name); + return 0; + } + /* Check no conflicts with LV names */ list_iterate_items(lvl1, &vg_to->lvs) { name1 = lvl1->lv->name; diff --git a/man/vgsplit.8 b/man/vgsplit.8 index 28a4061fc..8614c7328 100644 --- a/man/vgsplit.8 +++ b/man/vgsplit.8 @@ -15,8 +15,6 @@ vgsplit \- split a volume group into two .IR type ] .RB [ -p | \-\-maxphysicalvolumes .IR MaxPhysicalVolumes ] -.RB [ \-s | \-\-physicalextentsize -.IR PhysicalExtentSize [ \fBkKmMgGtT\fR ]] .RB [ \-t | \-\-test ] .RB [ \-v | \-\-verbose ] SourceVolumeGroupName DestinationVolumeGroupName @@ -35,17 +33,18 @@ If does not exist, a new volume group will be created. The default attributes for the new volume group can be specified with \fB\-\-alloc\fR, \fB\-\-clustered\fR, \fB\-\-maxlogicalvolumes\fR, \fB\-\-metadatatype\fR, -\fB\-\-maxphysicalvolumes\fR, and \fB\-\-physicalextentsize\fR -(see \fBvgcreate(8)\fR for a description of these options). -If any of these options are not given, the default attribute(s) are taken -from +and \fB\-\-maxphysicalvolumes\fR (see \fBvgcreate(8)\fR for a description +of these options). If any of these options are not given, default +attribute(s) are taken from .I SourceVolumeGroupName\fP. If .I DestinationVolumeGroupName does exist, it will be checked for compatibility with .I SourceVolumeGroupName -before the physical volumes are moved. +before the physical volumes are moved. Specifying any of the above default +volume group attributes with an existing destination volume group is an error, +and no split will occur. Logical Volumes cannot be split between Volume Groups. Each existing Logical Volume must be entirely on the Physical Volumes forming diff --git a/test/t-vgsplit-operation.sh b/test/t-vgsplit-operation.sh index 7ec44f1ba..dffa262fb 100755 --- a/test/t-vgsplit-operation.sh +++ b/test/t-vgsplit-operation.sh @@ -23,6 +23,9 @@ cleanup_() rm -f "$f1" "$f2" "$f3" "$f4" } +# FIXME: paramaterize lvm1 vs lvm2 metadata; most of these tests should run +# fine with lvm1 metadata as well; for now, just add disks 5 and 6 as lvm1 +# metadata test_expect_success \ 'set up temp files, loopback devices, PVs, vgnames' \ 'f1=$(pwd)/1 && d1=$(loop_setup_ "$f1") && @@ -48,20 +51,6 @@ test_expect_success \ vgremove $vg1 && vgremove $vg2' -#test_expect_success \ -# 'vgcreate accepts 8.00M physicalextentsize for VG' \ -# 'vgcreate $vg --physicalextentsize 8.00M $d1 $d2 && -# check_vg_field_ $vg vg_extent_size 8.00M && -# vgremove $vg' - -test_expect_success \ - 'vgsplit accepts 8.00M physicalextentsize for new VG' \ - 'vgcreate $vg1 $d1 $d2 && - vgsplit --physicalextentsize 8.00M $vg1 $vg2 $d1 && - check_vg_field_ $vg2 vg_extent_size 8.00M && - vgremove $vg1 && - vgremove $vg2' - test_expect_success \ 'vgsplit accepts --maxphysicalvolumes 128 on new VG' \ 'vgcreate $vg1 $d1 $d2 && @@ -79,22 +68,64 @@ test_expect_success \ vgremove $vg2' test_expect_success \ - 'vgsplit rejects vgs with incompatible extent_size' \ - 'vgcreate --physicalextentsize 4M $vg1 $d1 $d2 && - vgcreate --physicalextentsize 8M $vg2 $d3 $d4 && + 'vgsplit rejects split because max_pv of destination would be exceeded' \ + 'vgcreate --maxphysicalvolumes 2 $vg1 $d1 $d2 && + vgcreate --maxphysicalvolumes 2 $vg2 $d3 $d4 && vgsplit $vg1 $vg2 $d1 2>err; status=$?; echo status=$?; test $status = 5 && - grep "^ Extent sizes differ" err && + grep "^ Maximum number of physical volumes (2) exceeded" err && vgremove $vg2 && vgremove $vg1' test_expect_success \ - 'vgsplit rejects split because max_pv of destination would be exceeded' \ + 'vgsplit rejects split because maxphysicalvolumes given with existing vg' \ 'vgcreate --maxphysicalvolumes 2 $vg1 $d1 $d2 && vgcreate --maxphysicalvolumes 2 $vg2 $d3 $d4 && - vgsplit $vg1 $vg2 $d1 2>err; + vgsplit --maxphysicalvolumes 2 $vg1 $vg2 $d1 2>err; status=$?; echo status=$?; test $status = 5 && - grep "^ Maximum number of physical volumes (2) exceeded" err && + grep "^ Volume group \"$vg2\" exists, but new VG option specified" err && + vgremove $vg2 && + vgremove $vg1' + +test_expect_success \ + 'vgsplit rejects split because maxlogicalvolumes given with existing vg' \ + 'vgcreate --maxlogicalvolumes 2 $vg1 $d1 $d2 && + vgcreate --maxlogicalvolumes 2 $vg2 $d3 $d4 && + vgsplit --maxlogicalvolumes 2 $vg1 $vg2 $d1 2>err; + status=$?; echo status=$?; test $status = 5 && + grep "^ Volume group \"$vg2\" exists, but new VG option specified" err && + vgremove $vg2 && + vgremove $vg1' + +test_expect_success \ + 'vgsplit rejects split because alloc given with existing vg' \ + 'vgcreate --alloc cling $vg1 $d1 $d2 && + vgcreate --alloc cling $vg2 $d3 $d4 && + vgsplit --alloc cling $vg1 $vg2 $d1 2>err; + status=$?; echo status=$?; test $status = 5 && + grep "^ Volume group \"$vg2\" exists, but new VG option specified" err && + vgremove $vg2 && + vgremove $vg1' + +test_expect_success \ + 'vgsplit rejects split because clustered given with existing vg' \ + 'vgcreate --clustered n $vg1 $d1 $d2 && + vgcreate --clustered n $vg2 $d3 $d4 && + vgsplit --clustered n $vg1 $vg2 $d1 2>err; + status=$?; echo status=$?; test $status = 5 && + grep "^ Volume group \"$vg2\" exists, but new VG option specified" err && + vgremove $vg2 && + vgremove $vg1' + +test_expect_success \ + 'vgsplit rejects split because metadata types differ' \ + 'pvcreate -ff -M1 $d3 $d4 && + pvcreate -ff -M2 $d1 $d2 && + vgcreate -M1 $vg1 $d3 $d4 && + vgcreate -M2 $vg2 $d1 $d2 && + vgsplit $vg1 $vg2 $d3 2>err; + status=$?; echo status=$?; test $status = 5 && + grep "^ Metadata types differ" err && vgremove $vg2 && vgremove $vg1' diff --git a/tools/commands.h b/tools/commands.h index c132a6dbf..6b5158290 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -871,7 +871,7 @@ xx(vgscan, ignorelockingfailure_ARG, mknodes_ARG, partial_ARG) xx(vgsplit, - "Move physical volumes into a new volume group", + "Move physical volumes into a new or existing volume group", "vgsplit " "\n" "\t[-A|--autobackup {y|n}] " "\n" "\t[--alloc AllocationPolicy] " "\n" @@ -881,7 +881,6 @@ xx(vgsplit, "\t[-l|--maxlogicalvolumes MaxLogicalVolumes]" "\n" "\t[-M|--metadatatype 1|2] " "\n" "\t[-p|--maxphysicalvolumes MaxPhysicalVolumes] " "\n" - "\t[-s|--physicalextentsize PhysicalExtentSize[kKmMgGtTpPeE]] " "\n" "\t[-t|--test] " "\n" "\t[-v|--verbose] " "\n" "\t[--version]" "\n" @@ -890,7 +889,7 @@ xx(vgsplit, alloc_ARG, autobackup_ARG, clustered_ARG, maxlogicalvolumes_ARG, maxphysicalvolumes_ARG, - metadatatype_ARG, physicalextentsize_ARG, test_ARG) + metadatatype_ARG, test_ARG) xx(version, "Display software and driver version information", diff --git a/tools/vgsplit.c b/tools/vgsplit.c index f78eaf3bd..a30a3f839 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -210,6 +210,17 @@ static int _move_mirrors(struct volume_group *vg_from, return 1; } +/* + * Has the user given an option related to a new vg as the split destination? + */ +static int new_vg_option_specified(struct cmd_context *cmd) +{ + return(arg_count(cmd, clustered_ARG) || + arg_count(cmd, alloc_ARG) || + arg_count(cmd, maxphysicalvolumes_ARG) || + arg_count(cmd, maxlogicalvolumes_ARG)); +} + int vgsplit(struct cmd_context *cmd, int argc, char **argv) { struct vgcreate_params vp_new; @@ -253,7 +264,11 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) if ((vg_to = vg_lock_and_read(cmd, vg_name_to, NULL, LCK_VG_WRITE | LCK_NONBLOCK, 0, 0))) { - log_warn("Volume group \"%s\" already exists", vg_name_to); + if (new_vg_option_specified(cmd)) { + log_error("Volume group \"%s\" exists, but new VG " + "option specified", vg_name_to); + goto error; + } if (!vgs_are_compatible(cmd, vg_from,vg_to)) goto error; } else { -- 2.43.5