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

[Stefan Hajnoczi]

On Fri, Feb 14, 2014 at 4:38 PM, Andreas Färber <address@hidden> wrote:
> Am 14.02.2014 15:43, schrieb Stefan Hajnoczi:
>> 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 <address@hidden>
>>> Signed-off-by: Andreas Färber <address@hidden>
>>> ---
>>>  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.
>
> I just verified that this patch did not slip into my pull. :)
>
> Can you please propose an alternative solution or patch?

I have an idea to get rid of the pid file entirely.  Will send a patch.

Stefan