From: Simon Marchi Date: Thu, 17 Oct 2019 19:52:07 +0000 (-0400) Subject: src.ctf.fs: error out when failing to create index X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=29a8227a1b5d08e0b600a2575372a3b5ae232d21 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 Change-Id: I5320102352eb8ef5004f0555dec2cc2736297a4f --- diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index ee322c0e..09730401 100644 --- 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 ba9718d6..d1a1023d 100644 --- 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 542b06c8..c4b0a0a5 100644 --- 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 index 00000000..e275e81d --- /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 index 00000000..7682714b 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 index 00000000..c8f306cf 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 index 00000000..46d334f1 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 5f61b24d..7aacb3eb 100644 --- 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 index 00000000..7f8a9515 --- /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}"