From b8daca857033804311896d7acf647f6cd94d2d44 Mon Sep 17 00:00:00 2001 From: Dave Wysochanski Date: Mon, 14 Jan 2008 21:07:58 +0000 Subject: [PATCH] Allow vgcreate options as input to vgsplit when new vg is split destination. --- lib/config/defaults.h | 2 + lib/metadata/metadata-exported.h | 12 ++++ lib/metadata/metadata.c | 39 +++++++++++++ man/vgsplit.8 | 24 +++++--- test/lvm-utils.sh | 5 ++ test/t-vgcreate-usage.sh | 28 +++++++-- test/t-vgsplit-operation.sh | 30 ++++++++++ tools/commands.h | 11 +++- tools/toollib.c | 46 +++++++++++++++ tools/toollib.h | 2 + tools/vgcreate.c | 99 +++++--------------------------- tools/vgsplit.c | 14 ++++- 12 files changed, 211 insertions(+), 101 deletions(-) diff --git a/lib/config/defaults.h b/lib/config/defaults.h index 2c2428c70..75fa3dbd0 100644 --- a/lib/config/defaults.h +++ b/lib/config/defaults.h @@ -64,6 +64,8 @@ #define DEFAULT_PVMETADATACOPIES 1 #define DEFAULT_LABELSECTOR UINT64_C(1) #define DEFAULT_READ_AHEAD "auto" +#define DEFAULT_PE_SIZE 4096 /* In KB */ + #define DEFAULT_MSG_PREFIX " " #define DEFAULT_CMD_NAME 0 diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 41b7d0b92..d007a04ea 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -528,4 +528,16 @@ uint32_t pv_pe_alloc_count(const pv_t *pv); uint32_t vg_status(const vg_t *vg); +struct vgcreate_params { + char *vg_name; + uint32_t extent_size; + size_t max_pv; + size_t max_lv; + alloc_policy_t alloc; + int clustered; /* FIXME: put this into a 'status' variable instead? */ +}; + +int validate_vg_create_params(struct cmd_context *cmd, + struct vgcreate_params *vp); + #endif diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 1d3a0bc10..751634c37 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -378,6 +378,45 @@ const char *strip_dir(const char *vg_name, const char *dev_dir) return vg_name; } +/* + * Validate parameters to vg_create() before calling. + * FIXME: move this inside the library, maybe inside vg_create + * - TODO: resolve error codes + */ +int validate_vg_create_params(struct cmd_context *cmd, + struct vgcreate_params *vp) +{ + if (!validate_new_vg_name(cmd, vp->vg_name)) { + log_error("New volume group name \"%s\" is invalid", + vp->vg_name); + return 1; + } + + if (vp->alloc == ALLOC_INHERIT) { + log_error("Volume Group allocation policy cannot inherit " + "from anything"); + return 1; + } + + if (!vp->extent_size) { + log_error("Physical extent size may not be zero"); + return 1; + } + + if (!(cmd->fmt->features & FMT_UNLIMITED_VOLS)) { + if (!vp->max_lv) + vp->max_lv = 255; + if (!vp->max_pv) + vp->max_pv = 255; + if (vp->max_lv > 255 || vp->max_pv > 255) { + log_error("Number of volumes may not exceed 255"); + return 1; + } + } + + return 0; +} + struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name, uint32_t extent_size, uint32_t max_pv, uint32_t max_lv, alloc_policy_t alloc, diff --git a/man/vgsplit.8 b/man/vgsplit.8 index b3588d425..ec935fdac 100644 --- a/man/vgsplit.8 +++ b/man/vgsplit.8 @@ -3,13 +3,23 @@ vgsplit \- split a volume group into two .SH SYNOPSIS .B vgsplit -[\-A/\-\-autobackup y/n] -[\-d/\-\-debug] -[\-h/\-?/\-\-help] -[\-l/\-\-list] -[\-M/\-\-metadatatype 1/2] -[\-t/\-\-test] -[\-v/\-\-verbose] +.RB [ \-\-alloc +.IR AllocationPolicy ] +.RB [ \-A | \-\-autobackup " {" y | n }] +.RB [ \-c | \-\-clustered " {" y | n }] +.RB [ \-d | \-\-debug ] +.RB [ \-h | \-\-help ] +.RB [ \-l | \-\-list ] +.RB [ \-\-maxlogicalvolumes +.IR MaxLogicalVolumes ] +.RB [ -M | \-\-metadatatype +.IR type ] +.RB [ -p | \-\-maxphysicalvolumes +.IR MaxPhysicalVolumes ] +.RB [ \-s | \-\-physicalextentsize +.IR PhysicalExtentSize [ \fBkKmMgGtT\fR ]] +.RB [ \-t | \-\-test ] +.RB [ \-v | \-\-verbose ] SourceVolumeGroupName DestinationVolumeGroupName PhysicalVolumePath [PhysicalVolumePath...] .SH DESCRIPTION diff --git a/test/lvm-utils.sh b/test/lvm-utils.sh index 5c827fcd4..fec6453fe 100644 --- a/test/lvm-utils.sh +++ b/test/lvm-utils.sh @@ -50,6 +50,11 @@ loop_setup_() return 0; } +check_vg_field_() +{ + return $(test $(vgs --noheadings -o $2 $1) == $3) +} + check_pv_size_() { return $(test $(pvs --noheadings -o pv_free $1) == $2) diff --git a/test/t-vgcreate-usage.sh b/test/t-vgcreate-usage.sh index bbfae113e..735da3992 100755 --- a/test/t-vgcreate-usage.sh +++ b/test/t-vgcreate-usage.sh @@ -30,6 +30,24 @@ test_expect_success \ lv=vgcreate-usage-$$ +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 \ + 'vgcreate accepts smaller (128) maxlogicalvolumes for VG' \ + 'vgcreate $vg --maxlogicalvolumes 128 $d1 $d2 && + check_vg_field_ $vg max_lv 128 && + vgremove $vg' + +test_expect_success \ + 'vgcreate accepts smaller (128) maxphysicalvolumes for VG' \ + 'vgcreate $vg --maxphysicalvolumes 128 $d1 $d2 && + check_vg_field_ $vg max_pv 128 && + vgremove $vg' + test_expect_success \ 'vgcreate rejects a zero physical extent size' \ 'vgcreate --physicalextentsize 0 $vg $d1 $d2 2>err; @@ -50,14 +68,14 @@ test_expect_success \ test_expect_success \ 'vgcreate rejects vgname greater than 128 characters' \ - 'vg=thisnameisridiculouslylongtotestvalidationcodecheckingmaximumsizethisiswhathappenswhenprogrammersgetboredandorarenotcreativedonttrythisathome; - vgcreate $vg $d1 $d2 2>err; + 'vginvalid=thisnameisridiculouslylongtotestvalidationcodecheckingmaximumsizethisiswhathappenswhenprogrammersgetboredandorarenotcreativedonttrythisathome; + vgcreate $vginvalid $d1 $d2 2>err; status=$?; echo status=$?; test $status = 3 && - grep "New volume group name \"$vg\" is invalid\$" err' + grep "New volume group name \"$vginvalid\" is invalid\$" err' test_expect_success \ - 'vgcreate rejects already existing vgname "/dev/fd0"' \ - 'vg=/dev/fd0; vgcreate $vg $d1 $d2 2>err; + 'vgcreate rejects already existing vgname "/tmp/$vg"' \ + 'touch /tmp/$vg; vgcreate $vg $d1 $d2 2>err; status=$?; echo status=$?; test $status = 3 && grep "New volume group name \"$vg\" is invalid\$" err' diff --git a/test/t-vgsplit-operation.sh b/test/t-vgsplit-operation.sh index 19bbce51a..d4a08587f 100755 --- a/test/t-vgsplit-operation.sh +++ b/test/t-vgsplit-operation.sh @@ -48,6 +48,36 @@ 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 && + vgsplit --maxphysicalvolumes 128 $vg1 $vg2 $d1 && + check_vg_field_ $vg2 max_pv 128 && + vgremove $vg1 && + vgremove $vg2' + +test_expect_success \ + 'vgsplit accepts --maxlogicalvolumes 128 on new VG' \ + 'vgcreate $vg1 $d1 $d2 && + vgsplit --maxlogicalvolumes 128 $vg1 $vg2 $d1 && + check_vg_field_ $vg2 max_lv 128 && + vgremove $vg1 && + vgremove $vg2' + test_done # Local Variables: # indent-tabs-mode: nil diff --git a/tools/commands.h b/tools/commands.h index 0e610fde1..7a13270a9 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -874,17 +874,24 @@ xx(vgsplit, "Move physical volumes into a new volume group", "vgsplit " "\n" "\t[-A|--autobackup {y|n}] " "\n" + "\t[--alloc AllocationPolicy] " "\n" + "\t[-c|--clustered {y|n}] " "\n" "\t[-d|--debug] " "\n" "\t[-h|--help] " "\n" "\t[-l|--list]" "\n" + "\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" - "\tExistingVolumeGroupName NewVolumeGroupName" "\n" + "\tSourceVolumeGroupName DestinationVolumeGroupName" "\n" "\tPhysicalVolumePath [PhysicalVolumePath...]\n", - autobackup_ARG, list_ARG, metadatatype_ARG, test_ARG) + alloc_ARG, autobackup_ARG, clustered_ARG, list_ARG, + maxlogicalvolumes_ARG, maxphysicalvolumes_ARG, + metadatatype_ARG, physicalextentsize_ARG, test_ARG) xx(version, "Display software and driver version information", diff --git a/tools/toollib.c b/tools/toollib.c index 612d829bc..e167484ad 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1233,3 +1233,49 @@ int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name) return 1; } + + +/* + * Set members of struct vgcreate_params from cmdline. + * Do preliminary validation with arg_*() interface. + * Further, more generic validation is done in validate_vgcreate_params(). + * This function is to remain in tools directory, while + * 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) +{ + 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); + + /* Units of 512-byte sectors */ + vp->extent_size = + arg_uint_value(cmd, physicalextentsize_ARG, DEFAULT_PE_SIZE); + + if (arg_count(cmd, clustered_ARG)) + vp->clustered = + !strcmp(arg_str_value(cmd, clustered_ARG, "n"), "y"); + else + /* Default depends on current locking type */ + vp->clustered = locking_is_clustered(); + + if (arg_sign_value(cmd, physicalextentsize_ARG, 0) == SIGN_MINUS) { + log_error("Physical extent size may not be negative"); + return 1; + } + + if (arg_sign_value(cmd, maxlogicalvolumes_ARG, 0) == SIGN_MINUS) { + log_error("Max Logical Volumes may not be negative"); + return 1; + } + + if (arg_sign_value(cmd, maxphysicalvolumes_ARG, 0) == SIGN_MINUS) { + log_error("Max Physical Volumes may not be negative"); + return 1; + } + + return 0; +} + diff --git a/tools/toollib.h b/tools/toollib.h index 57dde35ab..b8830d228 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -98,4 +98,6 @@ 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); #endif diff --git a/tools/vgcreate.c b/tools/vgcreate.c index d2e257232..63cbc4434 100644 --- a/tools/vgcreate.c +++ b/tools/vgcreate.c @@ -15,54 +15,11 @@ #include "tools.h" -#define DEFAULT_EXTENT 4096 /* In KB */ - -static int validate_vg_create_params(struct cmd_context *cmd, - const char *vg_name, - const uint32_t extent_size, - size_t *max_pv, - size_t *max_lv, - const alloc_policy_t alloc) -{ - if (!validate_new_vg_name(cmd, vg_name)) { - log_error("New volume group name \"%s\" is invalid", vg_name); - return 0; - } - - if (alloc == ALLOC_INHERIT) { - log_error("Volume Group allocation policy cannot inherit " - "from anything"); - return 0; - } - - if (!extent_size) { - log_error("Physical extent size may not be zero"); - return 0; - } - - if (!(cmd->fmt->features & FMT_UNLIMITED_VOLS)) { - if (!*max_lv) - *max_lv = 255; - if (!*max_pv) - *max_pv = 255; - if (*max_lv > 255 || *max_pv > 255) { - log_error("Number of volumes may not exceed 255"); - return 0; - } - } - - return 1; -} - int vgcreate(struct cmd_context *cmd, int argc, char **argv) { - size_t max_lv, max_pv; - uint32_t extent_size; - char *vg_name; + struct vgcreate_params vp; struct volume_group *vg; const char *tag; - alloc_policy_t alloc; - int clustered; if (!argc) { log_error("Please provide volume group name and " @@ -75,44 +32,23 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; } - vg_name = skip_dev_dir(cmd, argv[0], NULL); - max_lv = arg_uint_value(cmd, maxlogicalvolumes_ARG, 0); - max_pv = arg_uint_value(cmd, maxphysicalvolumes_ARG, 0); - alloc = arg_uint_value(cmd, alloc_ARG, ALLOC_NORMAL); - - if (arg_sign_value(cmd, physicalextentsize_ARG, 0) == SIGN_MINUS) { - log_error("Physical extent size may not be negative"); + if (fill_vg_create_params(cmd, argv[0], &vp)) return EINVALID_CMD_LINE; - } - if (arg_sign_value(cmd, maxlogicalvolumes_ARG, 0) == SIGN_MINUS) { - log_error("Max Logical Volumes may not be negative"); - return EINVALID_CMD_LINE; - } - - if (arg_sign_value(cmd, maxphysicalvolumes_ARG, 0) == SIGN_MINUS) { - log_error("Max Physical Volumes may not be negative"); - return EINVALID_CMD_LINE; - } - - /* Units of 512-byte sectors */ - extent_size = - arg_uint_value(cmd, physicalextentsize_ARG, DEFAULT_EXTENT); - - if (!validate_vg_create_params(cmd, vg_name, extent_size, - &max_pv, &max_lv, alloc)) + if (validate_vg_create_params(cmd, &vp)) return EINVALID_CMD_LINE; /* Create the new VG */ - if (!(vg = vg_create(cmd, vg_name, extent_size, max_pv, max_lv, alloc, + if (!(vg = vg_create(cmd, vp.vg_name, vp.extent_size, vp.max_pv, + vp.max_lv, vp.alloc, argc - 1, argv + 1))) return ECMD_FAILED; - if (max_lv != vg->max_lv) + if (vp.max_lv != vg->max_lv) log_warn("WARNING: Setting maxlogicalvolumes to %d " "(0 means unlimited)", vg->max_lv); - if (max_pv != vg->max_pv) + if (vp.max_pv != vg->max_pv) log_warn("WARNING: Setting maxphysicalvolumes to %d " "(0 means unlimited)", vg->max_pv); @@ -129,18 +65,13 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) if (!str_list_add(cmd->mem, &vg->tags, tag)) { log_error("Failed to add tag %s to volume group %s", - tag, vg_name); + tag, vp.vg_name); return ECMD_FAILED; } } - if (arg_count(cmd, clustered_ARG)) - clustered = !strcmp(arg_str_value(cmd, clustered_ARG, "n"), "y"); - else - /* Default depends on current locking type */ - clustered = locking_is_clustered(); - - if (clustered) + /* FIXME: move this inside vg_create? */ + if (vp.clustered) vg->status |= CLUSTERED; else vg->status &= ~CLUSTERED; @@ -150,26 +81,26 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) return ECMD_FAILED; } - if (!lock_vol(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) { - log_error("Can't get lock for %s", vg_name); + if (!lock_vol(cmd, vp.vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) { + log_error("Can't get lock for %s", vp.vg_name); unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } if (!archive(vg)) { - unlock_vg(cmd, vg_name); + unlock_vg(cmd, vp.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, vg_name); + unlock_vg(cmd, vp.vg_name); unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } - unlock_vg(cmd, vg_name); + unlock_vg(cmd, vp.vg_name); unlock_vg(cmd, VG_ORPHANS); backup(vg); diff --git a/tools/vgsplit.c b/tools/vgsplit.c index 3f7796972..9ab0381c1 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -212,6 +212,7 @@ static int _move_mirrors(struct volume_group *vg_from, int vgsplit(struct cmd_context *cmd, int argc, char **argv) { + struct vgcreate_params vp; char *vg_name_from, *vg_name_to; struct volume_group *vg_to, *vg_from; int opt; @@ -259,11 +260,18 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) /* 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)) + return EINVALID_CMD_LINE; + + if (validate_vg_create_params(cmd, &vp)) + 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, vg_from->extent_size, - vg_from->max_pv, vg_from->max_lv, - vg_from->alloc, 0, NULL))) + if (!(vg_to = vg_create(cmd, vg_name_to, vp.extent_size, + vp.max_pv, vp.max_lv, + vp.alloc, 0, NULL))) goto error; if (vg_from->status & CLUSTERED)