autodisc: make it possible to interrupt auto source discovery
authorSimon Marchi <simon.marchi@efficios.com>
Tue, 20 Aug 2019 00:56:26 +0000 (20:56 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Mon, 26 Aug 2019 20:02:11 +0000 (16:02 -0400)
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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1972
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
src/autodisc/autodisc.c
src/autodisc/autodisc.h
src/bindings/python/bt2/bt2/native_bt_autodisc.i.h
src/cli/babeltrace2-cfg-cli-args-default.c
src/cli/babeltrace2-cfg-cli-args-default.h
src/cli/babeltrace2-cfg-cli-args.c
src/cli/babeltrace2-cfg-cli-args.h
src/cli/babeltrace2.c

index 02bdacc5542e4e26fefc3d2c18402bdfaa6fd153..6ba393e8f5323f01e157c37ac5ab63c380cca7e7 100644 (file)
 #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;
 }
index 09b209a41e50f25255acae99a9e698ccb6456d54..075566c28a9db5ec2afced8efb969e1ed63ce8c2 100644 (file)
 
 #include <babeltrace2/babeltrace.h>
 
+#define __BT_IN_BABELTRACE_H
+#include <babeltrace2/func-status.h>
+#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 */
index eb386610841f4dfe5d570bafe9d2edea1d92386f..9dcacddf0c52eb4201dfab50e67ba2135f639bb0 100644 (file)
@@ -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.");
index 823d8436d707ee9f1c61c28d1660a479172c4777..3db3604203a29b449801b5115f461fb9d431142d 100644 (file)
@@ -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 */
index 839b08c10345607ce379f463250b6f5a190b52de..6245c268d371d0b20f82fc1aefa4237618393d85 100644 (file)
@@ -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 */
index 8a9c1bf6d6e705ca3295bdfc454794daa6ea9fc5..9544139b7f942609e0c0d4e5af7d6a6f8e409254 100644 (file)
@@ -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,
index 8a458c22a3ad14897802b2a1d937736c0efbd6ce..e709263fbff86e9bd896ea37d54b901c3487ca62 100644 (file)
@@ -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 */
index e32e1a0930f63160c3dbff47f2cea5305dd77dbc..de0b6c2b7ef99c41e752fa4222c88c50bad00f83 100644 (file)
@@ -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);
 
This page took 0.034454 seconds and 4 git commands to generate.