Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

[Thomas Huth]

On 11.10.2016 10:27, Sascha Silbe wrote:
> Dear Peter,
> 
> Peter Maydell <address@hidden> writes:
> 
>> On 10 October 2016 at 15:13, Peter Maydell <address@hidden> wrote:
>>> This failed 'make check' on aarch64 host (everything else was ok):
>>>
>>> TEST: tests/pxe-test... (pid=11699)
>>>   /ppc64/pxe/virtio:                                                   **
>>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>>> assertion failed (signature == SIGN
>>> ATURE): (0x00002020 == 0x0000dead)
>>> FAIL
>>> GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
>>> (pid=19477)
>>>   /ppc64/pxe/spapr-vlan:                                               **
>>> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
>>> assertion failed (signature == SIGN
>>> ATURE): (0x00002020 == 0x0000dead)
>>> FAIL
>>> GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
>>> (pid=24055)
>>> FAIL: tests/pxe-test
>>
>> Several subsequent test runs passed, so I'm inclined to suspect
>> this is not related to the pull request but is actually an
>> over-enthusiastic timeout and the build machine was heavily
>> loaded or something.
> 
> There's a race condition in the tests. Both the "i386" and the "x86_64"
> set of tests are creating and removing the same set of files:
> tests/acpi-test-disk.raw (hard-coded in tests/bios-tables-test.c) and
> tests/pxe-test-disk.raw (hard-coded in tests/pxe-test.c).

Hmm, it's likely even worse - the new ppc64 test is using the same file
name, too - with different content in the file. So if the tests are
running in parallel, they will likely disturb each other.

I think we should use mkstemp() instead for creating a unique file. Is
anybody working on a patch already? If not, I could have a look...

> FWIW, it's not obvious at first sight that the tests added to
> check-qtest-i386-y will also be included in check-qtest-x86_64-y. The
> line responsible for that is hiding in the midst of other assignments,
> without even a blank line separating it from the rest.
> 
> Assigning to a common variable first and then including it in both
> check-qtest-i386-y and check-qtest-x86_64-y would make it a lot easier
> to understand IMO.

Could you maybe provide a patch for this, Sascha? (at least for adding
some blank lines inbetween - currently it is really hard to read)

 Thomas

signature.asc
Description: OpenPGP digital signature