On Mon, 2014-03-10 at 20:13 +0100, Stefan Hajnoczi wrote: > On Mon, Mar 10, 2014 at 05:21:17PM +0200, Marcel Apfelbaum wrote: > > On Mon, 2014-03-10 at 16:02 +0100, Stefan Hajnoczi wrote: > > > On Mon, Mar 10, 2014 at 02:12:12PM +0200, Marcel Apfelbaum wrote: > > > > 'socket_accept' waits for Qemu to init its unix socket. > > > > If Qemu encounters an error during command line parsing, > > > > it can exit before initializing the communication channel. > > > > It gets worse as the make check-qtest-* gets stuck without > > > > notifying which test exactly has problems, so debugging can > > > > be a challenge. > > > > > > > > The solution has two parts: > > > > - Use a timeout for the socket. > > > > - Expose a qtest_state_valid that checks that the connections > > > > with Qemu are OK. > > > > > > See below why I think qtest_state_valid() is unnecessary as a libqtest.h > > > API. > > > > > > > Asserting qtest_state_valid in each test after qtest_init > > > > is a must, as we need to trace which test failed. > > > > > > Inability to tell which qtest failed is a Makefile problem. The > > > solution is not to move all asserts to the outer-most level just so the > > > error message includes the test name. > > > > > > Either we need to invoke gtester separately for each test - that way the > > > Makefile can print "TEST <name>" for each binary. Or maybe gtester has > > > options for formatting output better. > > Hi Stefan, > > Thanks for the review. > > > > I am more concerned of PATCH 1/2, because it is a blocker for another > > series I am working on. > > I can resend only the first one which adds socket timeout and leaves the > > original assert. > > Would you be OK with this? > > Yes, that sounds fine. I posted comments on Patch 1 about ensuring the > sockets > are unlinked before we fail (to prevent leaking the UNIX domain socket nodes > in > the file system) - it does involve moving the assert to qtest_init(). Hi Stefan, Thanks a lot for your help! I already sent V2.
> > > Now regarding the issue you brought up (less important): > > - Tweaking the Makefile to run each qtest separately and not all tests per > > arch > > is a viable solution, however I am not familiar with the makefile magic > > and > > it will take me a lot of time to get into it. > > The following produces per-test output. This means you'll know which test > failed: Yes, it works like a charm! I added it :) Thanks, Marcel > > diff --git a/tests/Makefile b/tests/Makefile > index b17d41e..a8405c8 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -273,7 +273,7 @@ check-help: > @echo "changed with variable GTESTER_OPTIONS." > > SPEED = quick > -GTESTER_OPTIONS = -k $(if $(V),--verbose,-q) > +GTESTER_OPTIONS = -k #$(if $(V),--verbose,-q) > GCOV_OPTIONS = -n $(if $(V),-f,) > > # gtester tests, possibly with verbose output
