diff --git a/WHATS_NEW b/WHATS_NEW index 47a74eb78..c3dcd819e 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.29 - ================================== + Convert some vg_reads into vg_lock_and_reads. Avoid nested vg_reads when processing PVs in VGs and fix associated locking. Accept sizes with --readahead argument. Store size arguments as sectors internally. diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 909b7f899..9185d551a 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -308,6 +308,7 @@ int pv_write(struct cmd_context *cmd, struct physical_volume *pv, int is_orphan_vg(const char *vg_name); int is_orphan(pv_t *pv); vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name, + const char *vgid, uint32_t lock_flags, uint32_t status_flags, uint32_t misc_flags); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index a10009b1a..fbfffdffa 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -1922,6 +1922,7 @@ int vg_check_status(const struct volume_group *vg, uint32_t status) * non-NULL - success; volume group handle */ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name, + const char *vgid, uint32_t lock_flags, uint32_t status_flags, uint32_t misc_flags) { @@ -1942,7 +1943,7 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name, return NULL; } - if (!(vg = vg_read(cmd, vg_name, NULL, &consistent)) || + if (!(vg = vg_read(cmd, vg_name, vgid, &consistent)) || ((misc_flags & FAIL_INCONSISTENT) && !consistent)) { log_error("Volume group \"%s\" not found", vg_name); unlock_vg(cmd, vg_name); @@ -1953,10 +1954,10 @@ vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name, unlock_vg(cmd, vg_name); return NULL; } + return vg; } - /* * Gets/Sets for external LVM library */ diff --git a/tools/lvconvert.c b/tools/lvconvert.c index e3cc34f18..edc27b4b4 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -608,7 +608,7 @@ int lvconvert(struct cmd_context * cmd, int argc, char **argv) log_verbose("Checking for existing volume group \"%s\"", lp.vg_name); - if (!(vg = vg_lock_and_read(cmd, lp.vg_name, LCK_VG_WRITE, + if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE, CLUSTERED | EXPORTED_VG | LVM_WRITE, CORRECT_INCONSISTENT))) return ECMD_FAILED; diff --git a/tools/lvcreate.c b/tools/lvcreate.c index 2976cadb0..c9364787a 100644 --- a/tools/lvcreate.c +++ b/tools/lvcreate.c @@ -508,16 +508,16 @@ static int _lvcreate_params(struct lvcreate_params *lp, struct cmd_context *cmd, return 1; } -static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp) +static int _lvcreate(struct cmd_context *cmd, struct volume_group *vg, + struct lvcreate_params *lp) { uint32_t size_rest; uint32_t status = 0; uint64_t tmp_size; - struct volume_group *vg; struct logical_volume *lv, *org = NULL, *log_lv = NULL; struct list *pvh, tags; const char *tag = NULL; - int consistent = 1, origin_active = 0; + int origin_active = 0; struct alloc_handle *ah = NULL; char lv_name_buf[128]; const char *lv_name; @@ -526,17 +526,6 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp) status |= lp->permission | VISIBLE_LV; - /* does VG exist? */ - log_verbose("Finding volume group \"%s\"", lp->vg_name); - - if (!(vg = vg_read(cmd, lp->vg_name, NULL, &consistent))) { - log_error("Volume group \"%s\" doesn't exist", lp->vg_name); - return 0; - } - - if (!vg_check_status(vg, CLUSTERED | EXPORTED_VG | LVM_WRITE)) - return 0; - if (lp->lv_name && find_lv_in_vg(vg, lp->lv_name)) { log_error("Logical volume \"%s\" already exists in " "volume group \"%s\"", lp->lv_name, lp->vg_name); @@ -926,27 +915,24 @@ revert_new_lv: int lvcreate(struct cmd_context *cmd, int argc, char **argv) { - int r = ECMD_FAILED; + int r = ECMD_PROCESSED; struct lvcreate_params lp; + struct volume_group *vg; memset(&lp, 0, sizeof(lp)); if (!_lvcreate_params(&lp, cmd, argc, argv)) return EINVALID_CMD_LINE; - if (!lock_vol(cmd, lp.vg_name, LCK_VG_WRITE)) { - log_error("Can't get lock for %s", lp.vg_name); + log_verbose("Finding volume group \"%s\"", lp.vg_name); + if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE, + CLUSTERED | EXPORTED_VG | LVM_WRITE, + CORRECT_INCONSISTENT))) return ECMD_FAILED; - } - if (!_lvcreate(cmd, &lp)) { - stack; - goto out; - } + if (!_lvcreate(cmd, vg, &lp)) + r = ECMD_FAILED; - r = ECMD_PROCESSED; - - out: unlock_vg(cmd, lp.vg_name); return r; } diff --git a/tools/lvrename.c b/tools/lvrename.c index f8266333e..165c2e915 100644 --- a/tools/lvrename.c +++ b/tools/lvrename.c @@ -101,8 +101,7 @@ int lvrename(struct cmd_context *cmd, int argc, char **argv) } log_verbose("Checking for existing volume group \"%s\"", vg_name); - - if (!(vg = vg_lock_and_read(cmd, vg_name, LCK_VG_WRITE, + if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE, CLUSTERED | EXPORTED_VG | LVM_WRITE, CORRECT_INCONSISTENT))) return ECMD_FAILED; diff --git a/tools/lvresize.c b/tools/lvresize.c index 5ba1706fc..b083e3426 100644 --- a/tools/lvresize.c +++ b/tools/lvresize.c @@ -254,9 +254,9 @@ static int _lvresize_params(struct cmd_context *cmd, int argc, char **argv, return 1; } -static int _lvresize(struct cmd_context *cmd, struct lvresize_params *lp) +static int _lvresize(struct cmd_context *cmd, struct volume_group *vg, + struct lvresize_params *lp) { - struct volume_group *vg; struct logical_volume *lv; struct lvinfo info; uint32_t stripesize_extents = 0; @@ -268,7 +268,6 @@ static int _lvresize(struct cmd_context *cmd, struct lvresize_params *lp) alloc_policy_t alloc; struct logical_volume *lock_lv; struct lv_list *lvl; - int consistent = 1; struct lv_segment *seg; uint32_t seg_extents; uint32_t sz, str; @@ -276,14 +275,6 @@ static int _lvresize(struct cmd_context *cmd, struct lvresize_params *lp) char size_buf[SIZE_BUF]; char lv_path[PATH_MAX]; - if (!(vg = vg_read(cmd, lp->vg_name, NULL, &consistent))) { - log_error("Volume group %s doesn't exist", lp->vg_name); - return ECMD_FAILED; - } - - if (!vg_check_status(vg, CLUSTERED | EXPORTED_VG | LVM_WRITE)) - return ECMD_FAILED; - /* does LV exist? */ if (!(lvl = find_lv_in_vg(vg, lp->lv_name))) { log_error("Logical volume %s not found in volume group %s", @@ -649,6 +640,7 @@ static int _lvresize(struct cmd_context *cmd, struct lvresize_params *lp) int lvresize(struct cmd_context *cmd, int argc, char **argv) { struct lvresize_params lp; + struct volume_group *vg; int r; memset(&lp, 0, sizeof(lp)); @@ -657,12 +649,14 @@ int lvresize(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; log_verbose("Finding volume group %s", lp.vg_name); - if (!lock_vol(cmd, lp.vg_name, LCK_VG_WRITE)) { - log_error("Can't get lock for %s", lp.vg_name); + if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE, + CLUSTERED | EXPORTED_VG | LVM_WRITE, + CORRECT_INCONSISTENT))) { + log_error("Volume group %s doesn't exist", lp.vg_name); return ECMD_FAILED; } - if (!(r = _lvresize(cmd, &lp))) + if (!(r = _lvresize(cmd, vg, &lp))) stack; unlock_vg(cmd, lp.vg_name); diff --git a/tools/pvchange.c b/tools/pvchange.c index 3fdaed28d..066f3b651 100644 --- a/tools/pvchange.c +++ b/tools/pvchange.c @@ -32,7 +32,6 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, const char *orig_vg_name; char uuid[64] __attribute((aligned(8))); - int consistent = 1; int allocatable = 0; int tagarg = 0; @@ -54,27 +53,14 @@ static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv, /* If in a VG, must change using volume group. */ /* FIXME: handle PVs with no MDAs */ if (!is_orphan(pv)) { - log_verbose("Finding volume group of physical volume \"%s\"", - pv_name); - 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, vg_name, NULL, &consistent))) { - unlock_vg(cmd, vg_name); - log_error("Unable to find volume group of \"%s\"", - pv_name); - return 0; - } - - if (!vg_check_status(vg, - CLUSTERED | EXPORTED_VG | LVM_WRITE)) { - unlock_vg(cmd, vg_name); - return 0; - } + log_verbose("Finding volume group %s of physical volume %s", + vg_name, pv_name); + if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE, + CLUSTERED | EXPORTED_VG | LVM_WRITE, + CORRECT_INCONSISTENT))) + return_0; if (!(pvl = find_pv_in_vg(vg, pv_name))) { unlock_vg(cmd, vg_name); diff --git a/tools/pvdisplay.c b/tools/pvdisplay.c index c82ba5987..0528fdbf1 100644 --- a/tools/pvdisplay.c +++ b/tools/pvdisplay.c @@ -20,7 +20,6 @@ static int _pvdisplay_single(struct cmd_context *cmd, struct physical_volume *pv, void *handle) { struct pv_list *pvl; - int consistent = 0; int ret = ECMD_PROCESSED; uint64_t size; @@ -29,21 +28,12 @@ static int _pvdisplay_single(struct cmd_context *cmd, if (!is_orphan(pv) && !vg) { vg_name = pv_vg_name(pv); - if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { - log_error("Can't lock %s: skipping", vg_name); + if (!(vg = vg_lock_and_read(cmd, vg_name, (char *)&pv->vgid, + LCK_VG_READ, CLUSTERED, 0))) { + log_error("Skipping volume group %s", vg_name); return ECMD_FAILED; } - if (!(vg = vg_read(cmd, vg_name, (char *)&pv->vgid, &consistent))) { - log_error("Can't read %s: skipping", vg_name); - goto out; - } - - if (!vg_check_status(vg, CLUSTERED)) { - ret = ECMD_FAILED; - goto out; - } - /* * Replace possibly incomplete PV structure with new one * allocated in vg_read() path. diff --git a/tools/pvmove.c b/tools/pvmove.c index 5f1785d42..3a8d1085c 100644 --- a/tools/pvmove.c +++ b/tools/pvmove.c @@ -54,7 +54,7 @@ static struct volume_group *_get_vg(struct cmd_context *cmd, const char *vgname) dev_close_all(); - if (!(vg = vg_lock_and_read(cmd, vgname, LCK_VG_WRITE, + if (!(vg = vg_lock_and_read(cmd, vgname, NULL, LCK_VG_WRITE, CLUSTERED | EXPORTED_VG | LVM_WRITE, CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) return NULL; diff --git a/tools/reporter.c b/tools/reporter.c index 0c1b0f032..085d912ee 100644 --- a/tools/reporter.c +++ b/tools/reporter.c @@ -86,28 +86,18 @@ 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 (!is_orphan(pv) && !vg) { vg_name = pv_vg_name(pv); - if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { - log_error("Can't lock %s: skipping", vg_name); + if (!(vg = vg_lock_and_read(cmd, vg_name, (char *)&pv->vgid, + LCK_VG_READ, CLUSTERED, 0))) { + log_error("Skipping volume group %s", vg_name); return ECMD_FAILED; } - if (!(vg = vg_read(cmd, vg_name, (char *)&pv->vgid, &consistent))) { - log_error("Can't read %s: skipping", vg_name); - goto out; - } - - if (!vg_check_status(vg, CLUSTERED)) { - ret = ECMD_FAILED; - goto out; - } - /* * Replace possibly incomplete PV structure with new one * allocated in vg_read() path. diff --git a/tools/toollib.c b/tools/toollib.c index 7d071297b..35ea0d61e 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -427,24 +427,15 @@ int process_each_segment_in_pv(struct cmd_context *cmd, const char *vg_name = NULL; int ret_max = 0; int ret; - int consistent = 0; if (!vg) { vg_name = pv_vg_name(pv); - if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { - log_error("Can't lock %s: skipping", vg_name); + + if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_READ, + CLUSTERED, 0))) { + log_error("Skipping volume group %s", vg_name); return ECMD_FAILED; } - - if (!(vg = vg_read(cmd, vg_name, NULL, &consistent))) { - log_error("Can't read %s: skipping", vg_name); - goto out; - } - - if (!vg_check_status(vg, CLUSTERED)) { - ret = ECMD_FAILED; - goto out; - } } list_iterate_items(pvseg, &pv->segments) { diff --git a/tools/vgextend.c b/tools/vgextend.c index 25fd0f12f..4c02a1ecd 100644 --- a/tools/vgextend.c +++ b/tools/vgextend.c @@ -41,7 +41,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) } log_verbose("Checking for volume group \"%s\"", vg_name); - if (!(vg = vg_lock_and_read(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK, + if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE | LCK_NONBLOCK, CLUSTERED | EXPORTED_VG | LVM_WRITE | RESIZEABLE_VG, CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) { diff --git a/tools/vgmerge.c b/tools/vgmerge.c index 6cbbb3c18..51f8e2996 100644 --- a/tools/vgmerge.c +++ b/tools/vgmerge.c @@ -29,13 +29,13 @@ static int _vgmerge_single(struct cmd_context *cmd, const char *vg_name_to, } log_verbose("Checking for volume group \"%s\"", vg_name_to); - if (!(vg_to = vg_lock_and_read(cmd, vg_name_to, LCK_VG_WRITE, + if (!(vg_to = vg_lock_and_read(cmd, vg_name_to, NULL, LCK_VG_WRITE, CLUSTERED | EXPORTED_VG | LVM_WRITE, CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) return ECMD_FAILED; log_verbose("Checking for volume group \"%s\"", vg_name_from); - if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, + if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, NULL, LCK_VG_WRITE | LCK_NONBLOCK, CLUSTERED | EXPORTED_VG | LVM_WRITE, CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) { diff --git a/tools/vgsplit.c b/tools/vgsplit.c index 427967bb0..8c35ed212 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -234,7 +234,7 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) } log_verbose("Checking for volume group \"%s\"", vg_name_from); - if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, LCK_VG_WRITE, + if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, NULL, LCK_VG_WRITE, CLUSTERED | EXPORTED_VG | RESIZEABLE_VG | LVM_WRITE, CORRECT_INCONSISTENT | FAIL_INCONSISTENT)))