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 <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9660064ac6eb99f2aea8851dc98a94cfc1b810e6

diff --git a/configure.ac b/configure.ac
index c6e1de95e071cb69f5fab86058cb88b47c3ab597..137d956b8d76143cd151a43cceb588d69cb1d934 100644 (file)
--- 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 2beb4f711e6a60adbcafec0f9ab6c269de805234..073965ac67bd708e6184915d62431f5342312675 100644 (file)
--- 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 6b950a58b179b38de472ad815f663a23d0a3ff29..55e56c291e963f06b962ae06712078c2173421ff 100644 (file)
--- 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 d561a647959e6a12d3769379fbdb84a33a523aaa..fc39333c01a9a8bdcb500db222069bc79638c517 100644 (file)
--- 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 (file)
index 0000000..1f116a6
--- /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 (executable)
index 0000000..d4ffd56
--- /dev/null
+++ b/tests/regression/tools/channel/test_channel
@@ -0,0 +1,75 @@
+#!/bin/bash
+#
+# Copyright (C) 2021 Francis Deslauriers <francis.deslauriers@efficios.com>
+#
+# 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