Re: [Qemu-devel] [PATCH 0/2] qtest: don't leak pid files and UNIX domain sockets

[Andreas Färber]

Am 03.02.2014 10:54, schrieb Stefan Hajnoczi:
> On Fri, Jan 31, 2014 at 12:07:34AM +0000, Peter Maydell wrote:
>> On 21 November 2013 11:03, Stefan Hajnoczi <address@hidden> wrote:
>>> GLib uses abort(3) to exit failed test cases.  As a result, the pid file and
>>> UNIX domain sockets for a running test are leaked upon failure.
>>>
>>> Since abort(3) does not call atexit(3) handler functions, we could set up a
>>> SIGABRT handler that performs cleanup.  But there are other conditions where
>>> processes die, like SIGSEGV or SIGBUS.
>>>
>>> Let's unlink pid files and UNIX domain sockets as soon as the QEMU process 
>>> has
>>> initialized and connections have been made.  This eliminates the 
>>> possibility of
>>> leaking these files.
>>
>> So looking back through mailing list history suggests that these patches
>> are supposed to avoid intermittent make check failures like:
>>
>> TEST: tests/qom-test... (pid=5078)
>>   /i386/qom/none:                                                      **
>> ERROR:/home/petmay01/linaro/qemu-for-merges/tests/libqtest.c:71:init_socket:
>> assertion failed (ret != -1): (-1 != -1)
>> FAIL
>> GTester: last random seed: R02S79ea313790bc9a8b21d9af5ed55c2fff
>> (pid=5080)
>>   /i386/qom/pc:                                                        OK
>>   /i386/qom/isapc:                                                     OK
>>   /i386/qom/q35:                                                       OK
>> FAIL: tests/qom-test
>>
>> but this patch series doesn't actually say that's what it's for,
>> so does it fix that kind of error?
> 
> I still think we should merge these patches :).

+1

As an explanation, the temporary files contain the PID. When they remain
behind due to test failure *and* the PID wraps around and file names
thus happen to match, the error was triggered, and thereby not on each
run but seemingly "sometimes".

I am not 100% familiar with the unlinking and code ordering here, but it
had looked sane to me back when I looked at it, I just didn't feel
confident enough for a Reviewed-by. I could give it a spin and add a
Tested-by if that reassures PMM.

Andreas

>  Are you happy to merge
> them?
> 
> Stefan
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg