On 2 May 2016 at 09:04, Darrell Ball <dlu...@gmail.com> wrote: > On Mon, May 2, 2016 at 8:39 AM, William Tu <u9012...@gmail.com> wrote: > >> Hi Darrel, >> >> # Delete namespaces from the running OS >>>> m4_define([DEL_NAMESPACES], >>>> [m4_foreach([ns], [$@], >>>> - [ip netns del ns >>>> -]) >>>> + [if ip netns list | grep ns > /dev/null; then >>>> + ip netns del ns >>>> + fi >>>> + ]) >>>> ] >>>> ) >>>> >>> >>> Do we want to suppress an error on deletion in general ? >>> >>> >> No, I think it won't suppress errors on deletion. >> > > > Just to be clear, what the comment means is that if: > 1) DEL_NAMESPACE is called and there is no such ns, then this may be an > error with the surrounding code (i.e. a bug) or maybe the test itself. > > 2) Hence the above code in DEL_NAMESPACE would make the bug less > visible since there would be no visible complaint on trying to delete a ns > that does not exist
I was actually just wondering about why we need DEL_NAMESPACES. Originally, if you did a CTRL+C in the middle of the test, then cleanup would not properly occur so you'd end up with all of these test namespaces still existing. By deleting all of the specified namespaces at the start of ADD_NAMESPACES, it would allow the test to proceed without forcing the user to go through and delete all of the namespaces. However, if we were to queue up namespace deletion using on_exit "ip netns delete foo" immediately after creation, then the above issue should not exist, so maybe we could get rid of DELETE_NAMESPACES? In general I've advocated in the tests that while the test-writer needs to specify things like ADD_NAMESPACES(), those commands will queue up the cleanup to ensure that whether the test passes or fails, the system is left in a tidy state. This means that it is not necessary inside of tests to add the DELETE_NAMESPACES() towards the end (which would only execute if the rest of the test was successful). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
