refactor: move report grouping and log reporting handles from processing_handle to...

With patches that will follow, this will make it possible to widen log
report coverage when commands are executed from lvm shell so the amount
of messages that may end up in stderr/stdout instead of log report are
minimized.

diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c
index 82e990ed97b3e4514e2581b3d92fa22bcd4f9cd0..57d3bdc294d108518ecc0dbee5ed85c242c3cf56 100644 (file)
--- a/lib/commands/toolcontext.c
+++ b/lib/commands/toolcontext.c
@@ -2218,8 +2218,10 @@ void destroy_toolcontext(struct cmd_context *cmd)
        if (cmd->cft_def_hash)
                dm_hash_destroy(cmd->cft_def_hash);
 
-       if (cmd->log_rh)
-               dm_report_free(cmd->log_rh);
+       if (cmd->cmd_report.log_rh) {
+               dm_report_free(cmd->cmd_report.log_rh);
+               cmd->cmd_report.log_rh = NULL;
+       }
 
        if (cmd->libmem)
                dm_pool_destroy(cmd->libmem);

diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h
index ee849b5e475e2d915ad33b33c3fc1951c4373dee..c6d938d8b2c6abcfb4e1fb92a964615a21a8a391 100644 (file)
--- a/lib/commands/toolcontext.h
+++ b/lib/commands/toolcontext.h
@@ -66,6 +66,15 @@ struct cmd_context_initialized_parts {
        unsigned connections:1;
 };
 
