tests: Store the correct PID in hostapd-test.pid file

Message ID 1534959105-13163-1-git-send-email-andrei.otcheretianski@intel.com
State Changes Requested
Headers show
Series
  • tests: Store the correct PID in hostapd-test.pid file
Related show

Commit Message

Andrei Otcheretianski Aug. 22, 2018, 5:31 p.m.
The ./start.sh script spawns hostapd process using "sudo".
Since sudo forks a child process, $! holds the pid of sudo itself.
Fix that by storing the PID of the child process instead.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
---
 tests/hwsim/start.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jouni Malinen Oct. 20, 2018, 8:54 a.m. | #1
On Wed, Aug 22, 2018 at 08:31:43PM +0300, Andrei Otcheretianski wrote:
> The ./start.sh script spawns hostapd process using "sudo".
> Since sudo forks a child process, $! holds the pid of sudo itself.
> Fix that by storing the PID of the child process instead.

> diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh
> @@ -121,7 +121,10 @@ sudo $(printf -- "$VALGRIND_WPAS" 5) $WPAS -g /tmp/wpas-wlan5 -G$GROUP \
>  sudo $VALGRIND_HAPD $HAPD -ddKt$TRACE -g /var/run/hostapd-global -G $GROUP -f $LOGDIR/hostapd &
>  HPID=$!
> -echo $HPID > $LOGDIR/hostapd-test.pid
> +
> +# Sleep a bit, otherwise pgrep may run before the child is forked
> +sleep 0.1
> +pgrep -P $HPID > $LOGDIR/hostapd-test.pid

This breaks inside-VM testing where sudo is a dummy script (see
tests/hwsim/inside.sh generating the sudo scripts). In addition, sudo is
documented to relay signals. Could you please clarify what you are
trying to fix here?

That $VALGRIND_HAPD part could be a yet another program in the chain, so
it might make more sense to to determine the actual hostapd PID through
a new test command on the control interface to make this more robust.
Andrei Otcheretianski Oct. 21, 2018, 2:02 p.m. | #2
> This breaks inside-VM testing where sudo is a dummy script (see
> tests/hwsim/inside.sh generating the sudo scripts). In addition, sudo is
> documented to relay signals. Could you please clarify what you are trying to fix
> here?

Indeed the documentation of sudo says that the signals supposed to be relayed, but
on some of our machines it doesn't happen. I didn't really managed to understand why.
As the result ap_config_reload and ap_config_reload_file tests were failing.
I missed the part regarding the VM, I guess the simple solution is just to tell the start.sh
that it runs inside a VM. I'll re-send a fixed version.
> 
> That $VALGRIND_HAPD part could be a yet another program in the chain, so it
> might make more sense to to determine the actual hostapd PID through a new
> test command on the control interface to make this more robust.

I tested it with valgrind and it works.
Instead of adding a new command, I tried to use an existing "-P" option, so hostapd would write
a pid file itself. The problem here is that it requires also "-B" option, which changes the current working
directory and this causes other failures. So at the end I ended up with more changes, which are probably
an overkill for this.

Andrei
> 
> --
> Jouni Malinen                                            PGP id EFC895FA

Patch

diff --git a/tests/hwsim/start.sh b/tests/hwsim/start.sh
index 527ee22..7682fce 100755
--- a/tests/hwsim/start.sh
+++ b/tests/hwsim/start.sh
@@ -121,7 +121,10 @@  sudo $(printf -- "$VALGRIND_WPAS" 5) $WPAS -g /tmp/wpas-wlan5 -G$GROUP \
     -ddKt$TRACE -f $LOGDIR/log5 &
 sudo $VALGRIND_HAPD $HAPD -ddKt$TRACE -g /var/run/hostapd-global -G $GROUP -f $LOGDIR/hostapd &
 HPID=$!
-echo $HPID > $LOGDIR/hostapd-test.pid
+
+# Sleep a bit, otherwise pgrep may run before the child is forked
+sleep 0.1
+pgrep -P $HPID > $LOGDIR/hostapd-test.pid
 
 if [ -x $HLR_AUC_GW ]; then
     cp $DIR/auth_serv/hlr_auc_gw.milenage_db $LOGDIR/hlr_auc_gw.milenage_db