[lttng-dev] [PATCH lttng-tools 7/9] epoll/poll compat: expose interruptible API

[lttng-dev] [PATCH lttng-tools 7/9] epoll/poll compat: expose interruptible API

[Jonathan Rajotte-Julien]

Hi,

This patch does not apply.

Please rebase.

Cheers

On Fri, May 03, 2019 at 09:55:45AM -0400, Mathieu Desnoyers wrote:
> Some use of the epoll/poll wrapper require interruption
> by signals to make the poll call return -1, errno EINTR.
> Expose a new lttng_poll_wait_interruptible API for this
> purpose.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> CC: Yannick Lamarre <ylamarre at efficios.com>
> ---
>  src/common/compat/compat-epoll.c |  9 +++++----
>  src/common/compat/compat-poll.c  |  9 +++++----
>  src/common/compat/poll.h         | 12 ++++++++----
>  3 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/src/common/compat/compat-epoll.c b/src/common/compat/compat-epoll.c
> index 6a781c7a..7108b717 100644
> --- a/src/common/compat/compat-epoll.c
> +++ b/src/common/compat/compat-epoll.c
> @@ -241,7 +241,7 @@ error:
>  /*
>   * Wait on epoll set. This is a blocking call of timeout value.
>   */
> -int compat_epoll_wait(struct lttng_poll_event *events, int timeout)
> +int compat_epoll_wait(struct lttng_poll_event *events, int timeout, int interruptible)
>  {
>  	int ret;
>  	uint32_t new_size;
> @@ -273,10 +273,11 @@ int compat_epoll_wait(struct lttng_poll_event *events, int timeout)
>  
>  	do {
>  		ret = epoll_wait(events->epfd, events->events, events->nb_fd, timeout);
> -	} while (ret == -1 && errno == EINTR);
> +	} while (!interruptible && ret == -1 && errno == EINTR);
>  	if (ret < 0) {
> -		/* At this point, every error is fatal */
> -		PERROR("epoll_wait");
> +		if (errno != EINTR) {
> +			PERROR("epoll_wait");
> +		}
>  		goto error;
>  	}
>  
> diff --git a/src/common/compat/compat-poll.c b/src/common/compat/compat-poll.c
> index b45b39dc..cdb6f8b5 100644
> --- a/src/common/compat/compat-poll.c
> +++ b/src/common/compat/compat-poll.c
> @@ -281,7 +281,7 @@ error:
>  /*
>   * Wait on poll() with timeout. Blocking call.
>   */
> -int compat_poll_wait(struct lttng_poll_event *events, int timeout)
> +int compat_poll_wait(struct lttng_poll_event *events, int timeout, int interruptible)
>  {
>  	int ret;
>  
> @@ -308,10 +308,11 @@ int compat_poll_wait(struct lttng_poll_event *events, int timeout)
>  
>  	do {
>  		ret = poll(events->wait.events, events->wait.nb_fd, timeout);
> -	} while (ret == -1 && errno == EINTR);
> +	} while (!interruptible && ret == -1 && errno == EINTR);
>  	if (ret < 0) {
> -		/* At this point, every error is fatal */
> -		PERROR("poll wait");
> +		if (errno != EINTR) {
> +			PERROR("poll wait");
> +		}
>  		goto error;
>  	}
>  
> diff --git a/src/common/compat/poll.h b/src/common/compat/poll.h
> index d4bd87f5..5400e5b1 100644
> --- a/src/common/compat/poll.h
> +++ b/src/common/compat/poll.h
> @@ -152,9 +152,11 @@ static inline int compat_glibc_epoll_create(int size, int flags)
>   * Wait on epoll set with the number of fd registered to the lttng_poll_event
>   * data structure (events).
>   */
> -extern int compat_epoll_wait(struct lttng_poll_event *events, int timeout);
> +extern int compat_epoll_wait(struct lttng_poll_event *events, int timeout, int interruptible);
>  #define lttng_poll_wait(events, timeout) \
> -	compat_epoll_wait(events, timeout)
> +	compat_epoll_wait(events, timeout, 0)
> +#define lttng_poll_wait_interruptible(events, timeout) \
> +	compat_epoll_wait(events, timeout, 1)
>  
>  /*
>   * Add a fd to the epoll set and resize the epoll_event structure if needed.
> @@ -334,9 +336,11 @@ extern int compat_poll_create(struct lttng_poll_event *events, int size);
>   * Wait on poll(2) event with nb_fd registered to the lttng_poll_event data
>   * structure.
>   */
> -extern int compat_poll_wait(struct lttng_poll_event *events, int timeout);
> +extern int compat_poll_wait(struct lttng_poll_event *events, int timeout, int interruptible);
>  #define lttng_poll_wait(events, timeout) \
> -	compat_poll_wait(events, timeout)
> +	compat_poll_wait(events, timeout, 0)
> +#define lttng_poll_wait_interruptible(events, timeout) \
> +	compat_poll_wait(events, timeout, 1)
>  
>  /*
>   * Add the fd to the pollfd structure. Resize if needed.
> -- 
> 2.11.0
> 

-- 
Jonathan Rajotte-Julien
EfficiOS

[lttng-dev] [PATCH lttng-tools 8/9] lttng-ctl: notifications: use epoll()/poll() instead of select()

[Jonathan Rajotte-Julien]

On Fri, May 03, 2019 at 09:55:46AM -0400, Mathieu Desnoyers wrote:
> The select(2) system call is an ancient ABI limited to processes
> containing at most FD_SETSIZE file descriptors overall (typically
> 1024).
> 
> Those notification APIs will fail if the target file descriptor
> is above FD_SETSIZE in a process containing many file descriptors.
> 
> Never use select, use the lttng epoll/poll wrapper instead.
> 
> This patch depends on "Change lttng_poll_wait behaviour of compat-poll
> to match compat-epoll" posted by Yannick Lamarre.

Please split the current patchset in two. It will accelerate the acceptation of
the patches not depending on this work.

This is valid for 7,8,9 if I'm not mistaken.

Cheers

[lttng-dev] [PATCH lttng-tools 1/9] Improve handling of test SIGTERM/SIGINT

[Jonathan Rajotte-Julien]

Hi,

Please run shellcheck on this and fix only the issue introduced.

This is valid for all patches of this series.

Most comments for stop_*_opt functions applies to the others since the changes
are similar.

On Fri, May 03, 2019 at 09:55:39AM -0400, Mathieu Desnoyers wrote:
> The current state of signal handling for test scripts is: on
> SIGTERM/SIGINT of the tests (e.g. a CTRL-C on the console), session
> daemon and relay daemon are killed with SIGKILL, thus leaking all their
> resources, and leaving lttng kernel modules loaded.
> 
> Revamp the "stop" functions to take a signal number and a timeout
> as optional parameters. The default signal number is SIGTERM.
> 
> The full_cleanup trap handler now tries to nicely kill relayd and
> sessiond (if they are present) with SIGTERM, and wait up to the
> user-configurable LTTNG_TEST_TEARDOWN_TIMEOUT environment variable
> (which has a default of 60s). Then, if there are still either relayd,
> sessiond, or consumerd present, it will SIGKILL them and wait for
> them to vanish. If it had to kill sessiond with SIGKILL, it will
> also explicitly try to unload the lttng modules with modprobe.
> 
> This approach is inspired from sysv init script shutdown behavior.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  tests/utils/utils.sh | 180 +++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 137 insertions(+), 43 deletions(-)
> 
> diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
> index 94b3a3c4..d273b278 100644
> --- a/tests/utils/utils.sh
> +++ b/tests/utils/utils.sh
> @@ -15,14 +15,12 @@
>  
>  SESSIOND_BIN="lttng-sessiond"
>  SESSIOND_MATCH=".*lttng-sess.*"
> -SESSIOND_PIDS=""
>  RUNAS_BIN="lttng-runas"
>  RUNAS_MATCH=".*lttng-runas.*"
>  CONSUMERD_BIN="lttng-consumerd"
>  CONSUMERD_MATCH=".*lttng-consumerd.*"
>  RELAYD_BIN="lttng-relayd"
>  RELAYD_MATCH=".*lttng-relayd.*"
> -RELAYD_PIDS=""
>  LTTNG_BIN="lttng"
>  BABELTRACE_BIN="babeltrace"
>  OUTPUT_DEST=/dev/null
> @@ -48,11 +46,20 @@ export LTTNG_SESSIOND_PATH="/bin/true"
>  
>  source $TESTDIR/utils/tap/tap.sh
>  
> +if [ -z $LTTNG_TEST_TEARDOWN_TIMEOUT ]; then
> +	LTTNG_TEST_TEARDOWN_TIMEOUT=60
> +fi
> +
>  function full_cleanup ()
>  {
> -	if [ -n "${SESSIOND_PIDS}" ] || [ -n "${RELAYD_PIDS}" ]; then
> -		kill -9 ${SESSIOND_PIDS} ${RELAYD_PIDS} > /dev/null 2>&1
> -	fi
> +	# Try to kill daemons gracefully
> +	stop_lttng_relayd_notap SIGTERM $LTTNG_TEST_TEARDOWN_TIMEOUT
> +	stop_lttng_sessiond_notap SIGTERM $LTTNG_TEST_TEARDOWN_TIMEOUT
> +
> +	# If daemons are still present, forcibly kill them
> +	stop_lttng_relayd_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
> +	stop_lttng_sessiond_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
> +	stop_lttng_consumerd_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
>  
>  	# Disable trap for SIGTERM since the following kill to the
>  	# pidgroup will be SIGTERM. Otherwise it loops.
> @@ -397,8 +404,6 @@ function start_lttng_relayd_opt()
>  	else
>  		pass "Start lttng-relayd (opt: $opt)"
>  	fi
> -
> -	RELAYD_PIDS=$(pgrep $RELAYD_MATCH)
>  }
>  
>  function start_lttng_relayd()
> @@ -414,29 +419,58 @@ function start_lttng_relayd_notap()
>  function stop_lttng_relayd_opt()
>  {
>  	local withtap=$1
> +	local signal=$2
> +	local timeout=$3

What is timeout expected unit? seconds?

If so make it explicit.

> +	local dtimeleft=
> +	local fail=0
> +	local pids=$(pgrep $RELAYD_MATCH)
>  
> -	if [ $withtap -eq "1" ]; then
> -		diag "Killing lttng-relayd (pid: $RELAYD_PIDS)"
> +	if [ -n "$timeout" ]; then

Add a comment on why you are doing the multiplication here (easier arithmetic
down the line).

> +		dtimeleft=$(($timeout * 2))
> +	fi
> +
> +	if [ -z "$signal" ]; then
> +		signal="SIGTERM"
>  	fi
> -	kill $RELAYD_PIDS 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
> +
> +	if [ -z "$pids" ]; then
> +		if [ $withtap -eq "1" ]; then
> +			pass "No relay daemon to kill"
> +		fi
> +		return 0
> +	fi
> +
> +	diag "Killing (signal $signal) lttng-relayd (pid: $pids)"
> +
> +	kill -s $signal $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>  	retval=$?
>  
>  	if [ $? -eq 1 ]; then
> +		fail=1

"fail" is set here but never reused, is there a check on "fail" missing down the
road?

>  		if [ $withtap -eq "1" ]; then
>  			fail "Kill relay daemon"
>  		fi
> -		return 1
>  	else
>  		out=1
>  		while [ -n "$out" ]; do
>  			out=$(pgrep $RELAYD_MATCH)
> +			if [ -n "$dtimeleft" ]; then
> +				if [ $dtimeleft -lt 0 ]; then
> +					out=
> +					fail=1
> +				fi
> +				dtimeleft=$(($dtimeleft - 1))
> +			fi
>  			sleep 0.5

We could also consider using 1 here and remove the * 2 found earlier.

We aren't that time sensitive. If we want to go that route, let's use 0.1 here
and * 10 as the base multiplier.

>  		done
>  		if [ $withtap -eq "1" ]; then
> -			pass "Kill relay daemon"
> +			if [ $fail -eq "0" ]; then
> +				pass "Wait after kill relay daemon"
> +			else
> +				fail "Wait after kill relay daemon"
> +			fi
>  		fi
>  	fi
> -	RELAYD_PIDS=""

Should retval be 1 if we failed due to timeout?

The matter of "do we need retval here?" will be for another time.

>  	return $retval
>  }
>  
> @@ -508,7 +542,6 @@ function start_lttng_sessiond_opt()
>  			ok $status "Start session daemon"
>  		fi
>  	fi
> -	SESSIOND_PIDS=$(pgrep $SESSIOND_MATCH)
>  }
>  
>  function start_lttng_sessiond()
> @@ -525,24 +558,43 @@ function stop_lttng_sessiond_opt()
>  {
>  	local withtap=$1
>  	local signal=$2
> -	local kill_opt=""
> +	local timeout=$3
> +	local dtimeleft=
> +	local fail=0
>  
> -	if [ -n $TEST_NO_SESSIOND ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
> +	if [ -n "$timeout" ]; then
> +		dtimeleft=$(($timeout * 2))
> +	fi
> +
> +	if [ -n "$TEST_NO_SESSIOND" ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>  		# Env variable requested no session daemon
>  		return
>  	fi
>  
> -	local pids="${SESSIOND_PIDS} $(pgrep $RUNAS_MATCH)"
> +	local runas_pids=$(pgrep $RUNAS_MATCH)
> +	local pids=$(pgrep $SESSIOND_MATCH)
>  
> -	if [ -n "$2" ]; then
> -		kill_opt="$kill_opt -s $signal"
> +	if [ -n "$runas_pids" ]; then
> +		pids="$pids $runas_pids"
>  	fi
> -	if [ $withtap -eq "1" ]; then
> -		diag "Killing $SESSIOND_BIN and lt-$SESSIOND_BIN pids: $(echo $pids | tr '\n' ' ')"
> +
> +	if [ -z "$pids" ]; then
> +		if [ $withtap -eq "1" ]; then
> +			pass "No session daemon to kill"
> +		fi
> +		return
>  	fi
> -	kill $kill_opt $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
> +
> +	if [ -z "$signal" ]; then
> +		signal=SIGTERM
> +	fi

Move this after variable declaration at the top.

> +
> +	diag "Killing (signal $signal) $SESSIOND_BIN and lt-$SESSIOND_BIN pids: $(echo $pids | tr '\n' ' ')"
> +
> +	kill -s $signal $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>  
>  	if [ $? -eq 1 ]; then
> +		fail=1

"fail" is set but never used. Is it intentional?

>  		if [ $withtap -eq "1" ]; then
>  			fail "Kill sessions daemon"
>  		fi
> @@ -550,17 +602,44 @@ function stop_lttng_sessiond_opt()
>  		out=1
>  		while [ -n "$out" ]; do
>  			out=$(pgrep ${SESSIOND_MATCH})
> +			if [ -n "$dtimeleft" ]; then
> +				if [ $dtimeleft -lt 0 ]; then
> +					out=
> +					fail=1
> +				fi
> +				dtimeleft=$(($dtimeleft - 1))
> +			fi
>  			sleep 0.5
>  		done
>  		out=1
>  		while [ -n "$out" ]; do
>  			out=$(pgrep $CONSUMERD_MATCH)
> +			if [ -n "$dtimeleft" ]; then
> +				if [ $dtimeleft -lt 0 ]; then
> +					out=
> +					fail=1
> +				fi
> +				dtimeleft=$(($dtimeleft - 1))
> +			fi
>  			sleep 0.5
>  		done
>  
> -		SESSIOND_PIDS=""
>  		if [ $withtap -eq "1" ]; then
> -			pass "Kill session daemon"
> +			if [ $fail -eq "0" ]; then
> +				pass "Wait after kill session daemon"
> +			else
> +				fail "Wait after kill session daemon"
> +			fi
> +		fi
> +	fi
> +	if [ "$signal" = "SIGKILL" ]; then
> +		if [ "$(id -u)" -eq "0" ]; then
> +			local modules="$(lsmod | grep ^lttng | awk '{print $1}')"
> +
> +			if [ -n "$modules" ]; then
> +				diag "Unloading all LTTng modules"
> +				modprobe -r $modules
> +			fi
>  		fi
>  	fi
>  }
> @@ -579,21 +658,18 @@ function sigstop_lttng_sessiond_opt()
>  {
>  	local withtap=$1
>  	local signal=SIGSTOP
> -	local kill_opt=""
>  
> -	if [ -n $TEST_NO_SESSIOND ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
> +	if [ -n "$TEST_NO_SESSIOND" ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>  		# Env variable requested no session daemon
>  		return
>  	fi
>  
>  	PID_SESSIOND="$(pgrep ${SESSIOND_MATCH}) $(pgrep $RUNAS_MATCH)"
>  
> -	kill_opt="$kill_opt -s $signal"
> -
>  	if [ $withtap -eq "1" ]; then
>  		diag "Sending SIGSTOP to lt-$SESSIOND_BIN and $SESSIOND_BIN pids: $(echo $PID_SESSIOND | tr '\n' ' ')"
>  	fi
> -	kill $kill_opt $PID_SESSIOND 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
> +	kill -s $signal $PID_SESSIOND 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>  
>  	if [ $? -eq 1 ]; then
>  		if [ $withtap -eq "1" ]; then
> @@ -635,26 +711,37 @@ function stop_lttng_consumerd_opt()
>  {
>  	local withtap=$1
>  	local signal=$2
> -	local kill_opt=""
> +	local timeout=$3
> +	local dtimeleft=
> +	local fail=0
>  
>  	PID_CONSUMERD=$(pgrep $CONSUMERD_MATCH)
>  
> -	if [ -n "$2" ]; then
> -		kill_opt="$kill_opt -s $signal"
> +	if [ -n "$timeout" ]; then
> +		dtimeleft=$(($timeout * 2))
>  	fi
>  
> -	if [ $withtap -eq "1" ]; then
> -		diag "Killing $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n' ' ')"
> +	if [ -z "$signal" ]; then
> +		signal=SIGTERM
>  	fi
>  
> -	kill $kill_opt $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
> +	if [ -z "$PID_CONSUMERD" ]; then
> +		if [ $withtap -eq "1" ]; then
> +			pass "No consumer daemon to kill"
> +		fi
> +		return
> +	fi
> +
> +	diag "Killing (signal $signal) $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n' ' ')"
> +
> +	kill -s $signal $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>  	retval=$?
>  
>  	if [ $? -eq 1 ]; then
> +		fail=1
>  		if [ $withtap -eq "1" ]; then
>  			fail "Kill consumer daemon"
>  		fi
> -		return 1
>  	else
>  		out=1
>  		while [ $out -ne 0 ]; do
> @@ -669,10 +756,21 @@ function stop_lttng_consumerd_opt()
>  					out=1
>  				fi
>  			done
> +			if [ -n "$dtimeleft" ]; then
> +				if [ $dtimeleft -lt 0 ]; then
> +					out=0
> +					fail=1
> +				fi
> +				dtimeleft=$(($dtimeleft - 1))
> +			fi
>  			sleep 0.5
>  		done
>  		if [ $withtap -eq "1" ]; then
> -			pass "Kill consumer daemon"
> +			if [ $fail -eq "0" ]; then
> +				pass "Wait after kill consumer daemon"
> +			else
> +				fail "Wait after kill consumer daemon"
> +			fi
>  		fi
>  	fi
>  	return $retval
> @@ -692,16 +790,12 @@ function sigstop_lttng_consumerd_opt()
>  {
>  	local withtap=$1
>  	local signal=SIGSTOP
> -	local kill_opt=""
>  
>  	PID_CONSUMERD=$(pgrep $CONSUMERD_MATCH)
>  
> -	kill_opt="$kill_opt -s $signal"
> +	diag "Sending SIGSTOP to $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n' ' ')"
>  
> -	if [ $withtap -eq "1" ]; then
> -		diag "Sending SIGSTOP to $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n' ' ')"
> -	fi
> -	kill $kill_opt $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
> +	kill -s $signal $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>  	retval=$?
>  
>  	if [ $? -eq 1 ]; then
> -- 
> 2.11.0
> 

-- 
Jonathan Rajotte-Julien
EfficiOS

[lttng-dev] [PATCH lttng-tools 1/9] Improve handling of test SIGTERM/SIGINT

[Mathieu Desnoyers]

----- On May 15, 2019, at 12:19 PM, Jonathan Rajotte jonathan.rajotte-julien at efficios.com wrote:

> Hi,
> 
> Please run shellcheck on this and fix only the issue introduced.

OK

> 
> This is valid for all patches of this series.

OK

> 
> Most comments for stop_*_opt functions applies to the others since the changes
> are similar.

OK

> 
> On Fri, May 03, 2019 at 09:55:39AM -0400, Mathieu Desnoyers wrote:
>> The current state of signal handling for test scripts is: on
>> SIGTERM/SIGINT of the tests (e.g. a CTRL-C on the console), session
>> daemon and relay daemon are killed with SIGKILL, thus leaking all their
>> resources, and leaving lttng kernel modules loaded.
>> 
>> Revamp the "stop" functions to take a signal number and a timeout
>> as optional parameters. The default signal number is SIGTERM.
>> 
>> The full_cleanup trap handler now tries to nicely kill relayd and
>> sessiond (if they are present) with SIGTERM, and wait up to the
>> user-configurable LTTNG_TEST_TEARDOWN_TIMEOUT environment variable
>> (which has a default of 60s). Then, if there are still either relayd,
>> sessiond, or consumerd present, it will SIGKILL them and wait for
>> them to vanish. If it had to kill sessiond with SIGKILL, it will
>> also explicitly try to unload the lttng modules with modprobe.
>> 
>> This approach is inspired from sysv init script shutdown behavior.
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>> ---
>>  tests/utils/utils.sh | 180 +++++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 137 insertions(+), 43 deletions(-)
>> 
>> diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
>> index 94b3a3c4..d273b278 100644
>> --- a/tests/utils/utils.sh
>> +++ b/tests/utils/utils.sh
>> @@ -15,14 +15,12 @@
>>  
>>  SESSIOND_BIN="lttng-sessiond"
>>  SESSIOND_MATCH=".*lttng-sess.*"
>> -SESSIOND_PIDS=""
>>  RUNAS_BIN="lttng-runas"
>>  RUNAS_MATCH=".*lttng-runas.*"
>>  CONSUMERD_BIN="lttng-consumerd"
>>  CONSUMERD_MATCH=".*lttng-consumerd.*"
>>  RELAYD_BIN="lttng-relayd"
>>  RELAYD_MATCH=".*lttng-relayd.*"
>> -RELAYD_PIDS=""
>>  LTTNG_BIN="lttng"
>>  BABELTRACE_BIN="babeltrace"
>>  OUTPUT_DEST=/dev/null
>> @@ -48,11 +46,20 @@ export LTTNG_SESSIOND_PATH="/bin/true"
>>  
>>  source $TESTDIR/utils/tap/tap.sh
>>  
>> +if [ -z $LTTNG_TEST_TEARDOWN_TIMEOUT ]; then
>> +	LTTNG_TEST_TEARDOWN_TIMEOUT=60
>> +fi
>> +
>>  function full_cleanup ()
>>  {
>> -	if [ -n "${SESSIOND_PIDS}" ] || [ -n "${RELAYD_PIDS}" ]; then
>> -		kill -9 ${SESSIOND_PIDS} ${RELAYD_PIDS} > /dev/null 2>&1
>> -	fi
>> +	# Try to kill daemons gracefully
>> +	stop_lttng_relayd_notap SIGTERM $LTTNG_TEST_TEARDOWN_TIMEOUT
>> +	stop_lttng_sessiond_notap SIGTERM $LTTNG_TEST_TEARDOWN_TIMEOUT
>> +
>> +	# If daemons are still present, forcibly kill them
>> +	stop_lttng_relayd_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
>> +	stop_lttng_sessiond_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
>> +	stop_lttng_consumerd_notap SIGKILL $LTTNG_TEST_TEARDOWN_TIMEOUT
>>  
>>  	# Disable trap for SIGTERM since the following kill to the
>>  	# pidgroup will be SIGTERM. Otherwise it loops.
>> @@ -397,8 +404,6 @@ function start_lttng_relayd_opt()
>>  	else
>>  		pass "Start lttng-relayd (opt: $opt)"
>>  	fi
>> -
>> -	RELAYD_PIDS=$(pgrep $RELAYD_MATCH)
>>  }
>>  
>>  function start_lttng_relayd()
>> @@ -414,29 +419,58 @@ function start_lttng_relayd_notap()
>>  function stop_lttng_relayd_opt()
>>  {
>>  	local withtap=$1
>> +	local signal=$2
>> +	local timeout=$3
> 
> What is timeout expected unit? seconds?

Yes, will rename to "timeout_s".

> 
> If so make it explicit.
> 
>> +	local dtimeleft=
>> +	local fail=0
>> +	local pids=$(pgrep $RELAYD_MATCH)
>>  
>> -	if [ $withtap -eq "1" ]; then
>> -		diag "Killing lttng-relayd (pid: $RELAYD_PIDS)"
>> +	if [ -n "$timeout" ]; then
> 
> Add a comment on why you are doing the multiplication here (easier arithmetic
> down the line).

OK

> 
>> +		dtimeleft=$(($timeout * 2))
>> +	fi
>> +
>> +	if [ -z "$signal" ]; then
>> +		signal="SIGTERM"
>>  	fi
>> -	kill $RELAYD_PIDS 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>> +
>> +	if [ -z "$pids" ]; then
>> +		if [ $withtap -eq "1" ]; then
>> +			pass "No relay daemon to kill"
>> +		fi
>> +		return 0
>> +	fi
>> +
>> +	diag "Killing (signal $signal) lttng-relayd (pid: $pids)"
>> +
>> +	kill -s $signal $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>>  	retval=$?
>>  
>>  	if [ $? -eq 1 ]; then
>> +		fail=1
> 
> "fail" is set here but never reused, is there a check on "fail" missing down the
> road?

Will change the functions to return "$retval", and rename fail to retval,
and remove the retval=$? which becomes redundant.

> 
>>  		if [ $withtap -eq "1" ]; then
>>  			fail "Kill relay daemon"
>>  		fi
>> -		return 1
>>  	else
>>  		out=1
>>  		while [ -n "$out" ]; do
>>  			out=$(pgrep $RELAYD_MATCH)
>> +			if [ -n "$dtimeleft" ]; then
>> +				if [ $dtimeleft -lt 0 ]; then
>> +					out=
>> +					fail=1
>> +				fi
>> +				dtimeleft=$(($dtimeleft - 1))
>> +			fi
>>  			sleep 0.5
> 
> We could also consider using 1 here and remove the * 2 found earlier.
> 
> We aren't that time sensitive. If we want to go that route, let's use 0.1 here
> and * 10 as the base multiplier.

The intent of this patchset is not to tweak the delay wait increments.
Feel free to propose that change later on. I'm keeping the existing
delays for now.

> 
>>  		done
>>  		if [ $withtap -eq "1" ]; then
>> -			pass "Kill relay daemon"
>> +			if [ $fail -eq "0" ]; then
>> +				pass "Wait after kill relay daemon"
>> +			else
>> +				fail "Wait after kill relay daemon"
>> +			fi
>>  		fi
>>  	fi
>> -	RELAYD_PIDS=""
> 
> Should retval be 1 if we failed due to timeout?

Yes. I'm correcting this as well.

> 
> The matter of "do we need retval here?" will be for another time.

indeed.

> 
>>  	return $retval
>>  }
>>  
>> @@ -508,7 +542,6 @@ function start_lttng_sessiond_opt()
>>  			ok $status "Start session daemon"
>>  		fi
>>  	fi
>> -	SESSIOND_PIDS=$(pgrep $SESSIOND_MATCH)
>>  }
>>  
>>  function start_lttng_sessiond()
>> @@ -525,24 +558,43 @@ function stop_lttng_sessiond_opt()
>>  {
>>  	local withtap=$1
>>  	local signal=$2
>> -	local kill_opt=""
>> +	local timeout=$3
>> +	local dtimeleft=
>> +	local fail=0
>>  
>> -	if [ -n $TEST_NO_SESSIOND ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>> +	if [ -n "$timeout" ]; then
>> +		dtimeleft=$(($timeout * 2))
>> +	fi
>> +
>> +	if [ -n "$TEST_NO_SESSIOND" ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>>  		# Env variable requested no session daemon
>>  		return
>>  	fi
>>  
>> -	local pids="${SESSIOND_PIDS} $(pgrep $RUNAS_MATCH)"
>> +	local runas_pids=$(pgrep $RUNAS_MATCH)
>> +	local pids=$(pgrep $SESSIOND_MATCH)
>>  
>> -	if [ -n "$2" ]; then
>> -		kill_opt="$kill_opt -s $signal"
>> +	if [ -n "$runas_pids" ]; then
>> +		pids="$pids $runas_pids"
>>  	fi
>> -	if [ $withtap -eq "1" ]; then
>> -		diag "Killing $SESSIOND_BIN and lt-$SESSIOND_BIN pids: $(echo $pids | tr '\n'
>> ' ')"
>> +
>> +	if [ -z "$pids" ]; then
>> +		if [ $withtap -eq "1" ]; then
>> +			pass "No session daemon to kill"
>> +		fi
>> +		return
>>  	fi
>> -	kill $kill_opt $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>> +
>> +	if [ -z "$signal" ]; then
>> +		signal=SIGTERM
>> +	fi
> 
> Move this after variable declaration at the top.

ok

> 
>> +
>> +	diag "Killing (signal $signal) $SESSIOND_BIN and lt-$SESSIOND_BIN pids: $(echo
>> $pids | tr '\n' ' ')"
>> +
>> +	kill -s $signal $pids 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>>  
>>  	if [ $? -eq 1 ]; then
>> +		fail=1
> 
> "fail" is set but never used. Is it intentional?

fixing.

Thanks,

Mathieu

> 
>>  		if [ $withtap -eq "1" ]; then
>>  			fail "Kill sessions daemon"
>>  		fi
>> @@ -550,17 +602,44 @@ function stop_lttng_sessiond_opt()
>>  		out=1
>>  		while [ -n "$out" ]; do
>>  			out=$(pgrep ${SESSIOND_MATCH})
>> +			if [ -n "$dtimeleft" ]; then
>> +				if [ $dtimeleft -lt 0 ]; then
>> +					out=
>> +					fail=1
>> +				fi
>> +				dtimeleft=$(($dtimeleft - 1))
>> +			fi
>>  			sleep 0.5
>>  		done
>>  		out=1
>>  		while [ -n "$out" ]; do
>>  			out=$(pgrep $CONSUMERD_MATCH)
>> +			if [ -n "$dtimeleft" ]; then
>> +				if [ $dtimeleft -lt 0 ]; then
>> +					out=
>> +					fail=1
>> +				fi
>> +				dtimeleft=$(($dtimeleft - 1))
>> +			fi
>>  			sleep 0.5
>>  		done
>>  
>> -		SESSIOND_PIDS=""
>>  		if [ $withtap -eq "1" ]; then
>> -			pass "Kill session daemon"
>> +			if [ $fail -eq "0" ]; then
>> +				pass "Wait after kill session daemon"
>> +			else
>> +				fail "Wait after kill session daemon"
>> +			fi
>> +		fi
>> +	fi
>> +	if [ "$signal" = "SIGKILL" ]; then
>> +		if [ "$(id -u)" -eq "0" ]; then
>> +			local modules="$(lsmod | grep ^lttng | awk '{print $1}')"
>> +
>> +			if [ -n "$modules" ]; then
>> +				diag "Unloading all LTTng modules"
>> +				modprobe -r $modules
>> +			fi
>>  		fi
>>  	fi
>>  }
>> @@ -579,21 +658,18 @@ function sigstop_lttng_sessiond_opt()
>>  {
>>  	local withtap=$1
>>  	local signal=SIGSTOP
>> -	local kill_opt=""
>>  
>> -	if [ -n $TEST_NO_SESSIOND ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>> +	if [ -n "$TEST_NO_SESSIOND" ] && [ "$TEST_NO_SESSIOND" == "1" ]; then
>>  		# Env variable requested no session daemon
>>  		return
>>  	fi
>>  
>>  	PID_SESSIOND="$(pgrep ${SESSIOND_MATCH}) $(pgrep $RUNAS_MATCH)"
>>  
>> -	kill_opt="$kill_opt -s $signal"
>> -
>>  	if [ $withtap -eq "1" ]; then
>>  		diag "Sending SIGSTOP to lt-$SESSIOND_BIN and $SESSIOND_BIN pids: $(echo
>>  		$PID_SESSIOND | tr '\n' ' ')"
>>  	fi
>> -	kill $kill_opt $PID_SESSIOND 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>> +	kill -s $signal $PID_SESSIOND 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>>  
>>  	if [ $? -eq 1 ]; then
>>  		if [ $withtap -eq "1" ]; then
>> @@ -635,26 +711,37 @@ function stop_lttng_consumerd_opt()
>>  {
>>  	local withtap=$1
>>  	local signal=$2
>> -	local kill_opt=""
>> +	local timeout=$3
>> +	local dtimeleft=
>> +	local fail=0
>>  
>>  	PID_CONSUMERD=$(pgrep $CONSUMERD_MATCH)
>>  
>> -	if [ -n "$2" ]; then
>> -		kill_opt="$kill_opt -s $signal"
>> +	if [ -n "$timeout" ]; then
>> +		dtimeleft=$(($timeout * 2))
>>  	fi
>>  
>> -	if [ $withtap -eq "1" ]; then
>> -		diag "Killing $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n' ' ')"
>> +	if [ -z "$signal" ]; then
>> +		signal=SIGTERM
>>  	fi
>>  
>> -	kill $kill_opt $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>> +	if [ -z "$PID_CONSUMERD" ]; then
>> +		if [ $withtap -eq "1" ]; then
>> +			pass "No consumer daemon to kill"
>> +		fi
>> +		return
>> +	fi
>> +
>> +	diag "Killing (signal $signal) $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr
>> '\n' ' ')"
>> +
>> +	kill -s $signal $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>>  	retval=$?
>>  
>>  	if [ $? -eq 1 ]; then
>> +		fail=1
>>  		if [ $withtap -eq "1" ]; then
>>  			fail "Kill consumer daemon"
>>  		fi
>> -		return 1
>>  	else
>>  		out=1
>>  		while [ $out -ne 0 ]; do
>> @@ -669,10 +756,21 @@ function stop_lttng_consumerd_opt()
>>  					out=1
>>  				fi
>>  			done
>> +			if [ -n "$dtimeleft" ]; then
>> +				if [ $dtimeleft -lt 0 ]; then
>> +					out=0
>> +					fail=1
>> +				fi
>> +				dtimeleft=$(($dtimeleft - 1))
>> +			fi
>>  			sleep 0.5
>>  		done
>>  		if [ $withtap -eq "1" ]; then
>> -			pass "Kill consumer daemon"
>> +			if [ $fail -eq "0" ]; then
>> +				pass "Wait after kill consumer daemon"
>> +			else
>> +				fail "Wait after kill consumer daemon"
>> +			fi
>>  		fi
>>  	fi
>>  	return $retval
>> @@ -692,16 +790,12 @@ function sigstop_lttng_consumerd_opt()
>>  {
>>  	local withtap=$1
>>  	local signal=SIGSTOP
>> -	local kill_opt=""
>>  
>>  	PID_CONSUMERD=$(pgrep $CONSUMERD_MATCH)
>>  
>> -	kill_opt="$kill_opt -s $signal"
>> +	diag "Sending SIGSTOP to $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n'
>> ' ')"
>>  
>> -	if [ $withtap -eq "1" ]; then
>> -		diag "Sending SIGSTOP to $CONSUMERD_BIN pids: $(echo $PID_CONSUMERD | tr '\n'
>> ' ')"
>> -	fi
>> -	kill $kill_opt $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>> +	kill -s $signal $PID_CONSUMERD 1> $OUTPUT_DEST 2> $ERROR_OUTPUT_DEST
>>  	retval=$?
>>  
>>  	if [ $? -eq 1 ]; then
>> --
>> 2.11.0
>> 
> 
> --
> Jonathan Rajotte-Julien
> EfficiOS

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[lttng-dev] [PATCH lttng-tools 2/9] Fix: tests: error handling in high throughput limits test

[Jonathan Rajotte-Julien]

> @@ -168,8 +187,7 @@ skip $isroot "Root access is needed to set bandwith limits. Skipping all tests."
>  {
>  
>  	# Catch sigint and try to cleanup limits
> -	trap interrupt_cleanup SIGTERM
> -	trap interrupt_cleanup SIGINT
> +	trap interrupt_cleanup SIGTERM SIGINT
>  
>  	BW_LIMITS=(3200 1600 800 400 200 100 50 25)
>  	for BW in ${BW_LIMITS[@]};
> @@ -177,6 +195,7 @@ skip $isroot "Root access is needed to set bandwith limits. Skipping all tests."
>  		diag "Test high-throughput with bandwidth limit set to ${BW}kbits"
>  
>  		set_bw_limit $BW
> +		ok $? "Setting bandwidth limit"

In case of failure here we should force a reset since it could lead to long
timeout/hang depending on which tc command failed?

This could also be done inside set_bw_limit at each stage.

>  
>  		start_lttng_sessiond
>  		start_lttng_relayd "-o $TRACE_PATH"
> @@ -185,5 +204,6 @@ skip $isroot "Root access is needed to set bandwith limits. Skipping all tests."
>  		stop_lttng_relayd
>  		stop_lttng_sessiond
>  		reset_bw_limit
> +		ok $? "Reset bandwith limits"
>  	done
>  }
> -- 
> 2.11.0
> 

-- 
Jonathan Rajotte-Julien
EfficiOS

[lttng-dev] [PATCH lttng-tools 2/9] Fix: tests: error handling in high throughput limits test

[Mathieu Desnoyers]

----- On May 15, 2019, at 12:31 PM, Jonathan Rajotte jonathan.rajotte-julien at efficios.com wrote:

>> @@ -168,8 +187,7 @@ skip $isroot "Root access is needed to set bandwith limits.
>> Skipping all tests."
>>  {
>>  
>>  	# Catch sigint and try to cleanup limits
>> -	trap interrupt_cleanup SIGTERM
>> -	trap interrupt_cleanup SIGINT
>> +	trap interrupt_cleanup SIGTERM SIGINT
>>  
>>  	BW_LIMITS=(3200 1600 800 400 200 100 50 25)
>>  	for BW in ${BW_LIMITS[@]};
>> @@ -177,6 +195,7 @@ skip $isroot "Root access is needed to set bandwith limits.
>> Skipping all tests."
>>  		diag "Test high-throughput with bandwidth limit set to ${BW}kbits"
>>  
>>  		set_bw_limit $BW
>> +		ok $? "Setting bandwidth limit"
> 
> In case of failure here we should force a reset since it could lead to long
> timeout/hang depending on which tc command failed?
> 
> This could also be done inside set_bw_limit at each stage.

I'll do this. Will be in v2 of this patch.

Thanks,

Mathieu

> 
>>  
>>  		start_lttng_sessiond
>>  		start_lttng_relayd "-o $TRACE_PATH"
>> @@ -185,5 +204,6 @@ skip $isroot "Root access is needed to set bandwith limits.
>> Skipping all tests."
>>  		stop_lttng_relayd
>>  		stop_lttng_sessiond
>>  		reset_bw_limit
>> +		ok $? "Reset bandwith limits"
>>  	done
>>  }
>> --
>> 2.11.0
>> 
> 
> --
> Jonathan Rajotte-Julien
> EfficiOS

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[lttng-dev] [PATCH lttng-tools 6/9] tests: invoke full_cleanup from script trap handlers, use modprobe -r

[Jonathan Rajotte-Julien]

LGTM

On Fri, May 03, 2019 at 09:55:44AM -0400, Mathieu Desnoyers wrote:
> Scripts implementing their own trap handlers override the generic
> one provided by utils.sh (full_cleanup). Invoke it at the end of
> the handlers to provide the utils cleanup as well.
> 
> Moreover, change use of "rmmod" to "modprobe -r", which is better
> in trap handlers because it does not print errors if the module
> was not loaded yet when the signal occurs.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  tests/regression/kernel/test_clock_override                    | 10 +++-------
>  tests/regression/kernel/test_rotation_destroy_flush            |  7 +++----
>  tests/regression/tools/crash/test_crash                        |  3 +--
>  tests/regression/tools/notification/test_notification_kernel   |  2 +-
>  .../regression/tools/notification/test_notification_multi_app  |  2 +-
>  tests/regression/tools/notification/test_notification_ust      |  2 +-
>  tests/regression/tools/streaming/test_high_throughput_limits   |  1 -
>  .../ust/rotation-destroy-flush/test_rotation_destroy_flush     |  3 +--
>  tests/stress/test_multi_sessions_per_uid_10app                 |  5 ++---
>  tests/stress/test_multi_sessions_per_uid_5app_streaming        |  5 ++---
>  .../test_multi_sessions_per_uid_5app_streaming_kill_relayd     |  5 ++---
>  11 files changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/tests/regression/kernel/test_clock_override b/tests/regression/kernel/test_clock_override
> index e19b77e1..1fbba771 100755
> --- a/tests/regression/kernel/test_clock_override
> +++ b/tests/regression/kernel/test_clock_override
> @@ -49,11 +49,9 @@ source $TESTDIR/utils/utils.sh
>  function signal_cleanup()
>  {
>  	diag "*** Exiting ***"
> -	rmmod lttng-test
>  	stop_lttng_sessiond
> -	rmmod lttng-clock-plugin-test
> -	rmmod lttng-clock
> -	exit 1
> +	modprobe -r lttng-test lttng-clock-plugin-test lttng-clock
> +	full_cleanup
>  }
>  
>  function extract_clock_metadata()
> @@ -93,10 +91,8 @@ function test_clock_override_metadata()
>  	stop_lttng_tracing_ok $SESSION_NAME
>  	destroy_lttng_session_ok $SESSION_NAME
>  
> -	rmmod lttng-test
>  	stop_lttng_sessiond
> -	rmmod lttng-clock-plugin-test
> -	rmmod lttng-clock
> +	modprobe -r lttng-test lttng-clock-plugin-test lttng-clock
>  
>  	local TRACE_METADATA_FILE_PATH="$(find "$TRACE_PATH" -name metadata -type f)"
>  	local TRACE_METADATA_DIR="$(dirname "$TRACE_METADATA_FILE_PATH")"
> diff --git a/tests/regression/kernel/test_rotation_destroy_flush b/tests/regression/kernel/test_rotation_destroy_flush
> index 0b0b0ca7..03933a3a 100755
> --- a/tests/regression/kernel/test_rotation_destroy_flush
> +++ b/tests/regression/kernel/test_rotation_destroy_flush
> @@ -39,9 +39,8 @@ source $TESTDIR/utils/utils.sh
>  function signal_cleanup()
>  {
>  	diag "*** Exiting ***"
> -	rmmod lttng-test
> -	stop_lttng_sessiond
> -	exit 1
> +	modprobe -r lttng-test
> +	full_cleanup
>  }
>  
>  function enable_kernel_lttng_channel_size_limit ()
> @@ -107,7 +106,7 @@ function test_rotation_destroy_flush_single()
>  
>  	rm -rf $TRACE_PATH
>  
> -	rmmod lttng-test
> +	modprobe -r lttng-test
>  	stop_lttng_sessiond
>  }
>  
> diff --git a/tests/regression/tools/crash/test_crash b/tests/regression/tools/crash/test_crash
> index 13909c1b..5bad16e5 100755
> --- a/tests/regression/tools/crash/test_crash
> +++ b/tests/regression/tools/crash/test_crash
> @@ -392,8 +392,7 @@ function interrupt_cleanup()
>  {
>      diag "*** Cleaning-up test ***"
>      stop_test_apps
> -    stop_lttng_sessiond
> -    exit 1
> +    full_cleanup
>  }
>  
>  TESTS=(
> diff --git a/tests/regression/tools/notification/test_notification_kernel b/tests/regression/tools/notification/test_notification_kernel
> index e7368df2..cc6fc581 100755
> --- a/tests/regression/tools/notification/test_notification_kernel
> +++ b/tests/regression/tools/notification/test_notification_kernel
> @@ -56,7 +56,7 @@ function kernel_event_generator
>  	state_file=$1
>  	kernel_event_generator_suspended=0
>  	trap kernel_event_generator_toogle_state SIGUSR1
> -	trap "exit" SIGTERM SIGINT EXIT
> +
>  	while (true); do
>  		if [[ $kernel_event_generator_suspended -eq "1" ]]; then
>  			touch $state_file
> diff --git a/tests/regression/tools/notification/test_notification_multi_app b/tests/regression/tools/notification/test_notification_multi_app
> index 7465a83f..51d94e4f 100755
> --- a/tests/regression/tools/notification/test_notification_multi_app
> +++ b/tests/regression/tools/notification/test_notification_multi_app
> @@ -64,7 +64,7 @@ function kernel_event_generator
>  	state_file=$1
>  	kernel_event_generator_suspended=0
>  	trap kernel_event_generator_toogle_state SIGUSR1
> -	trap "exit" SIGTERM SIGINT
> +
>  	while (true); do
>  		if [[ $kernel_event_generator_suspended -eq "1" ]]; then
>  			touch $state_file
> diff --git a/tests/regression/tools/notification/test_notification_ust b/tests/regression/tools/notification/test_notification_ust
> index 8941e476..82f79a8e 100755
> --- a/tests/regression/tools/notification/test_notification_ust
> +++ b/tests/regression/tools/notification/test_notification_ust
> @@ -56,7 +56,7 @@ function ust_event_generator
>  	state_file=$1
>  	ust_event_generator_suspended=0
>  	trap ust_event_generator_toogle_state SIGUSR1
> -	trap "exit" SIGTERM SIGINT
> +
>  	while (true); do
>  		if [[ $ust_event_generator_suspended -eq "1" ]]; then
>  			touch $state_file
> diff --git a/tests/regression/tools/streaming/test_high_throughput_limits b/tests/regression/tools/streaming/test_high_throughput_limits
> index 68973c73..c49c13c2 100755
> --- a/tests/regression/tools/streaming/test_high_throughput_limits
> +++ b/tests/regression/tools/streaming/test_high_throughput_limits
> @@ -170,7 +170,6 @@ function interrupt_cleanup()
>  	reset_bw_limit
>  	# invoke utils cleanup
>  	full_cleanup
> -	exit 1
>  }
>  
>  plan_tests $NUM_TESTS
> diff --git a/tests/regression/ust/rotation-destroy-flush/test_rotation_destroy_flush b/tests/regression/ust/rotation-destroy-flush/test_rotation_destroy_flush
> index a7a93771..e404564e 100755
> --- a/tests/regression/ust/rotation-destroy-flush/test_rotation_destroy_flush
> +++ b/tests/regression/ust/rotation-destroy-flush/test_rotation_destroy_flush
> @@ -48,8 +48,7 @@ function run_app()
>  function signal_cleanup()
>  {
>  	diag "*** Exiting ***"
> -	stop_lttng_sessiond
> -	exit 1
> +	full_cleanup
>  }
>  
>  function enable_ust_lttng_channel_size_limit ()
> diff --git a/tests/stress/test_multi_sessions_per_uid_10app b/tests/stress/test_multi_sessions_per_uid_10app
> index 82e8ad50..c9f8403e 100755
> --- a/tests/stress/test_multi_sessions_per_uid_10app
> +++ b/tests/stress/test_multi_sessions_per_uid_10app
> @@ -112,11 +112,10 @@ function sighandler()
>  {
>  	cleanup
>  	rm $LOG_FILE
> -	exit 1
> +	full_cleanup
>  }
>  
> -trap sighandler SIGINT
> -trap sighandler SIGTERM
> +trap sighandler SIGINT SIGTERM
>  
>  # Make sure we collect a coredump if possible.
>  ulimit -c unlimited
> diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming b/tests/stress/test_multi_sessions_per_uid_5app_streaming
> index ed989498..4203ac30 100755
> --- a/tests/stress/test_multi_sessions_per_uid_5app_streaming
> +++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming
> @@ -142,11 +142,10 @@ function sighandler()
>  {
>  	cleanup
>  	rm $LOG_FILE_SESSIOND $LOG_FILE_RELAYD
> -	exit 1
> +	full_cleanup
>  }
>  
> -trap sighandler SIGINT
> -trap sighandler SIGTERM
> +trap sighandler SIGINT SIGTERM
>  
>  # Make sure we collect a coredump if possible.
>  ulimit -c unlimited
> diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
> index c699ac22..d0121e32 100755
> --- a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
> +++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
> @@ -144,11 +144,10 @@ function sighandler()
>  {
>  	cleanup
>  	#rm $LOG_FILE_SESSIOND $LOG_FILE_RELAYD
> -	exit 1
> +	full_cleanup
>  }
>  
> -trap sighandler SIGINT
> -trap sighandler SIGTERM
> +trap sighandler SIGINT SIGTERM
>  
>  # Make sure we collect a coredump if possible.
>  ulimit -c unlimited
> -- 
> 2.11.0
> 

-- 
Jonathan Rajotte-Julien
EfficiOS

[lttng-dev] [PATCH lttng-tools 5/9] Cleanup: test: don't stop relayd twice

[Jonathan Rajotte-Julien]

LGTM

On Fri, May 03, 2019 at 09:55:43AM -0400, Mathieu Desnoyers wrote:
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  tests/regression/tools/live/test_lttng_ust | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/regression/tools/live/test_lttng_ust b/tests/regression/tools/live/test_lttng_ust
> index 06017d01..830fc783 100755
> --- a/tests/regression/tools/live/test_lttng_ust
> +++ b/tests/regression/tools/live/test_lttng_ust
> @@ -34,7 +34,7 @@ TRACE_PATH=$(mktemp -d)
>  
>  DIR=$(readlink -f $TESTDIR)
>  
> -NUM_TESTS=12
> +NUM_TESTS=11
>  
>  source $TESTDIR/utils/utils.sh
>  
> @@ -84,5 +84,4 @@ stop_lttng_relayd
>  
>  test_custom_url
>  
> -stop_lttng_relayd
>  stop_lttng_sessiond
> -- 
> 2.11.0
> 

-- 
Jonathan Rajotte-Julien
EfficiOS