From 3dae1685cf1f1a368cd83928433eb778c8815dab Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 19 Aug 2019 20:56:26 -0400 Subject: [PATCH] autodisc: make it possible to interrupt auto source discovery It is currently not possible to interrupt babeltrace2 while it is doing auto source discovery. However, it can be a quite long process that the user might want to interrupt using ctrl-C. This patch makes it possible to do that. An interrupter object is taken by the auto source discovery code and passed all the way to support_info_query_all_sources. In the CLI, the global `the_interrupter` object, which is set on sigint, is passed. Because support_info_query_all_sources (and other internal functions) can now return a few different values (success with a match, success without a match, error and interrupted), I changed it to return an enum type. The entry point of auto source discovery, auto_discover_source_components, can now return "interrupted" as well. However, since that function doesn't return "no match", I didn't want to make it use the same return type as support_info_query_all_sources. I therefore made an "internal" and public version of the status enum. With this patch, there is no way to interrupt an auto source discovery started from Python. Change-Id: I0040c7fab1887c4e05decd0659fa8ff3c85a16a1 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/1972 Tested-by: jenkins Reviewed-by: Francis Deslauriers --- src/autodisc/autodisc.c | 135 +++++++++++------- src/autodisc/autodisc.h | 18 ++- .../python/bt2/bt2/native_bt_autodisc.i.h | 3 +- src/cli/babeltrace2-cfg-cli-args-default.c | 10 +- src/cli/babeltrace2-cfg-cli-args-default.h | 3 +- src/cli/babeltrace2-cfg-cli-args.c | 13 +- src/cli/babeltrace2-cfg-cli-args.h | 3 +- src/cli/babeltrace2.c | 22 +-- 8 files changed, 130 insertions(+), 77 deletions(-) diff --git a/src/autodisc/autodisc.c b/src/autodisc/autodisc.c index 02bdacc5..6ba393e8 100644 --- a/src/autodisc/autodisc.c +++ b/src/autodisc/autodisc.c @@ -37,6 +37,18 @@ #define BT_AUTODISC_LOGE_APPEND_CAUSE(_fmt, ...) \ BT_AUTODISC_LOG_AND_APPEND(BT_LOG_ERROR, _fmt, ##__VA_ARGS__) +/* + * Define a status enum for inside the auto source discovery code, + * as we don't want to return `NO_MATCH` to the caller. + */ +typedef enum auto_source_discovery_internal_status { + AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK = AUTO_SOURCE_DISCOVERY_STATUS_OK, + AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_ERROR = AUTO_SOURCE_DISCOVERY_STATUS_ERROR, + AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_MEMORY_ERROR = AUTO_SOURCE_DISCOVERY_STATUS_MEMORY_ERROR, + AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED = AUTO_SOURCE_DISCOVERY_STATUS_INTERRUPTED, + AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH = __BT_FUNC_STATUS_NO_MATCH, +} auto_source_discovery_internal_status; + /* Finalize and free a `struct auto_source_discovery_result`. */ static @@ -143,7 +155,8 @@ const bt_value *borrow_array_value_last_element_const(const bt_value *array) */ static -int auto_source_discovery_add(struct auto_source_discovery *auto_disc, +auto_source_discovery_internal_status auto_source_discovery_add( + struct auto_source_discovery *auto_disc, const char *plugin_name, const char *source_cc_name, const char *group, @@ -151,7 +164,7 @@ int auto_source_discovery_add(struct auto_source_discovery *auto_disc, uint64_t original_input_index, bt_logging_level log_level) { - int status; + auto_source_discovery_internal_status status; bt_value_array_append_element_status append_status; guint len; guint i; @@ -228,11 +241,11 @@ int auto_source_discovery_add(struct auto_source_discovery *auto_disc, } } - status = 0; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK; goto end; error: - status = -1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_ERROR; end: return status; @@ -326,26 +339,22 @@ end: * * If `component_class_restrict` is non-NULL, only query source component classes * with that name. - * - * Return: - * - * - > 0 on success, if no source component class has reported that it handles `input` - * - 0 on success, if a source component class has reported that it handles `input` - * - < 0 on failure (e.g. memory error) */ static -int support_info_query_all_sources(const char *input, +auto_source_discovery_internal_status support_info_query_all_sources( + const char *input, const char *input_type, uint64_t original_input_index, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc) + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter) { bt_value_map_insert_entry_status insert_status; bt_value *query_params = NULL; - int status; + auto_source_discovery_internal_status status; size_t i_plugins; const struct bt_value *query_result = NULL; struct { @@ -355,6 +364,11 @@ int support_info_query_all_sources(const char *input, double weigth; } winner = { NULL, NULL, NULL, 0 }; + if (interrupter && bt_interrupter_is_set(interrupter)) { + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED; + goto end; + } + query_params = bt_value_map_create(); if (!query_params) { BT_AUTODISC_LOGE_APPEND_CAUSE("Failed to allocate a map value."); @@ -527,19 +541,19 @@ int support_info_query_all_sources(const char *input, status = auto_source_discovery_add(auto_disc, plugin_name, source_name, group, input, original_input_index, log_level); - if (status != 0) { + if (status != AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK) { goto error; } } else { BT_LOGI("Input %s (%s) was not recognized by any source component class.", input, input_type); - status = 1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH; } goto end; error: - status = -1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_ERROR; end: bt_value_put_ref(query_result); @@ -557,55 +571,62 @@ end: */ static -int auto_discover_source_for_input_as_string(const char *input, +auto_source_discovery_internal_status auto_discover_source_for_input_as_string( + const char *input, uint64_t original_input_index, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc) + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter) { return support_info_query_all_sources(input, "string", original_input_index, plugins, plugin_count, - component_class_restrict, log_level, auto_disc); + component_class_restrict, log_level, auto_disc, + interrupter); } static -int auto_discover_source_for_input_as_dir_or_file_rec(GString *input, +auto_source_discovery_internal_status auto_discover_source_for_input_as_dir_or_file_rec( + GString *input, uint64_t original_input_index, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc) + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter) { - int status; + auto_source_discovery_internal_status status; GError *error = NULL; if (g_file_test(input->str, G_FILE_TEST_IS_REGULAR)) { /* It's a file. */ status = support_info_query_all_sources(input->str, "file", original_input_index, plugins, plugin_count, - component_class_restrict, log_level, auto_disc); + component_class_restrict, log_level, auto_disc, + interrupter); } else if (g_file_test(input->str, G_FILE_TEST_IS_DIR)) { GDir *dir; const gchar *dirent; gsize saved_input_len; - int dir_status = 1; + int dir_status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH; /* It's a directory. */ status = support_info_query_all_sources(input->str, "directory", original_input_index, plugins, plugin_count, component_class_restrict, log_level, - auto_disc); + auto_disc, interrupter); if (status < 0) { /* Fatal error. */ goto error; - } else if (status == 0) { + } else if (status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK || + status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED) { /* * A component class claimed this input as a directory, - * don't recurse. + * don't recurse. Or, we got interrupted. */ goto end; } @@ -617,7 +638,7 @@ int auto_discover_source_for_input_as_dir_or_file_rec(GString *input, if (error->code == G_FILE_ERROR_ACCES) { /* This is not a fatal error, we just skip it. */ - status = 1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH; goto end; } else { BT_AUTODISC_LOGE_APPEND_CAUSE(fmt, input->str, @@ -637,15 +658,18 @@ int auto_discover_source_for_input_as_dir_or_file_rec(GString *input, status = auto_discover_source_for_input_as_dir_or_file_rec( input, original_input_index, plugins, plugin_count, - component_class_restrict, log_level, auto_disc); + component_class_restrict, log_level, auto_disc, + interrupter); g_string_truncate(input, saved_input_len); if (status < 0) { /* Fatal error. */ goto error; - } else if (status == 0) { - dir_status = 0; + } else if (status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED) { + goto end; + } else if (status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK) { + dir_status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK; } } else if (errno != 0) { BT_LOGW_ERRNO("Failed to read directory entry", ": dir=%s", input->str); @@ -658,13 +682,13 @@ int auto_discover_source_for_input_as_dir_or_file_rec(GString *input, g_dir_close(dir); } else { BT_LOGD("Skipping %s, not a file or directory", input->str); - status = 1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH; } goto end; error: - status = -1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_ERROR; end: @@ -684,42 +708,47 @@ end: */ static -int auto_discover_source_for_input_as_dir_or_file(const char *input, +auto_source_discovery_internal_status auto_discover_source_for_input_as_dir_or_file( + const char *input, uint64_t original_input_index, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc) + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter) { GString *mutable_input; - int status; + auto_source_discovery_internal_status status; mutable_input = g_string_new(input); if (!mutable_input) { - status = -1; + status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_ERROR; goto end; } status = auto_discover_source_for_input_as_dir_or_file_rec( mutable_input, original_input_index, plugins, plugin_count, - component_class_restrict, log_level, auto_disc); + component_class_restrict, log_level, auto_disc, + interrupter); g_string_free(mutable_input, TRUE); end: return status; } -int auto_discover_source_components( +auto_source_discovery_status auto_discover_source_components( const bt_value *inputs, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc) + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter) { uint64_t i_inputs, input_count; - int status; + auto_source_discovery_internal_status internal_status; + auto_source_discovery_status status; input_count = bt_value_array_get_length(inputs); @@ -729,24 +758,26 @@ int auto_discover_source_components( input_value = bt_value_array_borrow_element_by_index_const(inputs, i_inputs); input = bt_value_string_get(input_value); - status = auto_discover_source_for_input_as_string(input, i_inputs, + internal_status = auto_discover_source_for_input_as_string(input, i_inputs, plugins, plugin_count, component_class_restrict, - log_level, auto_disc); - if (status < 0) { - /* Fatal error. */ + log_level, auto_disc, interrupter); + if (internal_status < 0 || internal_status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED) { + /* Fatal error or we got interrupted. */ + status = internal_status; goto end; - } else if (status == 0) { + } else if (internal_status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_OK) { /* A component class has claimed this input as an arbitrary string. */ continue; } - status = auto_discover_source_for_input_as_dir_or_file(input, + internal_status = auto_discover_source_for_input_as_dir_or_file(input, i_inputs, plugins, plugin_count, - component_class_restrict, log_level, auto_disc); - if (status < 0) { - /* Fatal error. */ + component_class_restrict, log_level, auto_disc, interrupter); + if (internal_status < 0 || internal_status == AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_INTERRUPTED) { + /* Fatal error or we got interrupted. */ + status = internal_status; goto end; - } else if (status == 0) { + } else if (internal_status == 0) { /* * This input (or something under it) was recognized. */ @@ -756,7 +787,7 @@ int auto_discover_source_components( BT_LOGW("No trace was found based on input `%s`.", input); } - status = 0; + status = AUTO_SOURCE_DISCOVERY_STATUS_OK; end: return status; } diff --git a/src/autodisc/autodisc.h b/src/autodisc/autodisc.h index 09b209a4..075566c2 100644 --- a/src/autodisc/autodisc.h +++ b/src/autodisc/autodisc.h @@ -27,6 +27,10 @@ #include +#define __BT_IN_BABELTRACE_H +#include +#undef __BT_IN_BABELTRACE_H + struct auto_source_discovery { /* Array of `struct auto_source_discovery_result *`. */ GPtrArray *results; @@ -59,22 +63,28 @@ struct auto_source_discovery_result { bt_value *original_input_indices; }; +typedef enum auto_source_discovery_status { + AUTO_SOURCE_DISCOVERY_STATUS_OK = __BT_FUNC_STATUS_OK, + AUTO_SOURCE_DISCOVERY_STATUS_ERROR = __BT_FUNC_STATUS_ERROR, + AUTO_SOURCE_DISCOVERY_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, + AUTO_SOURCE_DISCOVERY_STATUS_INTERRUPTED = __BT_FUNC_STATUS_INTERRUPTED, +} auto_source_discovery_status; + int auto_source_discovery_init(struct auto_source_discovery *auto_disc); void auto_source_discovery_fini(struct auto_source_discovery *auto_disc); /* * Given `inputs` a list of strings, query source component classes to discover * which source components should be instantiated to deal with these inputs. - * - * Return 0 if execution completed successfully, < 0 otherwise. */ -int auto_discover_source_components( +auto_source_discovery_status auto_discover_source_components( const bt_value *inputs, const bt_plugin **plugins, size_t plugin_count, const char *component_class_restrict, enum bt_logging_level log_level, - struct auto_source_discovery *auto_disc); + struct auto_source_discovery *auto_disc, + const bt_interrupter *interrupter); #endif /* AUTODISC_AUTODISC_H */ diff --git a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h index eb386610..9dcacddf 100644 --- a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h +++ b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h @@ -102,7 +102,8 @@ bt_value *bt_bt2_auto_discover_source_components(const bt_value *inputs, plugin_count, NULL, bt_python_bindings_bt2_log_level, - &auto_disc); + &auto_disc, + NULL); if (status != 0) { BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN(module_name, "Failed to auto discover sources."); diff --git a/src/cli/babeltrace2-cfg-cli-args-default.c b/src/cli/babeltrace2-cfg-cli-args-default.c index 823d8436..3db36042 100644 --- a/src/cli/babeltrace2-cfg-cli-args-default.c +++ b/src/cli/babeltrace2-cfg-cli-args-default.c @@ -37,7 +37,8 @@ #ifdef BT_SET_DEFAULT_IN_TREE_CONFIGURATION struct bt_config *bt_config_cli_args_create_with_default(int argc, - const char *argv[], int *retcode) + const char *argv[], int *retcode, + const bt_interrupter *interrupter) { bt_value *initial_plugin_paths; struct bt_config *cfg = NULL; @@ -71,7 +72,7 @@ struct bt_config *bt_config_cli_args_create_with_default(int argc, #endif cfg = bt_config_cli_args_create(argc, argv, retcode, true, true, - initial_plugin_paths); + initial_plugin_paths, interrupter); goto end; error: @@ -86,10 +87,11 @@ end: #else /* BT_SET_DEFAULT_IN_TREE_CONFIGURATION */ struct bt_config *bt_config_cli_args_create_with_default(int argc, - const char *argv[], int *retcode) + const char *argv[], int *retcode, + const bt_interrupter *interrupter) { return bt_config_cli_args_create(argc, argv, retcode, false, false, - NULL); + NULL, interrupter); } #endif /* BT_SET_DEFAULT_IN_TREE_CONFIGURATION */ diff --git a/src/cli/babeltrace2-cfg-cli-args-default.h b/src/cli/babeltrace2-cfg-cli-args-default.h index 839b08c1..6245c268 100644 --- a/src/cli/babeltrace2-cfg-cli-args-default.h +++ b/src/cli/babeltrace2-cfg-cli-args-default.h @@ -28,6 +28,7 @@ #include "babeltrace2-cfg.h" struct bt_config *bt_config_cli_args_create_with_default(int argc, - const char *argv[], int *retcode); + const char *argv[], int *retcode, + const bt_interrupter *interrupter); #endif /* CLI_BABELTRACE_CFG_CLI_ARGS_DEFAULT_H */ diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index 8a9c1bf6..9544139b 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -3136,7 +3136,7 @@ enum convert_current_item_type { static struct bt_config *bt_config_convert_from_args(int argc, const char *argv[], int *retcode, const bt_value *plugin_paths, - int *default_log_level) + int *default_log_level, const bt_interrupter *interrupter) { enum convert_current_item_type current_item_type = CONVERT_CURRENT_ITEM_TYPE_NONE; @@ -4053,9 +4053,13 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[], status = auto_discover_source_components(non_opts, plugins, plugin_count, auto_source_discovery_restrict_component_class_name, - *default_log_level, &auto_disc); + *default_log_level, &auto_disc, interrupter); if (status != 0) { + if (status == AUTO_SOURCE_DISCOVERY_STATUS_INTERRUPTED) { + BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN( + "Babeltrace CLI", "Automatic source discovery interrupted by the user"); + } goto error; } @@ -4428,7 +4432,8 @@ void print_gen_usage(FILE *fp) struct bt_config *bt_config_cli_args_create(int argc, const char *argv[], int *retcode, bool omit_system_plugin_path, bool omit_home_plugin_path, - const bt_value *initial_plugin_paths) + const bt_value *initial_plugin_paths, + const bt_interrupter *interrupter) { struct bt_config *config = NULL; int i; @@ -4658,7 +4663,7 @@ struct bt_config *bt_config_cli_args_create(int argc, const char *argv[], break; case COMMAND_TYPE_CONVERT: config = bt_config_convert_from_args(command_argc, command_argv, - retcode, plugin_paths, &default_log_level); + retcode, plugin_paths, &default_log_level, interrupter); break; case COMMAND_TYPE_LIST_PLUGINS: config = bt_config_list_plugins_from_args(command_argc, diff --git a/src/cli/babeltrace2-cfg-cli-args.h b/src/cli/babeltrace2-cfg-cli-args.h index 8a458c22..e709263f 100644 --- a/src/cli/babeltrace2-cfg-cli-args.h +++ b/src/cli/babeltrace2-cfg-cli-args.h @@ -36,6 +36,7 @@ struct bt_config *bt_config_cli_args_create(int argc, const char *argv[], int *retcode, bool force_omit_system_plugin_path, bool force_omit_home_plugin_path, - const bt_value *initial_plugin_paths); + const bt_value *initial_plugin_paths, + const bt_interrupter *interrupter); #endif /* CLI_BABELTRACE_CFG_CLI_ARGS_H */ diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c index e32e1a09..de0b6c2b 100644 --- a/src/cli/babeltrace2.c +++ b/src/cli/babeltrace2.c @@ -2601,12 +2601,22 @@ int main(int argc, const char **argv) { int ret; int retcode; - struct bt_config *cfg; + struct bt_config *cfg = NULL; init_log_level(); set_signal_handler(); init_loaded_plugins(); - cfg = bt_config_cli_args_create_with_default(argc, argv, &retcode); + + BT_ASSERT(!the_interrupter); + the_interrupter = bt_interrupter_create(); + if (!the_interrupter) { + BT_CLI_LOGE_APPEND_CAUSE("Failed to create an interrupter object."); + retcode = 1; + goto end; + } + + cfg = bt_config_cli_args_create_with_default(argc, argv, &retcode, + the_interrupter); if (retcode < 0) { /* Quit without errors; typically usage/version */ @@ -2640,14 +2650,6 @@ int main(int argc, const char **argv) } } - BT_ASSERT(!the_interrupter); - the_interrupter = bt_interrupter_create(); - if (!the_interrupter) { - BT_CLI_LOGE_APPEND_CAUSE("Failed to create an interrupter object."); - retcode = 1; - goto end; - } - BT_LOGI("Executing command: cmd=%d, command-name=\"%s\"", cfg->command, cfg->command_name); -- 2.34.1