| 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 | expand |
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.
> 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
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
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(-)