From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52926) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WN20x-0000Z2-8F for qemu-devel@nongnu.org; Mon, 10 Mar 2014 11:21:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WN20q-0000DA-3f for qemu-devel@nongnu.org; Mon, 10 Mar 2014 11:21:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49671) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WN20p-0000Cz-Pu for qemu-devel@nongnu.org; Mon, 10 Mar 2014 11:21:08 -0400 Message-ID: <1394464877.3981.21.camel@localhost.localdomain> From: Marcel Apfelbaum In-Reply-To: <20140310150259.GB32400@stefanha-thinkpad.redhat.com> References: <1394453534-24334-1-git-send-email-marcel.a@redhat.com> <20140310150259.GB32400@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Mon, 10 Mar 2014 17:21:17 +0200 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/2] tests: Fix possible deadlock in qtest initialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: armbru@redhat.com, aliguori@amazon.com, afaerber@suse.de, stefanha@redhat.com, qemu-devel@nongnu.org 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 " 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? 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. - I am not sure how gtester formatting options can helps us here, because we *will* get an assert (after the first patch), but it would be in the qtestlib which is not the desired place. (it is not a qtestlib bug) Thanks, Marcel > > Stefan >