On Fri, Feb 14, 2014 at 4:38 PM, Andreas Färber <afaer...@suse.de> 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 <stefa...@redhat.com>
>>> Signed-off-by: Andreas Färber <afaer...@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.
>
> 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
