From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEKqF-0005h7-FA for qemu-devel@nongnu.org; Fri, 14 Feb 2014 10:38:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WEKq9-00023j-5u for qemu-devel@nongnu.org; Fri, 14 Feb 2014 10:38:15 -0500 Received: from cantor2.suse.de ([195.135.220.15]:48398 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEKq8-00023X-V9 for qemu-devel@nongnu.org; Fri, 14 Feb 2014 10:38:09 -0500 Message-ID: <52FE385D.1090003@suse.de> Date: Fri, 14 Feb 2014 16:38:05 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1391944901-23475-1-git-send-email-afaerber@suse.de> <20140214144354.GJ17391@stefanha-thinkpad.redhat.com> In-Reply-To: <20140214144354.GJ17391@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qtest: Unlink pid file before reading from QMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, Stefan Hajnoczi Am 14.02.2014 15:43, schrieb Stefan Hajnoczi: > On Sun, Feb 09, 2014 at 12:21:41PM +0100, Andreas F=E4rber wrote: >> Despite 1ad3c6abc0d67e00b84abaa5527bc64b70ca2205, supplying invalid >> arguments to the QEMU process still leaked a /tmp/qtest-*.pid file. >> >> Fix this by reordering the reading and unlinking to before reading fro= m >> QMP socket, which relies on a running process. >> >> Cc: Stefan Hajnoczi >> Signed-off-by: Andreas F=E4rber >> --- >> tests/libqtest.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/tests/libqtest.c b/tests/libqtest.c >> index c9a4f89..9433782 100644 >> --- a/tests/libqtest.c >> +++ b/tests/libqtest.c >> @@ -157,14 +157,14 @@ QTestState *qtest_init(const char *extra_args) >> s->irq_level[i] =3D false; >> } >> =20 >> - /* Read the QMP greeting and then do the handshake */ >> - qtest_qmp_discard_response(s, ""); >> - qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"= ); >> - >> s->qemu_pid =3D read_pid_file(pid_file); >> unlink(pid_file); >> g_free(pid_file); >> =20 >> + /* Read the QMP greeting and then do the handshake */ >> + qtest_qmp_discard_response(s, ""); >> + qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"= ); >> + >=20 > Hmm...the original ordering was intentional. >=20 > In order to avoid race conditions between QEMU creating the pid file an= d > qtest reading the pid file, we wait until socket communication with QEM= U > has been performed. That way we're sure the pid file already exists. >=20 > I think the tests can now fail due to the race condition. I just verified that this patch did not slip into my pull. :) Can you please propose an alternative solution or patch? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg