From 66b10d6d126fdf476cf43eec25820b021752b383 Mon Sep 17 00:00:00 2001 From: Peter Rajnoha Date: Fri, 13 Feb 2015 10:36:06 +0100 Subject: [PATCH] cleanup: replace static struct processing_handle initializer with common init_processing_handle It's cleaner this way - do not mix static and dynamic (init_processing_handle) initializers. Use the dynamic one everywhere. This makes it easier to manage the code - there are no "exceptions" then and we don't need to take care about two ways of initializing the same thing - just use one common initializer throughout and it's clear. Also, add more comments, mainly in the report_for_selection fn explaining what is being done and why with respect to the processing_handle and selection_handle. --- lib/report/report.h | 3 ++- tools/lvconvert.c | 15 +++++++---- tools/polldaemon.c | 24 ++++++++++------- tools/pvresize.c | 16 ++++++++---- tools/reporter.c | 63 ++++++++++++++++++++++++++++++++++----------- tools/toollib.c | 6 ++--- tools/vgcfgbackup.c | 15 +++++++---- 7 files changed, 99 insertions(+), 43 deletions(-) diff --git a/lib/report/report.h b/lib/report/report.h index 5bd8b7930..84c5f91d1 100644 --- a/lib/report/report.h +++ b/lib/report/report.h @@ -71,7 +71,8 @@ void *report_init(struct cmd_context *cmd, const char *format, const char *keys, int quoted, int columns_as_rows, const char *selection); void *report_init_for_selection(struct cmd_context *cmd, report_type_t *report_type, const char *selection); -int report_for_selection(struct selection_handle *sh, +int report_for_selection(struct cmd_context *cmd, + struct selection_handle *sh, struct physical_volume *pv, struct volume_group *vg, struct logical_volume *lv); diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 2df2483f2..b2527e40f 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -3542,10 +3542,15 @@ int lvconvert(struct cmd_context * cmd, int argc, char **argv) struct lvconvert_params lp = { .target_attr = ~0, }; + struct processing_handle *handle = NULL; - struct processing_handle handle = { .internal_report_for_select = 1, - .selection_handle = NULL, - .custom_handle = &lp }; + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + ret = ECMD_FAILED; + goto out; + } + + handle->custom_handle = &lp; if (!_read_params(cmd, argc, argv, &lp)) { ret = EINVALID_CMD_LINE; @@ -3553,11 +3558,11 @@ int lvconvert(struct cmd_context * cmd, int argc, char **argv) } if (lp.merge) - ret = process_each_lv(cmd, argc, argv, READ_FOR_UPDATE, &handle, + ret = process_each_lv(cmd, argc, argv, READ_FOR_UPDATE, handle, &_lvconvert_merge_single); else ret = lvconvert_single(cmd, &lp); out: - destroy_processing_handle(cmd, &handle, 0); + destroy_processing_handle(cmd, handle, 1); return ret; } diff --git a/tools/polldaemon.c b/tools/polldaemon.c index 204c49520..722a385bf 100644 --- a/tools/polldaemon.c +++ b/tools/polldaemon.c @@ -227,21 +227,17 @@ static int _poll_vg(struct cmd_context *cmd, const char *vgname, } static void _poll_for_all_vgs(struct cmd_context *cmd, - struct daemon_parms *parms) + struct processing_handle *handle) { - struct processing_handle handle = { .internal_report_for_select = 1, - .selection_handle = NULL, - .custom_handle = parms }; + struct daemon_parms *parms = (struct daemon_parms *) handle->custom_handle; while (1) { parms->outstanding_count = 0; - process_each_vg(cmd, 0, NULL, READ_FOR_UPDATE, &handle, _poll_vg); + process_each_vg(cmd, 0, NULL, READ_FOR_UPDATE, handle, _poll_vg); if (!parms->outstanding_count) break; sleep(parms->interval); } - - destroy_processing_handle(cmd, &handle, 0); } /* @@ -255,6 +251,7 @@ int poll_daemon(struct cmd_context *cmd, const char *name, const char *uuid, uint64_t lv_type, struct poll_functions *poll_fns, const char *progress_title) { + struct processing_handle *handle = NULL; struct daemon_parms parms; int daemon_mode = 0; int ret = ECMD_PROCESSED; @@ -306,10 +303,18 @@ int poll_daemon(struct cmd_context *cmd, const char *name, const char *uuid, stack; ret = ECMD_FAILED; } - } else - _poll_for_all_vgs(cmd, &parms); + } else { + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + ret = ECMD_FAILED; + } else { + handle->custom_handle = &parms; + _poll_for_all_vgs(cmd, handle); + } + } if (parms.background && daemon_mode == 1) { + destroy_processing_handle(cmd, handle, 1); /* * child was successfully forked: * background polldaemon must not return to the caller @@ -320,5 +325,6 @@ int poll_daemon(struct cmd_context *cmd, const char *name, const char *uuid, _exit(lvm_return_code(ret)); } + destroy_processing_handle(cmd, handle, 1); return ret; } diff --git a/tools/pvresize.c b/tools/pvresize.c index f6253d985..ec638a47c 100644 --- a/tools/pvresize.c +++ b/tools/pvresize.c @@ -47,9 +47,7 @@ static int _pvresize_single(struct cmd_context *cmd, int pvresize(struct cmd_context *cmd, int argc, char **argv) { struct pvresize_params params; - struct processing_handle handle = { .internal_report_for_select = 1, - .selection_handle = NULL, - .custom_handle = ¶ms }; + struct processing_handle *handle = NULL; int ret = ECMD_PROCESSED; if (!argc) { @@ -70,12 +68,20 @@ int pvresize(struct cmd_context *cmd, int argc, char **argv) params.done = 0; params.total = 0; - ret = process_each_pv(cmd, argc, argv, NULL, READ_FOR_UPDATE, &handle, + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + ret = ECMD_FAILED; + goto out; + } + + handle->custom_handle = ¶ms; + + ret = process_each_pv(cmd, argc, argv, NULL, READ_FOR_UPDATE, handle, _pvresize_single); log_print_unless_silent("%d physical volume(s) resized / %d physical volume(s) " "not resized", params.done, params.total - params.done); out: - destroy_processing_handle(cmd, &handle, 0); + destroy_processing_handle(cmd, handle, 1); return ret; } diff --git a/tools/reporter.c b/tools/reporter.c index 0bfed51b0..9cc87bd01 100644 --- a/tools/reporter.c +++ b/tools/reporter.c @@ -447,7 +447,8 @@ static int _get_final_report_type(int args_are_pvs, return 1; } -int report_for_selection(struct selection_handle *sh, +int report_for_selection(struct cmd_context *cmd, + struct selection_handle *sh, struct physical_volume *pv, struct volume_group *vg, struct logical_volume *lv) @@ -455,9 +456,7 @@ int report_for_selection(struct selection_handle *sh, static const char *incorrect_report_type_msg = "report_for_selection: incorrect report type"; int args_are_pvs = sh->orig_report_type == PVS; int do_lv_info, do_lv_seg_status; - struct processing_handle handle = { .internal_report_for_select = 0, - .selection_handle = sh, - .custom_handle = NULL }; + struct processing_handle *handle; int r = 0; if (!_get_final_report_type(args_are_pvs, @@ -467,19 +466,46 @@ int report_for_selection(struct selection_handle *sh, &sh->report_type)) return_0; + if (!(handle = init_processing_handle(cmd))) + return_0; + + /* + * We're already reporting for select so override + * internal_report_for_select to 0 as we can call + * process_each_* functions again and we could + * end up in an infinite loop if we didn't stop + * internal reporting for select right here. + * + * So the overall call trace from top to bottom looks like this: + * + * process_each_* (top-level one, using processing_handle with internal reporting enabled and selection_handle) -> + * select_match_*(processing_handle with selection_handle) -> + * report for selection -> + * (creating new processing_handle here with internal reporting disabled!!!) + * reporting_fn OR process_each_* (using *new* processing_handle with original selection_handle) + * + * The selection_handle is still reused so we can track + * whether any of the items the top-level one is composed + * of are still selected or not unerneath. Do not destroy + * this selection handle - it needs to be passed to upper + * layers to check the overall selection status. + */ + handle->internal_report_for_select = 0; + handle->selection_handle = sh; + /* * Remember: - * sh->orig_report_type is the original report type requested - * sh->report_type is the report type actually used (it counts with all types of fields used in selection) + * sh->orig_report_type is the original report type requested (what are we selecting? PV/VG/LV?) + * sh->report_type is the report type actually used (it counts with all types of fields used in selection criteria) */ switch (sh->orig_report_type) { case LVS: switch (sh->report_type) { case LVS: - r = _do_lvs_with_info_and_status_single(vg->cmd, lv, do_lv_info, do_lv_seg_status, &handle); + r = _do_lvs_with_info_and_status_single(vg->cmd, lv, do_lv_info, do_lv_seg_status, handle); break; case SEGS: - r = process_each_segment_in_lv(vg->cmd, lv, &handle, + r = process_each_segment_in_lv(vg->cmd, lv, handle, do_lv_info && !do_lv_seg_status ? &_segs_with_info_single : !do_lv_info && do_lv_seg_status ? &_segs_with_status_single : do_lv_info && do_lv_seg_status ? &_segs_with_info_and_status_single : @@ -493,27 +519,27 @@ int report_for_selection(struct selection_handle *sh, case VGS: switch (sh->report_type) { case VGS: - r = _vgs_single(vg->cmd, vg->name, vg, &handle); + r = _vgs_single(vg->cmd, vg->name, vg, handle); break; case LVS: - r = process_each_lv_in_vg(vg->cmd, vg, NULL, NULL, 0, &handle, + r = process_each_lv_in_vg(vg->cmd, vg, NULL, NULL, 0, handle, do_lv_info && !do_lv_seg_status ? &_lvs_with_info_single : !do_lv_info && do_lv_seg_status ? &_lvs_with_status_single : do_lv_info && do_lv_seg_status ? &_lvs_with_info_and_status_single : &_lvs_single); break; case SEGS: - r = process_each_lv_in_vg(vg->cmd, vg, NULL, NULL, 0, &handle, + r = process_each_lv_in_vg(vg->cmd, vg, NULL, NULL, 0, handle, do_lv_info && !do_lv_seg_status ? &_lvsegs_with_info_single : !do_lv_info && do_lv_seg_status ? &_lvsegs_with_status_single : do_lv_info && do_lv_seg_status ? &_lvsegs_with_info_and_status_single : &_lvsegs_single); break; case PVS: - r = process_each_pv_in_vg(vg->cmd, vg, &handle, &_pvs_single); + r = process_each_pv_in_vg(vg->cmd, vg, handle, &_pvs_single); break; case PVSEGS: - r = process_each_pv_in_vg(vg->cmd, vg, &handle, + r = process_each_pv_in_vg(vg->cmd, vg, handle, do_lv_info && !do_lv_seg_status ? &_pvsegs_with_lv_info_single : !do_lv_info && do_lv_seg_status ? &_pvsegs_with_lv_status_single : do_lv_info && do_lv_seg_status ? &_pvsegs_with_lv_info_and_status_single : @@ -527,10 +553,10 @@ int report_for_selection(struct selection_handle *sh, case PVS: switch (sh->report_type) { case PVS: - r = _pvs_single(vg->cmd, vg, pv, &handle); + r = _pvs_single(vg->cmd, vg, pv, handle); break; case PVSEGS: - r = process_each_segment_in_pv(vg->cmd, vg, pv, &handle, + r = process_each_segment_in_pv(vg->cmd, vg, pv, handle, do_lv_info && !do_lv_seg_status ? &_pvsegs_with_lv_info_sub_single : !do_lv_info && do_lv_seg_status ? &_pvsegs_with_lv_status_sub_single : do_lv_info && do_lv_seg_status ? &_pvsegs_with_lv_info_and_status_sub_single : @@ -546,6 +572,13 @@ int report_for_selection(struct selection_handle *sh, break; } + /* + * Keep the selection handle provided from the caller - + * do not destroy it - the caller will still use it to + * pass the result through it to layers above. + */ + handle->selection_handle = NULL; + destroy_processing_handle(cmd, handle, 1); return r; } diff --git a/tools/toollib.c b/tools/toollib.c index 43c7fc100..4e0d0e7a6 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1639,7 +1639,7 @@ int select_match_vg(struct cmd_context *cmd, struct processing_handle *handle, sh->orig_report_type = VGS; - if (!report_for_selection(sh, NULL, vg, NULL)) { + if (!report_for_selection(cmd, sh, NULL, vg, NULL)) { log_error("Selection failed for VG %s.", vg->name); return 0; } @@ -1662,7 +1662,7 @@ int select_match_lv(struct cmd_context *cmd, struct processing_handle *handle, sh->orig_report_type = LVS; - if (!report_for_selection(sh, NULL, vg, lv)) { + if (!report_for_selection(cmd, sh, NULL, vg, lv)) { log_error("Selection failed for LV %s.", lv->name); return 0; } @@ -1685,7 +1685,7 @@ int select_match_pv(struct cmd_context *cmd, struct processing_handle *handle, sh->orig_report_type = PVS; - if (!report_for_selection(sh, pv, vg, NULL)) { + if (!report_for_selection(cmd, sh, pv, vg, NULL)) { log_error("Selection failed for PV %s.", dev_name(pv->dev)); return 0; } diff --git a/tools/vgcfgbackup.c b/tools/vgcfgbackup.c index 0a6c5f257..a25a7185a 100644 --- a/tools/vgcfgbackup.c +++ b/tools/vgcfgbackup.c @@ -83,19 +83,24 @@ int vgcfgbackup(struct cmd_context *cmd, int argc, char **argv) { int ret; char *last_filename = NULL; - struct processing_handle handle = { .internal_report_for_select = 1, - .selection_handle = NULL, - .custom_handle = &last_filename }; + struct processing_handle *handle = NULL; + + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; + } + + handle->custom_handle = &last_filename; init_pvmove(1); ret = process_each_vg(cmd, argc, argv, READ_ALLOW_INCONSISTENT, - &handle, &vg_backup_single); + handle, &vg_backup_single); dm_free(last_filename); init_pvmove(0); - destroy_processing_handle(cmd, &handle, 0); + destroy_processing_handle(cmd, handle, 1); return ret; }