src.ctf.fs: error out when failing to create index

This patch makes src.ctf.fs fail if it is not able to build an index for
a data stream file.  This is generally a sign of a corrupted trace.

The plan is to be strict by default (abort if we find the trace looks
corrupted), but to later introduce a parameter to tell the component to
make those checks not fatal.

So for now, we can assume that the index will always be present, hence
the comment change in fs.h.

Without this patch, the trace provided as a test leads to this abort:

    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_idx_file@data-stream-file.c:462 [auto-disc-source-ctf-fs] Invalid LTTng trace index file; indexed size != stream file size: file-size=16699392, total-packets-size=16896000
    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_stream_file@data-stream-file.c:596 [auto-disc-source-ctf-fs] Invalid packet size reported in file: stream="/tmp/single/ch-1_17", packet-offset=12701696, packet-size-bytes=4194304, file-size=16699392
    10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS add_ds_file_to_ds_file_group@fs.c:789 [auto-disc-source-ctf-fs] Failed to index CTF stream file '/tmp/single/ch-1_17'

     (╯°□°)╯︵ ┻━┻  /home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/fs.c:1572: fix_index_lttng_event_after_packet_bug(): Assertion `index` failed.
    [1]    14522 abort      ./src/cli/babeltrace2 /tmp/single

The reason is that the index fails to build (the cumulative packet sizes
is larger than the data stream file size), but we keep going anyway.
The fix_index_lttng_event_after_packet_bug function, expecting an index
to always be present, hits that assert.

I added the problematic trace to our test trace collection, and a new
test script "test_fail" to test how the src.ctf.fs component class
handles invalid traces.

Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I5320102352eb8ef5004f0555dec2cc2736297a4f

diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c
index ee322c0e50851d1b4e107f977776fe974c05f38b..097304012ba5f4e35a01796e6c3bbdf78663eef5 100644 (file)
--- a/src/plugins/ctf/fs-src/fs.c
+++ b/src/plugins/ctf/fs-src/fs.c
@@ -785,8 +785,11 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace,
 
        index = ctf_fs_ds_file_build_index(ds_file, ds_file_info);
        if (!index) {
-               BT_COMP_LOGW("Failed to index CTF stream file \'%s\'",
+               BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE(
+                       self_comp, self_comp_class,
+                       "Failed to index CTF stream file \'%s\'",
                        ds_file->file->path->str);
+               goto error;
        }
 
        if (begin_ns == -1) {

diff --git a/src/plugins/ctf/fs-src/fs.h b/src/plugins/ctf/fs-src/fs.h
index ba9718d689cdb5c0827c02ba81e251cfd213d5a6..d1a1023d3b9a58e7117a4896b64a091b6f20b7f1 100644 (file)
--- a/src/plugins/ctf/fs-src/fs.h
+++ b/src/plugins/ctf/fs-src/fs.h
@@ -161,12 +161,7 @@ struct ctf_fs_ds_file_group {
        struct ctf_fs_trace *ctf_fs_trace;
 
        /*
-        * Owned by this. May be NULL.
-        *
-        * A stream cannot be assumed to be indexed as the indexing might have
-        * been skipped. Moreover, the index's fields may not all be available
-        * depending on the producer (e.g. timestamp_begin/end are not
-        * mandatory).
+        * Owned by this.
         */
        struct ctf_fs_ds_index *index;
 };

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 542b06c8ba428977bb3f3f5e4e4efd38aa6a4b68..c4b0a0a507a4e2351e01d325d3cfd20a75b4f2b8 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -92,6 +92,7 @@ TESTS_LIB += lib/test_plugin
 endif
 
 TESTS_PLUGINS = \
+       plugins/src.ctf.fs/fail/test_fail \
        plugins/src.ctf.fs/succeed/test_succeed \
        plugins/sink.ctf.fs/succeed/test_succeed \
        plugins/sink.text.details/succeed/test_succeed

diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/README b/tests/data/ctf-traces/fail/invalid-packet-size/README
new file mode 100644 (file)
index 0000000..e275e81
--- /dev/null
+++ b/tests/data/ctf-traces/fail/invalid-packet-size/README
@@ -0,0 +1,2 @@
+The sum of the packet sizes in this trace (both in the index file and in the
+actual packet headers) is larger than the data file.

diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3 b/tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3
new file mode 100644 (file)
index 0000000..7682714
Binary files /dev/null and b/tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3 differ

diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx b/tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx
new file mode 100644 (file)
index 0000000..c8f306c
Binary files /dev/null and b/tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx differ

diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata b/tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata
new file mode 100644 (file)
index 0000000..46d334f
Binary files /dev/null and b/tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata differ

diff --git a/tests/plugins/src.ctf.fs/Makefile.am b/tests/plugins/src.ctf.fs/Makefile.am
index 5f61b24d6cd7600caee90c6b26d14f8000f2c3b5..7aacb3eba40bfda727adc61c95c5b52a7977bec9 100644 (file)
--- a/tests/plugins/src.ctf.fs/Makefile.am
+++ b/tests/plugins/src.ctf.fs/Makefile.am
@@ -1,6 +1,7 @@
 SUBDIRS = succeed
 
 dist_check_SCRIPTS = \
+       fail/test_fail \
        query/test_query_metadata_info \
        query/test_query_support_info \
        query/test_query_support_info.py \

diff --git a/tests/plugins/src.ctf.fs/fail/test_fail b/tests/plugins/src.ctf.fs/fail/test_fail
new file mode 100755 (executable)
index 0000000..7f8a951
--- /dev/null
+++ b/tests/plugins/src.ctf.fs/fail/test_fail
@@ -0,0 +1,55 @@
+#!/bin/bash
+#
+# Copyright (C) 2019 EfficiOS Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; only version 2
+# of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+
+# This test validates that a `src.ctf.fs` component handles gracefully invalid
+# CTF traces and produces the expected error message.
+
+SH_TAP=1
+
+if [ "x${BT_TESTS_SRCDIR:-}" != "x" ]; then
+       UTILSSH="$BT_TESTS_SRCDIR/utils/utils.sh"
+else
+       UTILSSH="$(dirname "$0")/../../../utils/utils.sh"
+fi
+
+# shellcheck source=../../../utils/utils.sh
+source "$UTILSSH"
+
+fail_trace_dir="$BT_CTF_TRACES_PATH/fail"
+
+stdout_file=$(mktemp -t test_ctf_fail_stdout.XXXXXX)
+stderr_file=$(mktemp -t test_ctf_fail_stderr.XXXXXX)
+
+test_fail() {
+       local name="$1"
+       local expected_error_msg="$2"
+
+       bt_cli "${stdout_file}" "${stderr_file}" \
+               "${fail_trace_dir}/${name}"
+       isnt $? 0 "Trace ${name}: babeltrace exits with an error"
+
+       grep --silent "${expected_error_msg}" "${stderr_file}"
+       ok $? "Trace ${name}: babeltrace produces the expected error message"
+}
+
+
+plan_tests 2
+
+test_fail "invalid-packet-size/trace" "Failed to index CTF stream file '.*channel0_3'"
+
+rm -f "${stdout_file}" "${stderr_file}"