On Mar 12, 2011, at 10:54 AM, Andrew Evans wrote: > On 03/12/11 10:04, Ben Pfaff wrote: >> On Sat, Mar 12, 2011 at 9:51 AM, Andrew Evans<[email protected]> wrote: >>> On 03/12/11 08:51, Ben Pfaff wrote: >>>> >>>> On Sat, Mar 12, 2011 at 8:49 AM, Andrew Evans<[email protected]> wrote: >>>>> >>>>> This check can be fooled by PID wrapping. It might be better to have >>>>> ovs-monitor-ipsec flock(2) its PID file and for netdev-vport to check >>>>> that >>>>> the file is locked, since the lock will be released even if the process >>>>> crashes. Or maybe it's not worth worrying about. What do you think? >>>> >>>> read_pidfile() verifies that the pidfile is locked, so this is a >>>> non-issue. >>> >>> Thanks, I see that now. So the kill(2) is unnecessary, then? >> >> I don't see how it helps. >> >> I guess that it is a cut-and-paste from similar code in >> vswitchd/system-stats.c. >> I don't se how it helps there, either, though. > > Yeah, for my money it just obscures the fact that read_pidfile() does more > than just read. I think the return value of read_pidfile() should be the sole > test for running vs. not running.
As Ben noted, I just cribbed that from system-stats.c. I've removed the kill() in my patch and sent out a new patch to make a similar change in system-stats.c. They're in the same branch, so I'll just push them both when I get the okay on the new patch. Thanks for the feedback. --Justin _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
