From: Francis Deslauriers Date: Thu, 25 Feb 2021 23:19:37 +0000 (-0500) Subject: Fix: mark channel as disabled even if the session is inactive X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=fbee89873ae51fa4e025455b19a8ebccd0e2d8a4;p=lttng-tools.git Fix: mark channel as disabled even if the session is inactive Observed issue ============== When the session is stopped, disable-channel commands are no-op. The following commands reproduce the issue: lttng create lttng enable-event -u -a lttng start sleep 4 lttng stop lttng disable-channel -u channel0 sleep 10 lttng start sleep 4 lttng stop lttng view Note that the sleep command there are to give the UST application time to produce events. Even after disabling the channel, we can see that events are still traced. This is due to the fact that the `channel_ust_disable()` function returns early if the session is inactive and omits to set the channel as disabled. Proposed fix ============ Move this following line before the check: uchan->enabled = 0; Test ==== Add a test case to exercise this exact scenario. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: I9660064ac6eb99f2aea8851dc98a94cfc1b810e6 --- diff --git a/configure.ac b/configure.ac index c6e1de95e..137d956b8 100644 --- a/configure.ac +++ b/configure.ac @@ -1151,6 +1151,7 @@ AC_CONFIG_FILES([ tests/regression/tools/save-load/configuration/Makefile tests/regression/tools/mi/Makefile tests/regression/tools/wildcard/Makefile + tests/regression/tools/channel/Makefile tests/regression/tools/crash/Makefile tests/regression/tools/regen-metadata/Makefile tests/regression/tools/regen-statedump/Makefile diff --git a/src/bin/lttng-sessiond/channel.c b/src/bin/lttng-sessiond/channel.c index 2beb4f711..073965ac6 100644 --- a/src/bin/lttng-sessiond/channel.c +++ b/src/bin/lttng-sessiond/channel.c @@ -522,6 +522,13 @@ int channel_ust_disable(struct ltt_ust_session *usess, DBG2("Channel UST %s already disabled", uchan->name); goto end; } + + uchan->enabled = 0; + + /* + * If session is inactive we don't notify the tracer right away. We + * wait for the next synchronization. + */ if (!usess->active) { goto end; } @@ -534,8 +541,6 @@ int channel_ust_disable(struct ltt_ust_session *usess, goto error; } - uchan->enabled = 0; - DBG2("Channel %s disabled successfully", uchan->name); return LTTNG_OK; diff --git a/tests/regression/Makefile.am b/tests/regression/Makefile.am index 6b950a58b..55e56c291 100644 --- a/tests/regression/Makefile.am +++ b/tests/regression/Makefile.am @@ -7,6 +7,7 @@ LOG_DRIVER = env PGREP='$(PGREP)' AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/config/tap-driver.sh TESTS = tools/base-path/test_ust \ + tools/channel/test_channel \ tools/filtering/test_invalid_filter \ tools/filtering/test_unsupported_op \ tools/filtering/test_valid_filter \ diff --git a/tests/regression/tools/Makefile.am b/tests/regression/tools/Makefile.am index d561a6479..fc39333c0 100644 --- a/tests/regression/tools/Makefile.am +++ b/tests/regression/tools/Makefile.am @@ -1,5 +1,25 @@ # SPDX-License-Identifier: GPL-2.0-only -SUBDIRS = streaming filtering health tracefile-limits snapshots live exclusion save-load mi \ - wildcard crash regen-metadata regen-statedump notification rotation \ - base-path metadata working-directory relayd-grouping clear tracker trigger +SUBDIRS = base-path \ + channel \ + clear \ + crash \ + exclusion \ + filtering \ + health \ + live \ + metadata \ + mi \ + notification \ + regen-metadata \ + regen-statedump \ + relayd-grouping \ + rotation \ + save-load \ + snapshots \ + streaming \ + tracefile-limits \ + tracker \ + trigger \ + wildcard \ + working-directory diff --git a/tests/regression/tools/channel/Makefile.am b/tests/regression/tools/channel/Makefile.am new file mode 100644 index 000000000..1f116a69d --- /dev/null +++ b/tests/regression/tools/channel/Makefile.am @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0-only + +AM_CPPFLAGS += -I$(top_srcdir)/tests -I$(srcdir) + +noinst_SCRIPTS = test_channel +EXTRA_DIST = test_channel + +all-local: + @if [ x"$(srcdir)" != x"$(builddir)" ]; then \ + for script in $(EXTRA_DIST); do \ + cp -f $(srcdir)/$$script $(builddir); \ + done; \ + fi + +clean-local: + @if [ x"$(srcdir)" != x"$(builddir)" ]; then \ + for script in $(EXTRA_DIST); do \ + rm -f $(builddir)/$$script; \ + done; \ + fi diff --git a/tests/regression/tools/channel/test_channel b/tests/regression/tools/channel/test_channel new file mode 100755 index 000000000..d4ffd56be --- /dev/null +++ b/tests/regression/tools/channel/test_channel @@ -0,0 +1,75 @@ +#!/bin/bash +# +# Copyright (C) 2021 Francis Deslauriers +# +# SPDX-License-Identifier: GPL-2.0-only + +TEST_DESC="LTTng - Channel tests" + +CURDIR=$(dirname $0)/ +TESTDIR=$CURDIR/../../.. +TESTAPP_PATH="$TESTDIR/utils/testapp" +TESTAPP_NAME="gen-ust-events" +TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME" +NR_ITER=100 +NR_USEC_WAIT=1 +NUM_TESTS=14 + +source $TESTDIR/utils/utils.sh + +function test_channel_disable_stopped_session() +{ + local TRACE_PATH=$(mktemp -d) + local SESSION_NAME="test_channel" + local CHAN_NAME="channel0" + local EVENT_NAME="tp:tptest" + + diag "Test channel disable on stop session" + + create_lttng_session_ok "$SESSION_NAME" "$TRACE_PATH" + + enable_ust_lttng_channel_ok "$SESSION_NAME" "$CHAN_NAME" + + enable_ust_lttng_event_ok "$SESSION_NAME" "$EVENT_NAME" "$CHAN_NAME" + + start_lttng_tracing_ok "$SESSION_NAME" + + "$TESTAPP_BIN" -i "$NR_ITER" -w "$NR_USEC_WAIT" + + stop_lttng_tracing_ok "$SESSION_NAME" + + trace_match_only "$EVENT_NAME" "$NR_ITER" "$TRACE_PATH" + + lttng_clear_session_ok "$SESSION_NAME" + + disable_ust_lttng_channel "$SESSION_NAME" "$CHAN_NAME" + + start_lttng_tracing_ok "$SESSION_NAME" + + # The channel is disabled so no events should be emited by this app. + "$TESTAPP_BIN" -i "$NR_ITER" -w "$NR_USEC_WAIT" + + stop_lttng_tracing_ok "$SESSION_NAME" + + trace_match_only "$EVENT_NAME" 0 "$TRACE_PATH" + + destroy_lttng_session_ok $SESSION_NAME + + rm -rf $TRACE_PATH +} + + +# MUST set TESTDIR before calling those functions +plan_tests $NUM_TESTS + +print_test_banner "$TEST_DESC" + +start_lttng_sessiond + +if [ ! -x "$TESTAPP_BIN" ]; then + BAIL_OUT "No UST nevents binary detected." +fi + +test_channel_disable_stopped_session + +stop_lttng_sessiond