From d38bf3616cc3ec1675894cb012297795897c1b50 Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Fri, 2 Nov 2007 20:40:05 +0000 Subject: [PATCH] Fix orphan-related locking in pvdisplay and pvs. Fix missing VG unlocks in some pvchange error paths. Add some missing validation of VG names. Rename validate_vg_name() to validate_new_vg_name(). Change orphan lock to VG_ORPHANS. Change format1 to use ORPHAN as orphan VG name. --- WHATS_NEW | 8 ++++- lib/format1/disk-rep.c | 6 ++-- lib/format1/import-export.c | 9 +++-- lib/metadata/metadata-exported.h | 2 +- lib/metadata/metadata.c | 8 ++++- lib/metadata/pv_manip.c | 5 ++- lib/metadata/snapshot_manip.c | 3 +- lib/snapshot/snapshot.c | 2 +- tools/lvconvert.c | 2 +- tools/lvcreate.c | 8 ++++- tools/lvresize.c | 6 ++++ tools/pvchange.c | 61 ++++++++++++++++---------------- tools/pvcreate.c | 10 +++--- tools/pvdisplay.c | 16 +++++---- tools/pvremove.c | 11 +++--- tools/reporter.c | 31 ++++++++++++---- tools/toollib.c | 11 +++--- tools/toollib.h | 2 +- tools/vgcfgrestore.c | 8 ++--- tools/vgcreate.c | 12 +++---- tools/vgextend.c | 14 +++----- tools/vgremove.c | 4 +-- tools/vgrename.c | 6 ++-- tools/vgsplit.c | 19 ++++------ 24 files changed, 148 insertions(+), 116 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 57d0fdc96..2d5996718 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,7 +1,13 @@ Version 2.02.29 - ================================== + Fix orphan-related locking in pvdisplay and pvs. + Fix missing VG unlocks in some pvchange error paths. + Add some missing validation of VG names. + Rename validate_vg_name() to validate_new_vg_name(). + Change orphan lock to VG_ORPHANS. + Change format1 to use ORPHAN as orphan VG name. Convert pvchange, pvdisplay, pvscan to use is_orphan() - Add is_orphan_vg() and change all hardcoded checks to use it. + Add is_orphan_vg() and change all hard-coded checks to use it. Detect md superblocks version 1.0, 1.1 and 1.2. Add _alloc_pv() and _free_pv() from _pv_create() code and fix error paths. Add pv_dev_name() to access PV device name. diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c index c2f880dc4..b127c6257 100644 --- a/lib/format1/disk-rep.c +++ b/lib/format1/disk-rep.c @@ -415,19 +415,19 @@ static struct disk_list *__read_disk(const struct format_type *fmt, struct disk_list *read_disk(const struct format_type *fmt, struct device *dev, struct dm_pool *mem, const char *vg_name) { - struct disk_list *r; + struct disk_list *dl; if (!dev_open(dev)) { stack; return NULL; } - r = __read_disk(fmt, dev, mem, vg_name); + dl = __read_disk(fmt, dev, mem, vg_name); if (!dev_close(dev)) stack; - return r; + return dl; } static void _add_pv_to_list(struct list *head, struct disk_list *data) diff --git a/lib/format1/import-export.c b/lib/format1/import-export.c index 7c61d7c52..e6b799264 100644 --- a/lib/format1/import-export.c +++ b/lib/format1/import-export.c @@ -25,6 +25,7 @@ #include "segtype.h" #include "pv_alloc.h" #include "display.h" +#include "lvmcache.h" #include @@ -59,8 +60,10 @@ int import_pv(const struct format_type *fmt, struct dm_pool *mem, memcpy(&pv->id, pvd->pv_uuid, ID_LEN); pv->dev = dev; - if (!(pv->vg_name = dm_pool_strdup(mem, (char *)pvd->vg_name))) { - stack; + if (!*pvd->vg_name) + pv->vg_name = ORPHAN; + else if (!(pv->vg_name = dm_pool_strdup(mem, (char *)pvd->vg_name))) { + log_error("Volume Group name allocation failed."); return 0; } @@ -644,7 +647,7 @@ int import_snapshots(struct dm_pool *mem __attribute((unused)), struct volume_gr continue; /* insert the snapshot */ - if (!vg_add_snapshot(vg, NULL, org, cow, NULL, + if (!vg_add_snapshot(NULL, org, cow, NULL, org->le_count, lvd->lv_chunk_size)) { log_err("Couldn't add snapshot."); diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index d45e3411d..396dc3b5c 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -412,7 +412,7 @@ struct lv_segment *find_cow(const struct logical_volume *lv); /* Given a cow LV, return its origin */ struct logical_volume *origin_from_cow(const struct logical_volume *lv); -int vg_add_snapshot(struct volume_group *vg, const char *name, +int vg_add_snapshot(const char *name, struct logical_volume *origin, struct logical_volume *cow, union lvid *lvid, uint32_t extent_count, uint32_t chunk_size); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 7f61b7119..68844d6f5 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -33,7 +33,7 @@ * FIXME: Check for valid handle before dereferencing field or log error? */ #define pv_field(handle, field) \ - (((struct physical_volume *)(handle))->field) + (((const struct physical_volume *)(handle))->field) static struct physical_volume *_pv_read(struct cmd_context *cmd, const char *pv_name, @@ -1931,6 +1931,12 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name, if (!(misc_flags & CORRECT_INCONSISTENT)) consistent = 0; + if (!validate_name(vg_name)) { + log_error("Volume group name %s has invalid characters", + vg_name); + return NULL; + } + if (!lock_vol(cmd, vg_name, lock_flags)) { log_error("Can't get lock for %s", vg_name); return NULL; diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c index cf2dc2c2b..91df78d1a 100644 --- a/lib/metadata/pv_manip.c +++ b/lib/metadata/pv_manip.c @@ -456,8 +456,7 @@ int pv_resize_single(struct cmd_context *cmd, list_init(&mdas); if (is_orphan_vg(pv_vg_name(pv))) { - vg_name = ORPHAN; - + vg_name = VG_ORPHANS; if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) { log_error("Can't get lock for orphans"); return 0; @@ -583,7 +582,7 @@ int pv_resize_single(struct cmd_context *cmd, unlock_vg(cmd, vg_name); } else { if (!(pv_write(cmd, pv, NULL, INT64_C(-1)))) { - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); log_error("Failed to store physical volume \"%s\"", pv_name); return 0; diff --git a/lib/metadata/snapshot_manip.c b/lib/metadata/snapshot_manip.c index d0a443ef8..9e84a2a05 100644 --- a/lib/metadata/snapshot_manip.c +++ b/lib/metadata/snapshot_manip.c @@ -48,8 +48,7 @@ struct logical_volume *origin_from_cow(const struct logical_volume *lv) return lv->snapshot->origin; } -int vg_add_snapshot(struct volume_group *vg, const char *name, - struct logical_volume *origin, +int vg_add_snapshot(const char *name, struct logical_volume *origin, struct logical_volume *cow, union lvid *lvid, uint32_t extent_count, uint32_t chunk_size) { diff --git a/lib/snapshot/snapshot.c b/lib/snapshot/snapshot.c index 62d2b0930..0e4a532df 100644 --- a/lib/snapshot/snapshot.c +++ b/lib/snapshot/snapshot.c @@ -69,7 +69,7 @@ static int _snap_text_import(struct lv_segment *seg, const struct config_node *s return 0; } - if (!vg_add_snapshot(seg->lv->vg, seg->lv->name, org, cow, + if (!vg_add_snapshot(seg->lv->name, org, cow, &seg->lv->lvid, seg->len, chunk_size)) { stack; return 0; diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 2f3ce1058..2864ea8e3 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -515,7 +515,7 @@ static int lvconvert_snapshot(struct cmd_context *cmd, return 0; } - if (!vg_add_snapshot(lv->vg, NULL, org, lv, NULL, org->le_count, + if (!vg_add_snapshot(NULL, org, lv, NULL, org->le_count, lp->chunk_size)) { log_error("Couldn't create snapshot."); return 0; diff --git a/tools/lvcreate.c b/tools/lvcreate.c index 47d874418..46573b92f 100644 --- a/tools/lvcreate.c +++ b/tools/lvcreate.c @@ -125,6 +125,12 @@ static int _lvcreate_name_params(struct lvcreate_params *lp, } } + if (!validate_name(lp->vg_name)) { + log_error("Volume group name %s has invalid characters", + lp->vg_name); + return 0; + } + if (lp->lv_name) { if ((ptr = strrchr(lp->lv_name, '/'))) lp->lv_name = ptr + 1; @@ -853,7 +859,7 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp) /* cow LV remains active and becomes snapshot LV */ - if (!vg_add_snapshot(vg, NULL, org, lv, NULL, + if (!vg_add_snapshot(NULL, org, lv, NULL, org->le_count, lp->chunk_size)) { log_err("Couldn't create snapshot."); return 0; diff --git a/tools/lvresize.c b/tools/lvresize.c index d2f0037e3..0bd892e8f 100644 --- a/tools/lvresize.c +++ b/tools/lvresize.c @@ -239,6 +239,12 @@ static int _lvresize_params(struct cmd_context *cmd, int argc, char **argv, log_error("Please provide a volume group name"); return 0; } + + if (!validate_name(lp->vg_name)) { + log_error("Volume group name %s has invalid characters", + lp->vg_name); + return NULL; + } if ((st = strrchr(lp->lv_name, '/'))) lp->lv_name = st + 1; diff --git a/tools/pvchange.c b/tools/pvchange.c index 9fc47b030..3fdaed28d 100644 --- a/tools/pvchange.c +++ b/tools/pvchange.c @@ -21,6 +21,7 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, void *handle __attribute((unused))) { struct volume_group *vg = NULL; + const char *vg_name = NULL; struct pv_list *pvl; struct list mdas; uint64_t sector; @@ -56,13 +57,14 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, log_verbose("Finding volume group of physical volume \"%s\"", pv_name); - if (!lock_vol(cmd, pv_vg_name(pv), LCK_VG_WRITE)) { - log_error("Can't get lock for %s", pv_vg_name(pv)); + vg_name = pv_vg_name(pv); + if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) { + log_error("Can't get lock for %s", vg_name); return 0; } - if (!(vg = vg_read(cmd, pv_vg_name(pv), NULL, &consistent))) { - unlock_vg(cmd, pv_vg_name(pv)); + if (!(vg = vg_read(cmd, vg_name, NULL, &consistent))) { + unlock_vg(cmd, vg_name); log_error("Unable to find volume group of \"%s\"", pv_name); return 0; @@ -70,25 +72,25 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, if (!vg_check_status(vg, CLUSTERED | EXPORTED_VG | LVM_WRITE)) { - unlock_vg(cmd, pv_vg_name(pv)); + unlock_vg(cmd, vg_name); return 0; } if (!(pvl = find_pv_in_vg(vg, pv_name))) { - unlock_vg(cmd, pv_vg_name(pv)); + unlock_vg(cmd, vg_name); log_error ("Unable to find \"%s\" in volume group \"%s\"", pv_name, vg->name); return 0; } if (tagarg && !(vg->fid->fmt->features & FMT_TAGS)) { - unlock_vg(cmd, pv_vg_name(pv)); + unlock_vg(cmd, vg_name); log_error("Volume group containing %s does not " "support tags", pv_name); return 0; } if (arg_count(cmd, uuid_ARG) && lvs_in_vg_activated(vg)) { - unlock_vg(cmd, pv_vg_name(pv)); + unlock_vg(cmd, vg_name); log_error("Volume group containing %s has active " "logical volumes", pv_name); return 0; @@ -102,17 +104,19 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, "in volume group", pv_name); return 0; } - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + + vg_name = VG_ORPHANS; + + if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) { log_error("Can't get lock for orphans"); return 0; } if (!(pv = pv_read(cmd, pv_name, &mdas, §or, 1))) { - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, vg_name); log_error("Unable to read PV \"%s\"", pv_name); return 0; } - } if (arg_count(cmd, allocatable_ARG)) { @@ -120,7 +124,7 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, !(pv->fmt->features & FMT_ORPHAN_ALLOCATABLE)) { log_error("Allocatability not supported by orphan " "%s format PV %s", pv->fmt->name, pv_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, vg_name); return 0; } @@ -128,20 +132,14 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, if (allocatable && (pv_status(pv) & ALLOCATABLE_PV)) { log_error("Physical volume \"%s\" is already " "allocatable", pv_name); - if (!is_orphan(pv)) - unlock_vg(cmd, pv_vg_name(pv)); - else - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, vg_name); return 1; } if (!allocatable && !(pv_status(pv) & ALLOCATABLE_PV)) { log_error("Physical volume \"%s\" is already " "unallocatable", pv_name); - if (!is_orphan(pv)) - unlock_vg(cmd, pv_vg_name(pv)); - else - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, vg_name); return 1; } @@ -160,12 +158,14 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, if (!str_list_add(cmd->mem, &pv->tags, tag)) { log_error("Failed to add tag %s to physical " "volume %s", tag, pv_name); + unlock_vg(cmd, vg_name); return 0; } } else { if (!str_list_del(&pv->tags, tag)) { log_error("Failed to remove tag %s from " "physical volume" "%s", tag, pv_name); + unlock_vg(cmd, vg_name); return 0; } } @@ -174,10 +174,12 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, if (!id_create(&pv->id)) { log_error("Failed to generate new random UUID for %s.", pv_name); + unlock_vg(cmd, vg_name); return 0; } if (!id_write_format(&pv->id, uuid, sizeof(uuid))) { stack; + unlock_vg(cmd, vg_name); return 0; } log_verbose("Changing uuid of %s to %s.", pv_name, uuid); @@ -189,6 +191,7 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, if (!(pv_write(cmd, pv, NULL, INT64_C(-1)))) { log_error("pv_write with new uuid failed " "for %s.", pv_name); + unlock_vg(cmd, vg_name); return 0; } pv->vg_name = orig_vg_name; @@ -199,23 +202,21 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, log_verbose("Updating physical volume \"%s\"", pv_name); if (!is_orphan(pv)) { if (!vg_write(vg) || !vg_commit(vg)) { - unlock_vg(cmd, pv_vg_name(pv)); + unlock_vg(cmd, vg_name); log_error("Failed to store physical volume \"%s\" in " "volume group \"%s\"", pv_name, vg->name); return 0; } backup(vg); - unlock_vg(cmd, pv_vg_name(pv)); - } else { - if (!(pv_write(cmd, pv, NULL, INT64_C(-1)))) { - unlock_vg(cmd, ORPHAN); - log_error("Failed to store physical volume \"%s\"", - pv_name); - return 0; - } - unlock_vg(cmd, ORPHAN); + } else if (!(pv_write(cmd, pv, NULL, INT64_C(-1)))) { + unlock_vg(cmd, vg_name); + log_error("Failed to store physical volume \"%s\"", + pv_name); + return 0; } + unlock_vg(cmd, vg_name); + log_print("Physical volume \"%s\" changed", pv_name); return 1; diff --git a/tools/pvcreate.c b/tools/pvcreate.c index 043eb5c13..4220efa88 100644 --- a/tools/pvcreate.c +++ b/tools/pvcreate.c @@ -65,13 +65,13 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name) /* Is there an md superblock here? */ if (!dev && md_filtering()) { - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); persistent_filter_wipe(cmd->filter); lvmcache_destroy(); init_md_filtering(0); - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); init_md_filtering(1); return 0; @@ -171,7 +171,7 @@ static int pvcreate_single(struct cmd_context *cmd, const char *pv_name, extent_count = pv_pe_count(existing_pv); } - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); return ECMD_FAILED; } @@ -254,11 +254,11 @@ static int pvcreate_single(struct cmd_context *cmd, const char *pv_name, log_print("Physical volume \"%s\" successfully created", pv_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_PROCESSED; error: - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } diff --git a/tools/pvdisplay.c b/tools/pvdisplay.c index e77bbce2d..495705942 100644 --- a/tools/pvdisplay.c +++ b/tools/pvdisplay.c @@ -25,15 +25,17 @@ static int _pvdisplay_single(struct cmd_context *cmd, uint64_t size; const char *pv_name = pv_dev_name(pv); + const char *vg_name = NULL; - if (pv_vg_name(pv)) { - if (!lock_vol(cmd, pv_vg_name(pv), LCK_VG_READ)) { - log_error("Can't lock %s: skipping", pv_vg_name(pv)); + if (!is_orphan(pv)) { + vg_name = pv_vg_name(pv); + if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { + log_error("Can't lock %s: skipping", vg_name); return ECMD_FAILED; } - if (!(vg = vg_read(cmd, pv_vg_name(pv), (char *)&pv->vgid, &consistent))) { - log_error("Can't read %s: skipping", pv_vg_name(pv)); + if (!(vg = vg_read(cmd, vg_name, (char *)&pv->vgid, &consistent))) { + log_error("Can't read %s: skipping", vg_name); goto out; } @@ -87,8 +89,8 @@ static int _pvdisplay_single(struct cmd_context *cmd, pvdisplay_segments(pv); out: - if (pv_vg_name(pv)) - unlock_vg(cmd, pv_vg_name(pv)); + if (vg_name) + unlock_vg(cmd, vg_name); return ret; } diff --git a/tools/pvremove.c b/tools/pvremove.c index 30a0e1ad1..5029fa33b 100644 --- a/tools/pvremove.c +++ b/tools/pvremove.c @@ -76,8 +76,9 @@ static int pvremove_single(struct cmd_context *cmd, const char *pv_name, void *handle __attribute((unused))) { struct device *dev; + int ret = ECMD_FAILED; - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); return ECMD_FAILED; } @@ -106,12 +107,12 @@ static int pvremove_single(struct cmd_context *cmd, const char *pv_name, log_print("Labels on physical volume \"%s\" successfully wiped", pv_name); - unlock_vg(cmd, ORPHAN); - return ECMD_PROCESSED; + ret = ECMD_PROCESSED; error: - unlock_vg(cmd, ORPHAN); - return ECMD_FAILED; + unlock_vg(cmd, VG_ORPHANS); + + return ret; } int pvremove(struct cmd_context *cmd, int argc, char **argv) diff --git a/tools/reporter.c b/tools/reporter.c index 2b8e52e7b..d603e91b0 100644 --- a/tools/reporter.c +++ b/tools/reporter.c @@ -103,17 +103,21 @@ static int _pvsegs_single(struct cmd_context *cmd, struct volume_group *vg, static int _pvs_single(struct cmd_context *cmd, struct volume_group *vg, struct physical_volume *pv, void *handle) { + struct pv_list *pvl; int consistent = 0; int ret = ECMD_PROCESSED; + const char *vg_name = NULL; - if (pv_vg_name(pv)) { - if (!lock_vol(cmd, pv_vg_name(pv), LCK_VG_READ)) { - log_error("Can't lock %s: skipping", pv_vg_name(pv)); + if (!is_orphan(pv)) { + vg_name = pv_vg_name(pv); + + if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { + log_error("Can't lock %s: skipping", vg_name); return ECMD_FAILED; } - if (!(vg = vg_read(cmd, pv_vg_name(pv), (char *)&pv->vgid, &consistent))) { - log_error("Can't read %s: skipping", pv_vg_name(pv)); + if (!(vg = vg_read(cmd, vg_name, (char *)&pv->vgid, &consistent))) { + log_error("Can't read %s: skipping", vg_name); goto out; } @@ -121,14 +125,27 @@ static int _pvs_single(struct cmd_context *cmd, struct volume_group *vg, ret = ECMD_FAILED; goto out; } + + /* + * Replace possibly incomplete PV structure with new one + * allocated in vg_read() path. + */ + if (!(pvl = find_pv_in_vg(vg, pv_dev_name(pv)))) { + log_error("Unable to find \"%s\" in volume group \"%s\"", + pv_dev_name(pv), vg->name); + ret = ECMD_FAILED; + goto out; + } + + pv = pvl->pv; } if (!report_object(handle, vg, NULL, pv, NULL, NULL)) ret = ECMD_FAILED; out: - if (pv_vg_name(pv)) - unlock_vg(cmd, pv_vg_name(pv)); + if (vg_name) + unlock_vg(cmd, vg_name); return ret; } diff --git a/tools/toollib.c b/tools/toollib.c index 9b9d94fe9..23c9ce839 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -341,7 +341,7 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, list_iterate_items(strl, vgnames) { vgname = strl->str; - if (!vgname || is_orphan_vg(vgname)) + if (is_orphan_vg(vgname)) continue; /* FIXME Unnecessary? */ if (!lock_vol(cmd, vgname, lock_type)) { log_error("Can't lock %s: skipping", vgname); @@ -474,7 +474,7 @@ static int _process_one_vg(struct cmd_context *cmd, const char *vg_name, int ret = 0; if (!lock_vol(cmd, vg_name, lock_type)) { - log_error("Can't lock %s: skipping", vg_name); + log_error("Can't lock volume group %s: skipping", vg_name); return ret_max; } @@ -588,7 +588,7 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv, } else { list_iterate_items(sl, vgnames) { vg_name = sl->str; - if (!vg_name || is_orphan_vg(vg_name)) + if (is_orphan_vg(vg_name)) continue; /* FIXME Unnecessary? */ ret_max = _process_one_vg(cmd, vg_name, NULL, &tags, &arg_vgnames, @@ -1192,16 +1192,13 @@ int apply_lvname_restrictions(const char *name) return 1; } -int validate_vg_name(struct cmd_context *cmd, const char *vg_name) +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; - if (is_orphan_vg(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); diff --git a/tools/toollib.h b/tools/toollib.h index 899430dab..8ee539b07 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -96,7 +96,7 @@ struct list *clone_pv_list(struct dm_pool *mem, struct list *pvs); int apply_lvname_restrictions(const char *name); -int validate_vg_name(struct cmd_context *cmd, const char *vg_name); +int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name); int generate_log_name_format(struct volume_group *vg, const char *lv_name, char *buffer, size_t size); diff --git a/tools/vgcfgrestore.c b/tools/vgcfgrestore.c index 87e8023c7..a19369e6e 100644 --- a/tools/vgcfgrestore.c +++ b/tools/vgcfgrestore.c @@ -43,14 +43,14 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv) return ECMD_PROCESSED; } - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Unable to lock orphans"); return ECMD_FAILED; } if (!lock_vol(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) { log_error("Unable to lock volume group %s", vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } @@ -59,7 +59,7 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv) arg_str_value(cmd, file_ARG, "")) : backup_restore(cmd, vg_name))) { unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); log_err("Restore failed."); return ECMD_FAILED; } @@ -67,6 +67,6 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv) log_print("Restored volume group %s", vg_name); unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_PROCESSED; } diff --git a/tools/vgcreate.c b/tools/vgcreate.c index bbc7b8489..247b97813 100644 --- a/tools/vgcreate.c +++ b/tools/vgcreate.c @@ -84,7 +84,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; } - if (!validate_vg_name(cmd, vg_name)) { + if (!validate_new_vg_name(cmd, vg_name)) { log_error("New volume group name \"%s\" is invalid", vg_name); return ECMD_FAILED; } @@ -131,32 +131,32 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) else vg->status &= ~CLUSTERED; - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); return ECMD_FAILED; } if (!lock_vol(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) { log_error("Can't get lock for %s", vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } if (!archive(vg)) { unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + 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, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); backup(vg); diff --git a/tools/vgextend.c b/tools/vgextend.c index 45382eb38..25fd0f12f 100644 --- a/tools/vgextend.c +++ b/tools/vgextend.c @@ -35,23 +35,17 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) argc--; argv++; - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); return ECMD_FAILED; } - if (!validate_name(vg_name)) { - log_error("Volume group name \"%s\" is invalid", - vg_name); - return ECMD_FAILED; - } - log_verbose("Checking for volume group \"%s\"", vg_name); if (!(vg = vg_lock_and_read(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK, CLUSTERED | EXPORTED_VG | LVM_WRITE | RESIZEABLE_VG, CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) { - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } /********** FIXME @@ -79,7 +73,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) backup(vg); unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); log_print("Volume group \"%s\" successfully extended", vg_name); @@ -87,6 +81,6 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) error: unlock_vg(cmd, vg_name); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ECMD_FAILED; } diff --git a/tools/vgremove.c b/tools/vgremove.c index ca44e710d..bf1ea2552 100644 --- a/tools/vgremove.c +++ b/tools/vgremove.c @@ -35,7 +35,7 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; } - if (!lock_vol(cmd, ORPHAN, LCK_VG_WRITE)) { + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { log_error("Can't get lock for orphan PVs"); return ECMD_FAILED; } @@ -44,7 +44,7 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv) LCK_VG_WRITE | LCK_NONBLOCK, 1, NULL, &vgremove_single); - unlock_vg(cmd, ORPHAN); + unlock_vg(cmd, VG_ORPHANS); return ret; } diff --git a/tools/vgrename.c b/tools/vgrename.c index f5de9d78b..7438bb5eb 100644 --- a/tools/vgrename.c +++ b/tools/vgrename.c @@ -44,7 +44,7 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path, return 0; } - if (!validate_vg_name(cmd, vg_name_new)) { + if (!validate_new_vg_name(cmd, vg_name_new)) { log_error("New volume group name \"%s\" is invalid", vg_name_new); return 0; @@ -65,8 +65,8 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path, list_iterate_items(sl, vgids) { vgid = sl->str; - if (!vgid || !(vg_name = vgname_from_vgid(NULL, vgid)) - || is_orphan_vg(vg_name)) + if (!vgid || !(vg_name = vgname_from_vgid(NULL, vgid)) || + is_orphan_vg(vg_name)) continue; if (!strcmp(vg_name, vg_name_old)) { if (match) { diff --git a/tools/vgsplit.c b/tools/vgsplit.c index 5422e3404..427967bb0 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -228,12 +228,6 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) argc -= 2; argv += 2; - if (!validate_name(vg_name_from)) { - log_error("Volume group name \"%s\" is invalid", - vg_name_from); - return ECMD_FAILED; - } - if (!strcmp(vg_name_to, vg_name_from)) { log_error("Duplicate volume group name \"%s\"", vg_name_from); return ECMD_FAILED; @@ -253,6 +247,13 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) return ECMD_FAILED; } + if (!validate_new_vg_name(cmd, vg_name_to)) { + log_error("New volume group name \"%s\" is invalid", + vg_name_to); + unlock_vg(cmd, vg_name_from); + return ECMD_FAILED; + } + consistent = 0; if ((vg_to = vg_read(cmd, vg_name_to, NULL, &consistent))) { /* FIXME Remove this restriction */ @@ -260,12 +261,6 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) goto error; } - if (!validate_vg_name(cmd, vg_name_to)) { - log_error("New volume group name \"%s\" is invalid", - vg_name_to); - goto error; - } - if ((active = lvs_in_vg_activated(vg_from))) { /* FIXME Remove this restriction */ log_error("Logical volumes in \"%s\" must be inactive",