Re: [Qemu-trivial] [PATCH] tests/boot-sector: Use mkstemp() to create a unique file name

[Thomas Huth <thuth@redhat.com>]

On 11.10.2016 15:38, Peter Maydell wrote:
> On 11 October 2016 at 14:32, Thomas Huth <thuth@redhat.com> wrote:
>> The pxe-test is run for three different targets now (x86_64, i386
>> and ppc64), and the bios-tables-test is run for two targets (x86_64
>> and i386). But each of the tests is using an invariant name for the
>> disk image with the boot sector code - so if the tests are running
>> in parallel, there is a race condition that they destroy the disk
>> image of a parallel test program. Let's use mkstemp() to create
>> unique temporary files here instead.
>>
>> Reported-by: Sascha Silbe <x-qemu@se-silbe.de>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/bios-tables-test.c | 2 +-
>>  tests/boot-sector.c      | 9 +++++++--
>>  tests/boot-sector.h      | 4 ++--
>>  tests/pxe-test.c         | 2 +-
>>  4 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>> index 6ea2b6d..812f830 100644
>> --- a/tests/bios-tables-test.c
>> +++ b/tests/bios-tables-test.c
>> @@ -112,7 +112,7 @@ typedef struct {
>>      g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
>>  } while (0)
>>
>> -static const char *disk = "tests/acpi-test-disk.raw";
>> +static char disk[] = "tests/acpi-test-disk-XXXXXX";
>>  static const char *data_dir = "tests/acpi-test-data";
>>  #ifdef CONFIG_IASL
>>  static const char *iasl = stringify(CONFIG_IASL);
>> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
>> index e3193c0..e75572f 100644
>> --- a/tests/boot-sector.c
>> +++ b/tests/boot-sector.c
>> @@ -69,10 +69,15 @@ static uint8_t boot_sector[0x7e000] = {
>>  };
>>
>>  /* Create boot disk file.  */
>> -int boot_sector_init(const char *fname)
>> +int boot_sector_init(char *fname)
>>  {
>> -    FILE *f = fopen(fname, "w");
>> +    FILE *f = NULL;
>> +    int fd;
>>
>> +    fd = mkstemp(fname);
>> +    if (fd != -1) {
>> +        f = fdopen(fd, "w");
>> +    }
> 
> Given that we're only writing a pile of bytes, we could
> just use write() and close() in this function rather
> than doing an fdopen() to use fwrite() and fclose().

You're right, that sounds nicer, so please ignore this version, I'll
send an update.

I think we might also need to increase the default timeout in
boot_sector_test() a little bit, since the pxe test is really rather
slow on ppc64 ... by increasing the timeout, we can make sure that it
also still passes on machines with high CPU load, so I'll add a patch to
do that, too.

 Thomas