[ovs-dev] [PATCH V2 4/4] tests: Fix fail of OVS_APP_EXIT_AND_WAIT on Windows

[Lance Richardson]

----- Original Message -----
> From: "Joe Stringer" <joe at ovn.org>
> To: "Lance Richardson" <lrichard at redhat.com>
> Cc: dev at openvswitch.org, "Paul Boca" <pboca at cloudbasesolutions.com>
> Sent: Thursday, June 2, 2016 9:41:23 PM
> Subject: Re: [ovs-dev] [PATCH V2 4/4] tests: Fix fail of OVS_APP_EXIT_AND_WAIT on Windows
> 

> Bear with me, I'm a little confused - firstly, about why the TMPPID
> stuff is supposed to work at all, and secondly why the behaviour
> differs between platforms.
> 
> One one hand, if the user of OVS_APP_EXIT_AND_WAIT() passes
> `pwd`/unixctl, wouldn't the path then become
> $OVS_RUNDIR/`pwd`/unixctl.pid ? That doesn't seem like a valid path to
> read to acquire a PID. In fact, when I removed the "2>/dev/null" from
> the TMPPID line, I found some tests that would quietly log that the
> file doesn't exist - and rightly so. This suggests that the
> $OVS_RUNDIR part of the cat command should be removed.
> 
> On the other hand, if that command returned effectively an empty
> string, then I'd expect the kill command would become something like
> "kill -0", which should print the usage and return a non-zero exit
> code. Because the exit code is nonzero, the OVS_WAIT_WHILE should exit
> immediately, so the test should continue. But this doesn't seem to be
> the behaviour on windows? Maybe one of my assumptions here is a bit
> off.
> 
> Lance, it seems like the "$OVS_RUNDIR" part of these commands was
> introduced by some of the changes you made around gcov support,
> specifically commits d9c8c57c48a2 ("tests: consistently use
> OVS_APP_EXIT_AND_WAIT() for daemon termination") and f9b11f2a09b4
> ("tests: Make OVS_APP_EXIT_AND_WAIT() wait for process termination").
> As far as I can tell, they're broken for a bunch of the tests. I
> started investigating by applying the diff below, and I can see that
> there's a lot of failures locating the pidfiles. Would you be able to
> take a look?
> 

OK, it appears that I was probably a little too focused on the ~250
locations where OVS_APP_EXIT_AND_WAIT() is invoked with a daemon
name (where the TMPPID stuff works fine) and not enough to the ~8
locations where it's invoked with the path of a unixctl socket.

Ben's suggestion to have the Windows-specfic kill() function emulate
the kill command's handling of "kill -0" should make this macro
behave as it does on other systems, but I'm wondering if it might
also be good to create a separate macro for the unixctl case. Ideally
there would be a way to derive the pid from the unixctl socket path,
but I believe we can't assume the existence of things like fuser or
lsof even on Linux or BSD systems. Or maybe this variant could
take both the unixctl socket path and pid file path as parameters.

Maybe something like:

dnl OVS_APP_EXIT_AND_WAIT_UNIXCTL(SOCKET PIDFILE)
dnl
dnl Ask the daemon associated with SOCKET to exit,
dnl via ovs-appctl, and then wait for it to exit.
m4_define([OVS_APP_EXIT_AND_WAIT_UNIXCTL],
  [TMPPID=$(cat $2 2>/dev/null)
   AT_CHECK([ovs-appctl -t $1 exit])
   OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])])

and change these spots to use the new macro:

daemon.at:  OVS_APP_EXIT_AND_WAIT([`pwd`/unixctl])
ovsdb-monitor.at:   OVS_APP_EXIT_AND_WAIT(["`pwd`"/unixctl])
ovsdb-server.at:  [OVS_APP_EXIT_AND_WAIT(["`pwd`"/unixctl])])
ovs-vswitchd.at:OVS_APP_EXIT_AND_WAIT([`pwd`/unixctl])
ovs-vswitchd.at:OVS_APP_EXIT_AND_WAIT([`pwd`/unixctl])
ovs-vswitchd.at:OVS_APP_EXIT_AND_WAIT([`pwd`/unixctl])
vlog.at:OVS_APP_EXIT_AND_WAIT([test-unixctl])
vlog.at:OVS_APP_EXIT_AND_WAIT([test-unixctl])

It looks like some of ovs-vswitchd.at cases would need to changed to
start ovs-vswitchd with "--pidfile xxx" in order to have a pidfile.

If that sounds reasonable, I can work up the patches to do it.

Thanks,

   Lance