Re: [Qemu-devel] [PATCH] qtest: Unlink pid file before reading from QMP

[Stefan Hajnoczi <stefanha@gmail.com>]

On Sun, Feb 09, 2014 at 12:21:41PM +0100, Andreas Färber 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 from
> QMP socket, which relies on a running process.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  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] = false;
>      }
>  
> -    /* 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 = read_pid_file(pid_file);
>      unlink(pid_file);
>      g_free(pid_file);
>  
> +    /* Read the QMP greeting and then do the handshake */
> +    qtest_qmp_discard_response(s, "");
> +    qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }");
> +

Hmm...the original ordering was intentional.

In order to avoid race conditions between QEMU creating the pid file and
qtest reading the pid file, we wait until socket communication with QEMU
has been performed.  That way we're sure the pid file already exists.

I think the tests can now fail due to the race condition.

Stefan