From 223c62e7b727bc8d37c076de256de4a6b68bcacc Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Wed, 14 Nov 2007 18:41:05 +0000 Subject: [PATCH] Avoid nested vg_reads when processing PVs in VGs and fix associated locking. --- WHATS_NEW | 1 + tools/pvdisplay.c | 39 ++++++++++++++++++++------------------- tools/pvresize.c | 3 ++- tools/reporter.c | 30 ++++++------------------------ tools/toollib.c | 42 +++++++++++++++++++++++++++++++++++++++--- tools/toollib.h | 2 +- tools/vgreduce.c | 2 +- 7 files changed, 70 insertions(+), 49 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index fdb4aaa5e..47a74eb78 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.29 - ================================== + Avoid nested vg_reads when processing PVs in VGs and fix associated locking. Accept sizes with --readahead argument. Store size arguments as sectors internally. Attempt to remove incomplete LVs with lvcreate zeroing/activation problems. diff --git a/tools/pvdisplay.c b/tools/pvdisplay.c index 495705942..c82ba5987 100644 --- a/tools/pvdisplay.c +++ b/tools/pvdisplay.c @@ -16,7 +16,7 @@ #include "tools.h" static int _pvdisplay_single(struct cmd_context *cmd, - struct volume_group *vg __attribute((unused)), + struct volume_group *vg, struct physical_volume *pv, void *handle) { struct pv_list *pvl; @@ -27,27 +27,27 @@ static int _pvdisplay_single(struct cmd_context *cmd, const char *pv_name = pv_dev_name(pv); const char *vg_name = NULL; - if (!is_orphan(pv)) { + 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); - return ECMD_FAILED; - } + 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, vg_name, (char *)&pv->vgid, &consistent))) { - log_error("Can't read %s: skipping", vg_name); - goto out; - } + 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; - } + if (!vg_check_status(vg, CLUSTERED)) { + ret = ECMD_FAILED; + goto out; + } - /* - * Replace possibly incomplete PV structure with new one - * allocated in vg_read() path. - */ + /* + * Replace possibly incomplete PV structure with new one + * allocated in vg_read() path. + */ if (!(pvl = find_pv_in_vg(vg, pv_name))) { log_error("Unable to find \"%s\" in volume group \"%s\"", pv_name, vg->name); @@ -119,5 +119,6 @@ int pvdisplay(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; } - return process_each_pv(cmd, argc, argv, NULL, NULL, _pvdisplay_single); + return process_each_pv(cmd, argc, argv, NULL, LCK_VG_READ, NULL, + _pvdisplay_single); } diff --git a/tools/pvresize.c b/tools/pvresize.c index 18494a2ca..f3b9d3926 100644 --- a/tools/pvresize.c +++ b/tools/pvresize.c @@ -61,7 +61,8 @@ int pvresize(struct cmd_context *cmd, int argc, char **argv) params.done = 0; params.total = 0; - ret = process_each_pv(cmd, argc, argv, NULL, ¶ms, _pvresize_single); + ret = process_each_pv(cmd, argc, argv, NULL, LCK_VG_WRITE, ¶ms, + _pvresize_single); log_print("%d physical volume(s) resized / %d physical volume(s) " "not resized", params.done, params.total - params.done); diff --git a/tools/reporter.c b/tools/reporter.c index d603e91b0..0c1b0f032 100644 --- a/tools/reporter.c +++ b/tools/reporter.c @@ -54,33 +54,15 @@ static int _segs_single(struct cmd_context *cmd __attribute((unused)), return ECMD_PROCESSED; } -static int _pvsegs_sub_single(struct cmd_context *cmd, struct volume_group *vg, +static int _pvsegs_sub_single(struct cmd_context *cmd __attribute((unused)), + struct volume_group *vg, struct pv_segment *pvseg, void *handle) { - int consistent = 0; - struct physical_volume *pv = pvseg->pv; int ret = ECMD_PROCESSED; - if (!lock_vol(cmd, pv_vg_name(pv), LCK_VG_READ)) { - log_error("Can't lock %s: skipping", pv_vg_name(pv)); - return ECMD_FAILED; - } - - if (!(vg = vg_read(cmd, pv_vg_name(pv), NULL, &consistent))) { - log_error("Can't read %s: skipping", pv_vg_name(pv)); - goto out; - } - - if (!vg_check_status(vg, CLUSTERED)) { - ret = ECMD_FAILED; - goto out; - } - - if (!report_object(handle, vg, NULL, pv, NULL, pvseg)) + if (!report_object(handle, vg, NULL, pvseg->pv, NULL, pvseg)) ret = ECMD_FAILED; -out: - unlock_vg(cmd, pv_vg_name(pv)); return ret; } @@ -108,7 +90,7 @@ static int _pvs_single(struct cmd_context *cmd, struct volume_group *vg, int ret = ECMD_PROCESSED; const char *vg_name = NULL; - if (!is_orphan(pv)) { + if (!is_orphan(pv) && !vg) { vg_name = pv_vg_name(pv); if (!lock_vol(cmd, vg_name, LCK_VG_READ)) { @@ -334,7 +316,7 @@ static int _report(struct cmd_context *cmd, int argc, char **argv, break; case PVS: if (args_are_pvs) - r = process_each_pv(cmd, argc, argv, NULL, + r = process_each_pv(cmd, argc, argv, NULL, LCK_VG_READ, report_handle, &_pvs_single); else r = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, @@ -346,7 +328,7 @@ static int _report(struct cmd_context *cmd, int argc, char **argv, break; case PVSEGS: if (args_are_pvs) - r = process_each_pv(cmd, argc, argv, NULL, + r = process_each_pv(cmd, argc, argv, NULL, LCK_VG_READ, report_handle, &_pvsegs_single); else r = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, diff --git a/tools/toollib.c b/tools/toollib.c index 1f0f25fa9..7d071297b 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -424,8 +424,28 @@ int process_each_segment_in_pv(struct cmd_context *cmd, void *handle)) { struct pv_segment *pvseg; + 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); + 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) { ret = process_single(cmd, vg, pvseg, handle); @@ -435,6 +455,10 @@ int process_each_segment_in_pv(struct cmd_context *cmd, return ret_max; } +out: + if (vg_name) + unlock_vg(cmd, vg_name); + return ret_max; } @@ -665,7 +689,7 @@ static int _process_all_devs(struct cmd_context *cmd, void *handle, } int process_each_pv(struct cmd_context *cmd, int argc, char **argv, - struct volume_group *vg, void *handle, + struct volume_group *vg, uint32_t lock_type, void *handle, int (*process_single) (struct cmd_context * cmd, struct volume_group * vg, struct physical_volume * pv, @@ -735,20 +759,32 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv, if (!list_empty(&tags) && (vgnames = get_vgs(cmd, 0)) && !list_empty(vgnames)) { list_iterate_items(sll, vgnames) { + if (!lock_vol(cmd, sll->str, lock_type)) { + log_error("Can't lock %s: skipping", sll->str); + continue; + } if (!(vg = vg_read(cmd, sll->str, NULL, &consistent))) { log_error("Volume group \"%s\" not found", sll->str); + unlock_vg(cmd, sll->str); ret_max = ECMD_FAILED; continue; } - if (!consistent) + if (!consistent) { + unlock_vg(cmd, sll->str); continue; + } - if (!vg_check_status(vg, CLUSTERED)) + if (!vg_check_status(vg, CLUSTERED)) { + unlock_vg(cmd, sll->str); continue; + } ret = process_each_pv_in_vg(cmd, vg, &tags, handle, process_single); + + unlock_vg(cmd, sll->str); + if (ret > ret_max) ret_max = ret; if (sigint_caught()) diff --git a/tools/toollib.h b/tools/toollib.h index 8ee539b07..b89f0c3f8 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -34,7 +34,7 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv, int consistent, void *handle)); int process_each_pv(struct cmd_context *cmd, int argc, char **argv, - struct volume_group *vg, void *handle, + struct volume_group *vg, uint32_t lock_type, void *handle, int (*process_single) (struct cmd_context * cmd, struct volume_group * vg, struct physical_volume * pv, diff --git a/tools/vgreduce.c b/tools/vgreduce.c index 134a5e281..6352119ff 100644 --- a/tools/vgreduce.c +++ b/tools/vgreduce.c @@ -539,7 +539,7 @@ int vgreduce(struct cmd_context *cmd, int argc, char **argv) /* FIXME: Pass private struct through to all these functions */ /* and update in batch here? */ - ret = process_each_pv(cmd, argc, argv, vg, NULL, + ret = process_each_pv(cmd, argc, argv, vg, LCK_VG_WRITE, NULL, _vgreduce_single); }