Re: [libvirt] [PATCH 3/3] When testing qemu, unset the SDL driver environment variables.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/1/18 Daniel Veillard <veillard@xxxxxxxxxx>:
> On Mon, Dec 28, 2009 at 04:35:56PM +0100, Matthias Bolte wrote:
>> 2009/12/23 Diego Elio Pettenò <flameeyes@xxxxxxxxx>:
>> > With these variables present, the tests would fail because they are not
>> > expected.
>> > ---
>> >  tests/qemuxml2argvtest.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> > index 3b0aa2b..b64dc6a 100644
>> > --- a/tests/qemuxml2argvtest.c
>> > +++ b/tests/qemuxml2argvtest.c
>> > @@ -188,6 +188,8 @@ mymain(int argc, char **argv)
>> >     unsetenv("TMPDIR");
>> >     unsetenv("LD_PRELOAD");
>> >     unsetenv("LD_LIBRARY_PATH");
>> > +    unsetenv("SDL_AUDIODRIVER");
>> > +    unsetenv("SDL_VIDEODRIVER");
>> >
>> >     DO_TEST("minimal", QEMUD_CMD_FLAG_NAME);
>> >     DO_TEST("machine-aliases1", 0);
>> > --
>> > 1.6.6.rc4
>> >
>>
>> You're right, all envvars copied in qemudBuildCommandLine should be
>> unset or set to known values in this testcase to make sure the test is
>> not affected by the actual envvar values.
>>
>> But SDL_VIDEODRIVER is not copied in qemudBuildCommandLine and
>> QEMU_AUDIO_DRV is copied but not unset in the test.
>>
>> I suggest the attached patch.
>>
>> Matthias
>
>> From 891e786c4e0863668b3b5b2e1be4f1acfd2b0f97 Mon Sep 17 00:00:00 2001
>> From: Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
>> Date: Mon, 28 Dec 2009 16:21:15 +0100
>> Subject: [PATCH] Unset copied environment variables in qemuxml2argvtest
>>
>> The test expected all environment variables copied in qemudBuildCommandLine
>> to have known values. So all of them have to be either set to a known value
>> or be unset. SDL_VIDEODRIVER and QEMU_AUDIO_DRV are not handled at all but
>> should be handled. Unset both, otherwise the test will fail if they are set
>> in the testing environment.
>>
>> * src/qemu/qemu_conf.c: add a comment about copied environment variables
>>   and qemuxml2argvtest
>> * tests/qemuxml2argvtest.c: unset SDL_VIDEODRIVER and QEMU_AUDIO_DRV
>> ---
>>  src/qemu/qemu_conf.c     |    2 ++
>>  tests/qemuxml2argvtest.c |    5 +++++
>>  2 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index f53c1f7..1f22826 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -2034,6 +2034,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
>>          qenv[qenvc++] = envval;                                         \
>>      } while (0)
>>
>> +    /* Make sure to unset or set all envvars in qemuxml2argvtest.c that
>> +     * are copied here using this macro, otherwise the test may fail */
>>  #define ADD_ENV_COPY(envname)                                           \
>>      do {                                                                \
>>          char *val = getenv(envname);                                    \
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 3b0aa2b..4e97294 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -181,6 +181,9 @@ mymain(int argc, char **argv)
>>  #define DO_TEST(name, extraFlags)                       \
>>          DO_TEST_FULL(name, extraFlags, NULL)
>>
>> +    /* Unset or set all envvars here that are copied in qemudBuildCommandLine
>> +     * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
>> +     * values for these envvars */
>>      setenv("PATH", "/bin", 1);
>>      setenv("USER", "test", 1);
>>      setenv("LOGNAME", "test", 1);
>> @@ -188,6 +191,8 @@ mymain(int argc, char **argv)
>>      unsetenv("TMPDIR");
>>      unsetenv("LD_PRELOAD");
>>      unsetenv("LD_LIBRARY_PATH");
>> +    unsetenv("QEMU_AUDIO_DRV");
>> +    unsetenv("SDL_AUDIODRIVER");
>>
>>      DO_TEST("minimal", QEMUD_CMD_FLAG_NAME);
>>      DO_TEST("machine-aliases1", 0);
>
>  ACK,
>
> Daniel
>
>

Thanks, pushed.

Matthias

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]