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

[Thomas Huth <thuth@redhat.com>]

On 11.10.2016 15:55, Greg Kurz wrote:
> On Tue, 11 Oct 2016 15:32:02 +0200
> 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");
>> +    }
> 
> Unless we really want to control how the temporary file is named (maybe for
> debugging purpose?), I have the impression that using tmpfile(3) would result
> in a simpler patch.

That unfortunately does not work here - the file name is needed for
starting the QEMU process later (see test_acpi_one() or test_pxe_one()
for details).

 Thomas