| Message ID | 20170608173048.27271-1-fbl@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jun 08, 2017 at 02:30:48PM -0300, Flavio Leitner wrote: > The daemon is killed leaving resources behind when a test fails. > This fixes to first signal the daemon to exit gracefully. > > Suggested-by: Joe Stringer <joe@ovn.org> > Fixes: 0f28164be02ac ("netdev-linux: make tap devices persistent") > Signed-off-by: Flavio Leitner <fbl@redhat.com> > --- > tests/ofproto-macros.at | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > index faff5b0..5ac5d05 100644 > --- a/tests/ofproto-macros.at > +++ b/tests/ofproto-macros.at > @@ -323,6 +323,9 @@ m4_define([_OVS_VSWITCHD_START], > AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) > AT_CAPTURE_FILE([ovs-vswitchd.log]) > on_exit "kill `cat ovs-vswitchd.pid`" > + dnl Wait for the daemon to exit gracefully > + on_exit "for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break; sleep 0.1 || sleep 1; done" > + on_exit "ovs-appctl -t ovs-vswitchd exit --cleanup" Thanks for the patch. At first, I thought that this did the steps in the wrong order, but "on_exit" reverses the order. It would be less surprising to do this with just one call to on_exit, e.g. on_exit ' ovs-appctl -t ovs-vswitchd exit --cleanup for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break sleep 0.1 || sleep 1 done kill `cat ovs-vswitchd.pid` ' Actually, I think that all of this could be put in a shell function: kill_ovs_vswitchd() { ovs-appctl -t ovs-vswitchd exit --cleanup for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break sleep 0.1 || sleep 1 done kill `cat ovs-vswitchd.pid` } and then just "on_exit kill_ovs_vswitchd". Maybe that is the best approach. Thanks, Ben.
On Thu, Jun 08, 2017 at 10:53:05AM -0700, Ben Pfaff wrote: > On Thu, Jun 08, 2017 at 02:30:48PM -0300, Flavio Leitner wrote: > > The daemon is killed leaving resources behind when a test fails. > > This fixes to first signal the daemon to exit gracefully. > > > > Suggested-by: Joe Stringer <joe@ovn.org> > > Fixes: 0f28164be02ac ("netdev-linux: make tap devices persistent") > > Signed-off-by: Flavio Leitner <fbl@redhat.com> > > --- > > tests/ofproto-macros.at | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > > index faff5b0..5ac5d05 100644 > > --- a/tests/ofproto-macros.at > > +++ b/tests/ofproto-macros.at > > @@ -323,6 +323,9 @@ m4_define([_OVS_VSWITCHD_START], > > AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) > > AT_CAPTURE_FILE([ovs-vswitchd.log]) > > on_exit "kill `cat ovs-vswitchd.pid`" > > + dnl Wait for the daemon to exit gracefully > > + on_exit "for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break; sleep 0.1 || sleep 1; done" > > + on_exit "ovs-appctl -t ovs-vswitchd exit --cleanup" > > Thanks for the patch. > > At first, I thought that this did the steps in the wrong order, but > "on_exit" reverses the order. That's documented! :) > It would be less surprising to do this with just one call to on_exit, > e.g. > > on_exit ' > ovs-appctl -t ovs-vswitchd exit --cleanup > for i in 1 2 3 4 5 6 7 8 9; do > kill -0 `cat ovs-vswitchd.pid` || break > sleep 0.1 || sleep 1 > done > kill `cat ovs-vswitchd.pid` > ' > > Actually, I think that all of this could be put in a shell function: > > kill_ovs_vswitchd() { > ovs-appctl -t ovs-vswitchd exit --cleanup > for i in 1 2 3 4 5 6 7 8 9; do > kill -0 `cat ovs-vswitchd.pid` || break > sleep 0.1 || sleep 1 > done > kill `cat ovs-vswitchd.pid` > } > > and then just "on_exit kill_ovs_vswitchd". Maybe that is the best > approach. I agree, let me respin the patch.
On Thu, Jun 08, 2017 at 04:19:00PM -0300, Flavio Leitner wrote: > On Thu, Jun 08, 2017 at 10:53:05AM -0700, Ben Pfaff wrote: > > On Thu, Jun 08, 2017 at 02:30:48PM -0300, Flavio Leitner wrote: > > > The daemon is killed leaving resources behind when a test fails. > > > This fixes to first signal the daemon to exit gracefully. > > > > > > Suggested-by: Joe Stringer <joe@ovn.org> > > > Fixes: 0f28164be02ac ("netdev-linux: make tap devices persistent") > > > Signed-off-by: Flavio Leitner <fbl@redhat.com> > > > --- > > > tests/ofproto-macros.at | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > > > index faff5b0..5ac5d05 100644 > > > --- a/tests/ofproto-macros.at > > > +++ b/tests/ofproto-macros.at > > > @@ -323,6 +323,9 @@ m4_define([_OVS_VSWITCHD_START], > > > AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) > > > AT_CAPTURE_FILE([ovs-vswitchd.log]) > > > on_exit "kill `cat ovs-vswitchd.pid`" > > > + dnl Wait for the daemon to exit gracefully > > > + on_exit "for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break; sleep 0.1 || sleep 1; done" > > > + on_exit "ovs-appctl -t ovs-vswitchd exit --cleanup" > > > > Thanks for the patch. > > > > At first, I thought that this did the steps in the wrong order, but > > "on_exit" reverses the order. > > That's documented! :) I wrote the implementation *and* the documentation for this bit here. One forgets ;-) > > It would be less surprising to do this with just one call to on_exit, > > e.g. > > > > on_exit ' > > ovs-appctl -t ovs-vswitchd exit --cleanup > > for i in 1 2 3 4 5 6 7 8 9; do > > kill -0 `cat ovs-vswitchd.pid` || break > > sleep 0.1 || sleep 1 > > done > > kill `cat ovs-vswitchd.pid` > > ' > > > > Actually, I think that all of this could be put in a shell function: > > > > kill_ovs_vswitchd() { > > ovs-appctl -t ovs-vswitchd exit --cleanup > > for i in 1 2 3 4 5 6 7 8 9; do > > kill -0 `cat ovs-vswitchd.pid` || break > > sleep 0.1 || sleep 1 > > done > > kill `cat ovs-vswitchd.pid` > > } > > > > and then just "on_exit kill_ovs_vswitchd". Maybe that is the best > > approach. > > I agree, let me respin the patch. Thanks!
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index faff5b0..5ac5d05 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -323,6 +323,9 @@ m4_define([_OVS_VSWITCHD_START], AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) AT_CAPTURE_FILE([ovs-vswitchd.log]) on_exit "kill `cat ovs-vswitchd.pid`" + dnl Wait for the daemon to exit gracefully + on_exit "for i in 1 2 3 4 5 6 7 8 9; do kill -0 `cat ovs-vswitchd.pid` || break; sleep 0.1 || sleep 1; done" + on_exit "ovs-appctl -t ovs-vswitchd exit --cleanup" AT_CHECK([[sed < stderr ' /ovs_numa|INFO|Discovered /d /vlog|INFO|opened log file/d
The daemon is killed leaving resources behind when a test fails. This fixes to first signal the daemon to exit gracefully. Suggested-by: Joe Stringer <joe@ovn.org> Fixes: 0f28164be02ac ("netdev-linux: make tap devices persistent") Signed-off-by: Flavio Leitner <fbl@redhat.com> --- tests/ofproto-macros.at | 3 +++ 1 file changed, 3 insertions(+)