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

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



On Mon, Dec 28, 2009 at 04:35:56PM +0100, Matthias Bolte wrote:
> 2009/12/23 Diego Elio Pettenò <flameeyes gmail com>:
> > 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 googlemail com>
> 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


-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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