From c80d4c65ee2320f014281e979bb732ad1bf14ce9 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 2 May 2019 12:21:20 -0400 Subject: [PATCH] src.ctf.fs: make trace-info query accept clock-class-offset-{s,ns} parameters The src.ctf.fs components accept clock-class-offset-s and clock-class-offset-ns parameters, allowing to add an offset to the clock. The trace-info query currently doesn't accept the same parameters. In general, I think we'll want the components and trace-info to accept pretty much the same set of parameters, so that the information reported by trace-info accurately represents how an eventual component reading the same traces will work. This patch splits the parameter handling of the component in a standalone function re-used by the query. The advantage of sharing a function like this is that if we add a parameter to the component, we'll be forced to think about what happens to the trace-info query. One behavior change is that trace_info_query now returns BT_QUERY_STATUS_INVALID_PARAMS if the passed parameters are invalid. A test is added to test the new functionality. Change-Id: I3d917b8713fce4ce56a79a6e7c7bdc3e32d9ec0c Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/1244 Reviewed-by: Philippe Proulx --- configure.ac | 2 + plugins/ctf/fs-src/fs.c | 80 ++++++++----- plugins/ctf/fs-src/fs.h | 14 ++- plugins/ctf/fs-src/query.c | 8 +- tests/plugins/Makefile.am | 2 + tests/plugins/ctf/Makefile.am | 7 ++ tests/plugins/ctf/test_ctf_plugin.in | 25 +++++ tests/plugins/ctf/test_query_trace_info.py | 125 +++++++++++++++++++++ 8 files changed, 231 insertions(+), 32 deletions(-) create mode 100644 tests/plugins/ctf/Makefile.am create mode 100644 tests/plugins/ctf/test_ctf_plugin.in create mode 100644 tests/plugins/ctf/test_query_trace_info.py diff --git a/configure.ac b/configure.ac index d23373bb..bb1d967c 100644 --- a/configure.ac +++ b/configure.ac @@ -762,6 +762,7 @@ AC_CONFIG_FILES([ tests/bindings/python/Makefile tests/bindings/python/bt2/Makefile tests/plugins/Makefile + tests/plugins/ctf/Makefile tests/python-plugin-provider/Makefile extras/Makefile extras/valgrind/Makefile @@ -800,6 +801,7 @@ AC_CONFIG_FILES([tests/lib/test_ctf_writer_complete], [chmod +x tests/lib/test_c AC_CONFIG_FILES([tests/lib/test_plugin_complete], [chmod +x tests/lib/test_plugin_complete]) AC_CONFIG_FILES([tests/lib/trace-ir/test_trace_ir], [chmod +x tests/lib/trace-ir/test_trace_ir]) AC_CONFIG_FILES([tests/lib/ctf-writer/test_ctf_writer], [chmod +x tests/lib/ctf-writer/test_ctf_writer]) +AC_CONFIG_FILES([tests/plugins/ctf/test_ctf_plugin], [chmod +x tests/plugins/ctf/test_ctf_plugin]) AC_CONFIG_FILES([tests/plugins/test-utils-muxer-complete], [chmod +x tests/plugins/test-utils-muxer-complete]) AC_CONFIG_FILES([tests/plugins/test_lttng_utils_debug_info], [chmod +x tests/plugins/test_lttng_utils_debug_info]) AC_CONFIG_FILES([tests/plugins/test_dwarf_complete], [chmod +x tests/plugins/test_dwarf_complete]) diff --git a/plugins/ctf/fs-src/fs.c b/plugins/ctf/fs-src/fs.c index 4ebd4efd..6672071f 100644 --- a/plugins/ctf/fs-src/fs.c +++ b/plugins/ctf/fs-src/fs.c @@ -1659,6 +1659,12 @@ end: return ret; } +/* + * Validate the "paths" parameter passed to this component. It must be + * present, and it must be an array of strings. + */ + +static bool validate_paths_parameter(const bt_value *paths) { bool ret; @@ -1699,23 +1705,67 @@ end: return ret; } +bool read_src_fs_parameters(const bt_value *params, + const bt_value **paths, struct ctf_fs_component *ctf_fs) { + bool ret; + const bt_value *value; + + /* paths parameter */ + *paths = bt_value_map_borrow_entry_value_const(params, "paths"); + if (!validate_paths_parameter(*paths)) { + goto error; + } + + /* clock-class-offset-s parameter */ + value = bt_value_map_borrow_entry_value_const(params, + "clock-class-offset-s"); + if (value) { + if (!bt_value_is_integer(value)) { + BT_LOGE("clock-class-offset-s must be an integer"); + goto error; + } + ctf_fs->metadata_config.clock_class_offset_s = + bt_value_integer_get(value); + } + + /* clock-class-offset-ns parameter */ + value = bt_value_map_borrow_entry_value_const(params, + "clock-class-offset-ns"); + if (value) { + if (!bt_value_is_integer(value)) { + BT_LOGE("clock-class-offset-ns must be an integer"); + goto error; + } + ctf_fs->metadata_config.clock_class_offset_ns = + bt_value_integer_get(value); + } + + + ret = true; + goto end; + +error: + ret = false; + +end: + return ret; +} + static struct ctf_fs_component *ctf_fs_create( bt_self_component_source *self_comp, const bt_value *params) { struct ctf_fs_component *ctf_fs = NULL; - const bt_value *value = NULL; guint i; const bt_value *paths_value; - paths_value = bt_value_map_borrow_entry_value_const(params, "paths"); - if (!validate_paths_parameter(paths_value)) { + ctf_fs = ctf_fs_component_create(); + if (!ctf_fs) { goto error; } - ctf_fs = ctf_fs_component_create(); - if (!ctf_fs) { + if (!read_src_fs_parameters(params, &paths_value, ctf_fs)) { goto error; } @@ -1730,26 +1780,6 @@ struct ctf_fs_component *ctf_fs_create( */ ctf_fs->self_comp = self_comp; - value = bt_value_map_borrow_entry_value_const(params, - "clock-class-offset-s"); - if (value) { - if (!bt_value_is_integer(value)) { - BT_LOGE("clock-class-offset-s should be an integer"); - goto error; - } - ctf_fs->metadata_config.clock_class_offset_s = bt_value_integer_get(value); - } - - value = bt_value_map_borrow_entry_value_const(params, - "clock-class-offset-ns"); - if (value) { - if (!bt_value_is_integer(value)) { - BT_LOGE("clock-class-offset-ns should be an integer"); - goto error; - } - ctf_fs->metadata_config.clock_class_offset_ns = bt_value_integer_get(value); - } - if (ctf_fs_component_create_ctf_fs_traces(self_comp, ctf_fs, paths_value)) { goto error; } diff --git a/plugins/ctf/fs-src/fs.h b/plugins/ctf/fs-src/fs.h index 1ee4a682..6594b611 100644 --- a/plugins/ctf/fs-src/fs.h +++ b/plugins/ctf/fs-src/fs.h @@ -204,10 +204,18 @@ int ctf_fs_component_create_ctf_fs_traces(bt_self_component_source *self_comp, BT_HIDDEN void ctf_fs_destroy(struct ctf_fs_component *ctf_fs); -/* Validate the "paths" parameter passed to this component. It must be - present, and it must be an array of strings. */ +/* + * Read and validate parameters taken by the src.ctf.fs plugin. + * + * - The mandatory `paths` parameter is returned in `*paths`. + * - The optional `clock-class-offset-s` and `clock-class-offset-ns`, if + * present, are recorded in the `ctf_fs` structure. + * + * Return true on success, false if any parameter didn't pass validation. + */ BT_HIDDEN -bool validate_paths_parameter(const bt_value *paths); +bool read_src_fs_parameters(const bt_value *params, + const bt_value **paths, struct ctf_fs_component *ctf_fs); #endif /* BABELTRACE_PLUGIN_CTF_FS_H */ diff --git a/plugins/ctf/fs-src/query.c b/plugins/ctf/fs-src/query.c index b56997c6..d632d42d 100644 --- a/plugins/ctf/fs-src/query.c +++ b/plugins/ctf/fs-src/query.c @@ -484,13 +484,13 @@ bt_query_status trace_info_query( goto error; } - paths_value = bt_value_map_borrow_entry_value_const(params, "paths"); - if (!validate_paths_parameter(paths_value)) { + ctf_fs = ctf_fs_component_create(); + if (!ctf_fs) { goto error; } - ctf_fs = ctf_fs_component_create(); - if (!ctf_fs) { + if (!read_src_fs_parameters(params, &paths_value, ctf_fs)) { + status = BT_QUERY_STATUS_INVALID_PARAMS; goto error; } diff --git a/tests/plugins/Makefile.am b/tests/plugins/Makefile.am index 63a2f72b..a4055ddd 100644 --- a/tests/plugins/Makefile.am +++ b/tests/plugins/Makefile.am @@ -1,3 +1,5 @@ +SUBDIRS = ctf + AM_CPPFLAGS += -I$(top_srcdir)/tests/utils -I$(top_srcdir)/plugins LIBTAP=$(top_builddir)/tests/utils/tap/libtap.la diff --git a/tests/plugins/ctf/Makefile.am b/tests/plugins/ctf/Makefile.am new file mode 100644 index 00000000..7ea14a44 --- /dev/null +++ b/tests/plugins/ctf/Makefile.am @@ -0,0 +1,7 @@ +if ENABLE_PYTHON_BINDINGS + +TESTS = test_ctf_plugin + +endif + +dist_check_SCRIPTS = test_query_trace_info.py diff --git a/tests/plugins/ctf/test_ctf_plugin.in b/tests/plugins/ctf/test_ctf_plugin.in new file mode 100644 index 00000000..39164451 --- /dev/null +++ b/tests/plugins/ctf/test_ctf_plugin.in @@ -0,0 +1,25 @@ +#!/bin/bash +# +# Copyright (C) 2019 Simon Marchi +# +# 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. + +NO_SH_TAP=1 +. "@abs_top_builddir@/tests/utils/common.sh" + +TESTRUNNER_PY="$BT_SRC_PATH/tests/utils/python/testrunner.py" +THIS_DIR="$BT_SRC_PATH/tests/plugins/ctf" + +exec "$BT_BUILD_PATH/tests/utils/test_python_bt2_env" python3 "$TESTRUNNER_PY" "$THIS_DIR" diff --git a/tests/plugins/ctf/test_query_trace_info.py b/tests/plugins/ctf/test_query_trace_info.py new file mode 100644 index 00000000..c8b5923c --- /dev/null +++ b/tests/plugins/ctf/test_query_trace_info.py @@ -0,0 +1,125 @@ +# Copyright (C) 2019 Simon Marchi +# +# 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. + +import unittest +import bt2 +import os + + +test_ctf_traces_path = os.environ['TEST_CTF_TRACES_PATH'] + + +# Key to streams by their being timestamp. Used to get the list of streams in +# a predictable order. + +def sort_by_begin(stream): + return stream['range-ns']['begin'] + + +class QueryTraceInfoClockOffsetTestCase(unittest.TestCase): + + def setUp(self): + ctf = bt2.find_plugin('ctf') + self._fs = ctf.source_component_classes['fs'] + + self._paths = [os.path.join(test_ctf_traces_path, 'intersection', '3eventsintersect')] + self._executor = bt2.QueryExecutor() + + def _check(self, trace, offset): + self.assertEqual(trace['range-ns']['begin'], 13515309000000000 + offset) + self.assertEqual(trace['range-ns']['end'], 13515309000000120 + offset) + self.assertEqual(trace['intersection-range-ns']['begin'], 13515309000000070 + offset) + self.assertEqual(trace['intersection-range-ns']['end'], 13515309000000100 + offset) + + streams = sorted(trace['streams'], key=sort_by_begin) + self.assertEqual(streams[0]['range-ns']['begin'], 13515309000000000 + offset) + self.assertEqual(streams[0]['range-ns']['end'], 13515309000000100 + offset) + self.assertEqual(streams[1]['range-ns']['begin'], 13515309000000070 + offset) + self.assertEqual(streams[1]['range-ns']['end'], 13515309000000120 + offset) + + # Test various cominations of the clock-class-offset-s and + # clock-class-offset-ns parameters to trace-info queries. + + # Without clock class offset + + def test_no_clock_class_offset(self): + res = self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + }) + trace = res[0] + self._check(trace, 0) + + # With clock-class-offset-s + + def test_clock_class_offset_s(self): + res = self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-s': 2, + }) + trace = res[0] + self._check(trace, 2000000000) + + # With clock-class-offset-ns + + def test_clock_class_offset_ns(self): + res = self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-ns': 2, + }) + trace = res[0] + self._check(trace, 2) + + # With both, negative + + def test_clock_class_offset_both(self): + res = self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-s': -2, + 'clock-class-offset-ns': -2, + }) + trace = res[0] + self._check(trace, -2000000002) + + def test_clock_class_offset_s_wrong_type(self): + with self.assertRaises(bt2.InvalidQueryParams): + self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-s': "2", + }) + + def test_clock_class_offset_s_wrong_type_none(self): + with self.assertRaises(bt2.InvalidQueryParams): + self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-s': None, + }) + + def test_clock_class_offset_ns_wrong_type(self): + with self.assertRaises(bt2.InvalidQueryParams): + self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-ns': "2", + }) + + def test_clock_class_offset_ns_wrong_type_none(self): + with self.assertRaises(bt2.InvalidQueryParams): + self._executor.query(self._fs, 'trace-info', { + 'paths': self._paths, + 'clock-class-offset-ns': None, + }) + +if __name__ == '__main__': + unittest.main() -- 2.34.1