+struct cmd_report {
+       int log_only;
+       dm_report_group_type_t report_group_type;
+       struct dm_report_group *report_group;
+       struct dm_report *log_rh;
+       const char *log_name;
+       log_report_t saved_log_report_state;
+};
+
 /* FIXME Split into tool & library contexts */
 /* command-instance-related variables needed by library */
 struct cmd_context {
@@ -186,9 +195,9 @@ struct cmd_context {
        char proc_dir[PATH_MAX];
 
        /*
-        * Command log reporting.
+        * Reporting.
         */
-       struct dm_report *log_rh;               /* keep log report of last cmd for further queries if cmd line is interactive (e.g. lvm shell) */
+       struct cmd_report cmd_report;
 
        /*
         * Buffers.

diff --git a/lib/report/report.h b/lib/report/report.h
index ba3a8fad09e936a47bd4454b4f2c8d35abcfd33a..cf4145e6b681e9c3c1596462c5ad8006e66276a4 100644 (file)
--- a/lib/report/report.h
+++ b/lib/report/report.h
@@ -81,9 +81,7 @@ struct processing_handle;
 typedef int (*field_report_fn) (struct report_handle * dh, struct field * field,
                                const void *data);
 
-int report_format_init(struct cmd_context *cmd, dm_report_group_type_t *report_group_type,
-                      struct dm_report_group **report_group, struct dm_report **log_rh,
-                      int *log_only, log_report_t *saved_log_report_state);
+int report_format_init(struct cmd_context *cmd);
 
 void *report_init(struct cmd_context *cmd, const char *format, const char *keys,
                  report_type_t *report_type, const char *separator,

diff --git a/tools/lvm.c b/tools/lvm.c
index ec8a3522e3052102284479aa6106853b485de3cf..c2d509669736009fe54974bf035cd8e720e82589 100644 (file)
--- a/tools/lvm.c
+++ b/tools/lvm.c
@@ -185,12 +185,12 @@ static int _log_shell_command_status(struct cmd_context *cmd, int ret_code)
 {
        log_report_t log_state;
 
-       if (!cmd->log_rh)
+       if (!cmd->cmd_report.log_rh)
                return 1;
 
        log_state = log_get_report_state();
 
-       return report_cmdlog(cmd->log_rh, REPORT_OBJECT_CMDLOG_NAME,
+       return report_cmdlog(cmd->cmd_report.log_rh, REPORT_OBJECT_CMDLOG_NAME,
                             log_get_report_context_name(log_state.context),
                             log_get_report_object_type_name(log_state.object_type),
                             log_state.object_name, log_state.object_id,
@@ -263,10 +263,10 @@ int lvm_shell(struct cmd_context *cmd, struct cmdline_context *cmdline)
 
                is_lastlog_cmd = !strcmp(argv[0], "lastlog");
 
-               if (cmd->log_rh && !is_lastlog_cmd) {
+               if (cmd->cmd_report.log_rh && !is_lastlog_cmd) {
                        /* drop old log report */
-                       dm_report_free(cmd->log_rh);
-                       cmd->log_rh = NULL;
+                       dm_report_free(cmd->cmd_report.log_rh);
+                       cmd->cmd_report.log_rh = NULL;
                }
 
                ret = lvm_run_command(cmd, argc, argv);

diff --git a/tools/reporter.c b/tools/reporter.c
index ad0a35617d1ea2ceecfa2a288480c86994a7e054..d386efca092ceb187c0ce32ad11467174db6036c 100644 (file)
--- a/tools/reporter.c
+++ b/tools/reporter.c
@@ -1055,7 +1055,7 @@ static int _do_report(struct cmd_context *cmd, struct processing_handle *handle,
                goto_out;
 
        if (!(args->log_only && (single_args->report_type != CMDLOG))) {
-               if (!dm_report_group_push(handle->report_group, report_handle, (void *) single_args->report_name))
+               if (!dm_report_group_push(cmd->cmd_report.report_group, report_handle, (void *) single_args->report_name))
                        goto_out;
                report_in_group = 1;
        }
@@ -1174,7 +1174,7 @@ static int _do_report(struct cmd_context *cmd, struct processing_handle *handle,
                unlock_vg(cmd, NULL, VG_GLOBAL);
 out:
        if (report_handle) {
-               if (report_in_group && !dm_report_group_pop(handle->report_group))
+               if (report_in_group && !dm_report_group_pop(cmd->cmd_report.report_group))
                        stack;
                dm_report_free(report_handle);
        }
@@ -1197,7 +1197,7 @@ static int _full_report_single(struct cmd_context *cmd,
 
        args->full_report_vg = vg;
 
-       if (!args->log_only && !dm_report_group_push(handle->report_group, NULL, NULL))
+       if (!args->log_only && !dm_report_group_push(cmd->cmd_report.report_group, NULL, NULL))
                goto out;
 
        if (orphan) {
@@ -1213,7 +1213,7 @@ static int _full_report_single(struct cmd_context *cmd,
                        stack;
        }
 
-       if (!args->log_only && !dm_report_group_pop(handle->report_group))
+       if (!args->log_only && !dm_report_group_pop(cmd->cmd_report.report_group))
                goto_out;
 out:
        args->full_report_vg = NULL;
@@ -1367,15 +1367,15 @@ static int _report(struct cmd_context *cmd, int argc, char **argv, report_type_t
        handle->internal_report_for_select = 0;
        handle->include_historical_lvs = cmd->include_historical_lvs;
 
-       args.report_group_type = handle->report_group_type;
-       args.log_only = handle->log_only;
+       args.report_group_type = cmd->cmd_report.report_group_type;
+       args.log_only = cmd->cmd_report.log_only;
 
        if (!_config_report(cmd, &args, single_args)) {
                destroy_processing_handle(cmd, handle);
                return_ECMD_FAILED;
        }
 
-       if (!args.log_only && !dm_report_group_push(handle->report_group, NULL, report_name)) {
+       if (!args.log_only && !dm_report_group_push(cmd->cmd_report.report_group, NULL, report_name)) {
                log_error("Failed to add main report section to report group.");
                destroy_processing_handle(cmd, handle);
                return ECMD_FAILED;
@@ -1433,9 +1433,7 @@ int devtypes(struct cmd_context *cmd, int argc, char **argv)
 #define REPORT_FORMAT_NAME_BASIC "basic"
 #define REPORT_FORMAT_NAME_JSON "json"
 
-int report_format_init(struct cmd_context *cmd, dm_report_group_type_t *report_group_type,
-                      struct dm_report_group **report_group, struct dm_report **log_rh,
-                      int *log_only, log_report_t *saved_log_report_state)
+int report_format_init(struct cmd_context *cmd)
 {
        int config_set = find_config_tree_node(cmd, report_output_format_CFG, NULL) != NULL;
        const char *config_format_str = find_config_tree_str(cmd, report_output_format_CFG, NULL);
@@ -1446,7 +1444,7 @@ int report_format_init(struct cmd_context *cmd, dm_report_group_type_t *report_g
        struct dm_report_group *new_report_group;
        struct dm_report *tmp_log_rh = NULL;
 
-       args.log_only = arg_is_set(cmd, logonly_ARG) || *log_rh;
+       args.log_only = arg_is_set(cmd, logonly_ARG);
        report_command_log = args.log_only || find_config_tree_bool(cmd, log_report_command_log_CFG, NULL);
 
        if (!format_str || !strcmp(format_str, REPORT_FORMAT_NAME_BASIC)) {
@@ -1462,10 +1460,8 @@ int report_format_init(struct cmd_context *cmd, dm_report_group_type_t *report_g
                return 0;
        }
 
-       if (report_group_type)
-               *report_group_type = args.report_group_type;
-       if (log_only)
-               *log_only = args.log_only;
+       cmd->cmd_report.report_group_type = args.report_group_type;
+       cmd->cmd_report.log_only = args.log_only;
 
        if (!(new_report_group = dm_report_group_create(args.report_group_type, NULL))) {
                log_error("Failed to create report group.");
@@ -1476,40 +1472,33 @@ int report_format_init(struct cmd_context *cmd, dm_report_group_type_t *report_g
                single_args = &args.single_args[REPORT_IDX_LOG];
                single_args->report_type = CMDLOG;
 
-               if (!*log_rh) {
-                       if (!_config_report(cmd, &args, single_args))
-                               goto_bad;
+               if (!_config_report(cmd, &args, single_args))
+                       goto_bad;
 
-                       if (!(tmp_log_rh = report_init(NULL, single_args->options, single_args->keys, &single_args->report_type,
-                                                         args.separator, args.aligned, args.buffered, args.headings,
-                                                         args.field_prefixes, args.quoted, args.columns_as_rows,
-                                                         single_args->selection, 1))) {
-                               log_error("Failed to create log report.");
-                               goto bad;
-                       }
-               } else {
-                       /*
-                        * We reusing existing log report handle.
-                        * Just get report's name and prefix now.
-                        */
-                       if (!_set_report_prefix_and_name(&args, single_args))
-                               goto_bad;
+               if (!(tmp_log_rh = report_init(NULL, single_args->options, single_args->keys, &single_args->report_type,
+                                                 args.separator, args.aligned, args.buffered, args.headings,
+                                                 args.field_prefixes, args.quoted, args.columns_as_rows,
+                                                 single_args->selection, 1))) {
+                       log_error("Failed to create log report.");
+                       goto bad;
                }
 
-               if (!(dm_report_group_push(new_report_group, tmp_log_rh ? : *log_rh, (void *) single_args->report_name))) {
+               if (!(dm_report_group_push(new_report_group, tmp_log_rh, (void *) single_args->report_name))) {
                        log_error("Failed to add log report to report group.");
                        goto bad;
                }
+
+               cmd->cmd_report.log_rh = tmp_log_rh;
+               if (!(cmd->cmd_report.log_name = dm_pool_strdup(cmd->libmem, single_args->report_name))) {
+                       log_error("Failed to set log report name for command context.");
+                       goto bad;
+               }
        }
 
-       *report_group = new_report_group;
-       if (tmp_log_rh)
-               *log_rh = tmp_log_rh;
+       cmd->cmd_report.report_group = new_report_group;
+       cmd->cmd_report.saved_log_report_state = log_get_report_state();
+       log_set_report(cmd->cmd_report.log_rh);
 
-       if (saved_log_report_state) {
-               *saved_log_report_state = log_get_report_state();
-               log_set_report(*log_rh);
-       }
        return 1;
 bad:
        if (!dm_report_group_destroy(new_report_group))
@@ -1525,23 +1514,20 @@ int lastlog(struct cmd_context *cmd, int argc, char **argv)
        const char *selection;
        int r = ECMD_FAILED;
 
-       if (!cmd->log_rh) {
+       if (!cmd->cmd_report.log_rh) {
                log_error("No log report stored.");
                goto out;
        }
 
-       if (!report_format_init(cmd, NULL, &report_group, &cmd->log_rh, NULL, NULL))
-               goto_out;
-
        if (!_do_report_get_selection(cmd, CMDLOG, 1, NULL, &selection))
                goto_out;
 
-       if (!dm_report_set_selection(cmd->log_rh, selection)) {
+       if (!dm_report_set_selection(cmd->cmd_report.log_rh, selection)) {
                log_error("Failed to set selection for log report.");
                goto out;
        }
 
-       if (!dm_report_output(cmd->log_rh) ||
+       if (!dm_report_output(cmd->cmd_report.log_rh) ||
            !dm_report_group_pop(report_group))
                goto_out;
 

diff --git a/tools/toollib.c b/tools/toollib.c
index 4cd902097272d6a940c0b2ca3307d41dd547e825..bcf94ead2597ad0607d0611ca47f52a3098b19c0 100644 (file)
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1754,15 +1754,13 @@ struct processing_handle *init_processing_handle(struct cmd_context *cmd, struct
        handle->internal_report_for_select = arg_is_set(cmd, select_ARG);
        handle->include_historical_lvs = cmd->include_historical_lvs;
 
-       if (!parent_handle) {
-               if (!report_format_init(cmd, &handle->report_group_type, &handle->report_group,
-                                       &handle->log_rh, &handle->log_only,
-                                       &handle->saved_log_report_state)) {
+       if (!parent_handle && !cmd->cmd_report.report_group) {
+               if (!report_format_init(cmd)) {
                        dm_pool_free(cmd->mem, handle);
                        return NULL;
                }
        } else
-               handle->saved_log_report_state = log_get_report_state();
+               cmd->cmd_report.saved_log_report_state = log_get_report_state();
 
        log_set_report_context(LOG_REPORT_CONTEXT_PROCESSING);
        return handle;
@@ -1798,19 +1796,13 @@ void destroy_processing_handle(struct cmd_context *cmd, struct processing_handle
                if (handle->selection_handle && handle->selection_handle->selection_rh)
                        dm_report_free(handle->selection_handle->selection_rh);
 
-               log_restore_report_state(handle->saved_log_report_state);
+               log_restore_report_state(cmd->cmd_report.saved_log_report_state);
 
-               if (!dm_report_group_destroy(handle->report_group))
+               if (!dm_report_group_destroy(cmd->cmd_report.report_group))
                        stack;
-               if (handle->log_rh) {
-                       if (cmd->is_interactive) {
-                               /*
-                                * Keep log report if we're interactive so
-                                * we can do further queries on this report.
-                                */
-                               cmd->log_rh = handle->log_rh;
-                       } else
-                               dm_report_free(handle->log_rh);
+               if (!cmd->is_interactive && cmd->cmd_report.log_rh) {
+                       dm_report_free(cmd->cmd_report.log_rh);
+                       cmd->cmd_report.log_rh = NULL;
                }
                /*
                 * TODO: think about better alternatives:

diff --git a/tools/toollib.h b/tools/toollib.h
index ed5ec7635afd4e242c97b54da70dad51d55cac9b..d5de73319085e704b355a3a991abc698d7dfecc7 100644 (file)
--- a/tools/toollib.h
+++ b/tools/toollib.h
@@ -73,11 +73,6 @@ struct processing_handle {
        int internal_report_for_select;
        int include_historical_lvs;
        struct selection_handle *selection_handle;
-       dm_report_group_type_t report_group_type;
-       struct dm_report_group *report_group;
-       struct dm_report *log_rh;
-       int log_only;
-       log_report_t saved_log_report_state;
        void *custom_handle;
 };