[Qemu-devel] [PATCH 1/2] build-sys: build ui-spice-app as a module

[Qemu-devel] [PATCH 1/2] build-sys: build ui-spice-app as a module

[Marc-André Lureau]

This reverts commit 45db1ac157 ("modules-test: ui-spice-app is not
built as module") and fixes commit d8aec9d9f1 ("display: add -display
spice-app launching a Spice client").

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/modules-test.c | 3 +++
 ui/Makefile.objs     | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/modules-test.c b/tests/modules-test.c
index f9de3afdb7..a8118e9042 100644
--- a/tests/modules-test.c
+++ b/tests/modules-test.c
@@ -52,6 +52,9 @@ int main(int argc, char *argv[])
 #endif
 #ifdef CONFIG_SDL
         "ui-", "sdl",
+#endif
+#if defined(CONFIG_SPICE) && defined(CONFIG_GIO)
+        "ui-", "spice-app",
 #endif
     };
     int i;
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index cc2bf5b180..ba39080edb 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -49,7 +49,9 @@ curses.mo-objs := curses.o
 curses.mo-cflags := $(CURSES_CFLAGS) $(ICONV_CFLAGS)
 curses.mo-libs := $(CURSES_LIBS) $(ICONV_LIBS)
 
-common-obj-$(call land,$(CONFIG_SPICE),$(CONFIG_GIO)) += spice-app.mo
+ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),yy)
+common-obj-$(if $(CONFIG_MODULES),m,y) += spice-app.mo
+endif
 spice-app.mo-objs := spice-app.o
 spice-app.mo-cflags := $(GIO_CFLAGS)
 spice-app.mo-libs := $(GIO_LIBS)
-- 
2.23.0

[Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Marc-André Lureau]

Fixes: eb062cfa733 ("tests: add module loading test")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/modules-test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/modules-test.c b/tests/modules-test.c
index a8118e9042..d1a6ace218 100644
--- a/tests/modules-test.c
+++ b/tests/modules-test.c
@@ -1,12 +1,14 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 
+const char common_args[] = "-nodefaults -machine none";
+
 static void test_modules_load(const void *data)
 {
     QTestState *qts;
     const char **args = (const char **)data;
 
-    qts = qtest_init(NULL);
+    qts = qtest_init(common_args);
     qtest_module_load(qts, args[0], args[1]);
     qtest_quit(qts);
 }
-- 
2.23.0

Re: [Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Thomas Huth]

On 27/08/2019 16.02, Marc-André Lureau wrote:
> Fixes: eb062cfa733 ("tests: add module loading test")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/modules-test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/modules-test.c b/tests/modules-test.c
> index a8118e9042..d1a6ace218 100644
> --- a/tests/modules-test.c
> +++ b/tests/modules-test.c
> @@ -1,12 +1,14 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  
> +const char common_args[] = "-nodefaults -machine none";

"-nodefaults" likely does not do much with the "none" machine ... OTOH,
it also does not hurt here.

>  static void test_modules_load(const void *data)
>  {
>      QTestState *qts;
>      const char **args = (const char **)data;
>  
> -    qts = qtest_init(NULL);
> +    qts = qtest_init(common_args);
>      qtest_module_load(qts, args[0], args[1]);
>      qtest_quit(qts);
>  }
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Peter, will you apply this directly as a fix for the travis builds, or
shall I take it through the qtest tree?

 Thomas

Re: [Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Peter Maydell]

On Tue, 27 Aug 2019 at 15:46, Thomas Huth <thuth@redhat.com> wrote:
>
> On 27/08/2019 16.02, Marc-André Lureau wrote:
> > Fixes: eb062cfa733 ("tests: add module loading test")
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/modules-test.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/modules-test.c b/tests/modules-test.c
> > index a8118e9042..d1a6ace218 100644
> > --- a/tests/modules-test.c
> > +++ b/tests/modules-test.c
> > @@ -1,12 +1,14 @@
> >  #include "qemu/osdep.h"
> >  #include "libqtest.h"
> >
> > +const char common_args[] = "-nodefaults -machine none";
>
> "-nodefaults" likely does not do much with the "none" machine ... OTOH,
> it also does not hurt here.
>
> >  static void test_modules_load(const void *data)
> >  {
> >      QTestState *qts;
> >      const char **args = (const char **)data;
> >
> > -    qts = qtest_init(NULL);
> > +    qts = qtest_init(common_args);
> >      qtest_module_load(qts, args[0], args[1]);
> >      qtest_quit(qts);
> >  }
> >
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> Peter, will you apply this directly as a fix for the travis builds, or
> shall I take it through the qtest tree?

I'm happy to take it directly (assuming patch 1/2 doesn't
have anything too big in it -- it hasn't arrived here yet.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] build-sys: build ui-spice-app as a module

[Thomas Huth]

On 27/08/2019 16.02, Marc-André Lureau wrote:
> This reverts commit 45db1ac157 ("modules-test: ui-spice-app is not
> built as module") and fixes commit d8aec9d9f1 ("display: add -display
> spice-app launching a Spice client").
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/modules-test.c | 3 +++
>  ui/Makefile.objs     | 4 +++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/modules-test.c b/tests/modules-test.c
> index f9de3afdb7..a8118e9042 100644
> --- a/tests/modules-test.c
> +++ b/tests/modules-test.c
> @@ -52,6 +52,9 @@ int main(int argc, char *argv[])
>  #endif
>  #ifdef CONFIG_SDL
>          "ui-", "sdl",
> +#endif
> +#if defined(CONFIG_SPICE) && defined(CONFIG_GIO)
> +        "ui-", "spice-app",
>  #endif
>      };
>      int i;
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index cc2bf5b180..ba39080edb 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -49,7 +49,9 @@ curses.mo-objs := curses.o
>  curses.mo-cflags := $(CURSES_CFLAGS) $(ICONV_CFLAGS)
>  curses.mo-libs := $(CURSES_LIBS) $(ICONV_LIBS)
>  
> -common-obj-$(call land,$(CONFIG_SPICE),$(CONFIG_GIO)) += spice-app.mo
> +ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),yy)
> +common-obj-$(if $(CONFIG_MODULES),m,y) += spice-app.mo
> +endif
>  spice-app.mo-objs := spice-app.o
>  spice-app.mo-cflags := $(GIO_CFLAGS)
>  spice-app.mo-libs := $(GIO_LIBS)
> 

Looks reasonable to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] build-sys: build ui-spice-app as a module

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> This reverts commit 45db1ac157 ("modules-test: ui-spice-app is not
> built as module") and fixes commit d8aec9d9f1 ("display: add -display
> spice-app launching a Spice client").
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/modules-test.c | 3 +++
>  ui/Makefile.objs     | 4 +++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/modules-test.c b/tests/modules-test.c
> index f9de3afdb7..a8118e9042 100644
> --- a/tests/modules-test.c
> +++ b/tests/modules-test.c
> @@ -52,6 +52,9 @@ int main(int argc, char *argv[])
>  #endif
>  #ifdef CONFIG_SDL
>          "ui-", "sdl",
> +#endif
> +#if defined(CONFIG_SPICE) && defined(CONFIG_GIO)
> +        "ui-", "spice-app",
>  #endif
>      };
>      int i;
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index cc2bf5b180..ba39080edb 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -49,7 +49,9 @@ curses.mo-objs := curses.o
>  curses.mo-cflags := $(CURSES_CFLAGS) $(ICONV_CFLAGS)
>  curses.mo-libs := $(CURSES_LIBS) $(ICONV_LIBS)
>
> -common-obj-$(call land,$(CONFIG_SPICE),$(CONFIG_GIO)) += spice-app.mo
> +ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),yy)
> +common-obj-$(if $(CONFIG_MODULES),m,y) += spice-app.mo
> +endif
>  spice-app.mo-objs := spice-app.o
>  spice-app.mo-cflags := $(GIO_CFLAGS)
>  spice-app.mo-libs := $(GIO_LIBS)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Fixes: eb062cfa733 ("tests: add module loading test")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/modules-test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tests/modules-test.c b/tests/modules-test.c
> index a8118e9042..d1a6ace218 100644
> --- a/tests/modules-test.c
> +++ b/tests/modules-test.c
> @@ -1,12 +1,14 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>
> +const char common_args[] = "-nodefaults -machine none";
> +
>  static void test_modules_load(const void *data)
>  {
>      QTestState *qts;
>      const char **args = (const char **)data;
>
> -    qts = qtest_init(NULL);
> +    qts = qtest_init(common_args);
>      qtest_module_load(qts, args[0], args[1]);
>      qtest_quit(qts);
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 27 Aug 2019 at 15:46, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 27/08/2019 16.02, Marc-André Lureau wrote:
>> > Fixes: eb062cfa733 ("tests: add module loading test")
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  tests/modules-test.c | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/modules-test.c b/tests/modules-test.c
>> > index a8118e9042..d1a6ace218 100644
>> > --- a/tests/modules-test.c
>> > +++ b/tests/modules-test.c
>> > @@ -1,12 +1,14 @@
>> >  #include "qemu/osdep.h"
>> >  #include "libqtest.h"
>> >
>> > +const char common_args[] = "-nodefaults -machine none";
>>
>> "-nodefaults" likely does not do much with the "none" machine ... OTOH,
>> it also does not hurt here.
>>
>> >  static void test_modules_load(const void *data)
>> >  {
>> >      QTestState *qts;
>> >      const char **args = (const char **)data;
>> >
>> > -    qts = qtest_init(NULL);
>> > +    qts = qtest_init(common_args);
>> >      qtest_module_load(qts, args[0], args[1]);
>> >      qtest_quit(qts);
>> >  }
>> >
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> Peter, will you apply this directly as a fix for the travis builds, or
>> shall I take it through the qtest tree?
>
> I'm happy to take it directly (assuming patch 1/2 doesn't
> have anything too big in it -- it hasn't arrived here yet.)

I've been testing this in my testing/next tree and as you're not back
until tomorrow I can have these sitting in the PR I'm pulling together
now if that's ok.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] tests: fix modules-test with no default machine

[Peter Maydell]

On Mon, 2 Sep 2019 at 10:52, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Tue, 27 Aug 2019 at 15:46, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> On 27/08/2019 16.02, Marc-André Lureau wrote:
> >> > Fixes: eb062cfa733 ("tests: add module loading test")
> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> > ---
> >> >  tests/modules-test.c | 4 +++-
> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/tests/modules-test.c b/tests/modules-test.c
> >> > index a8118e9042..d1a6ace218 100644
> >> > --- a/tests/modules-test.c
> >> > +++ b/tests/modules-test.c
> >> > @@ -1,12 +1,14 @@
> >> >  #include "qemu/osdep.h"
> >> >  #include "libqtest.h"
> >> >
> >> > +const char common_args[] = "-nodefaults -machine none";
> >>
> >> "-nodefaults" likely does not do much with the "none" machine ... OTOH,
> >> it also does not hurt here.
> >>
> >> >  static void test_modules_load(const void *data)
> >> >  {
> >> >      QTestState *qts;
> >> >      const char **args = (const char **)data;
> >> >
> >> > -    qts = qtest_init(NULL);
> >> > +    qts = qtest_init(common_args);
> >> >      qtest_module_load(qts, args[0], args[1]);
> >> >      qtest_quit(qts);
> >> >  }
> >> >
> >>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>
> >> Peter, will you apply this directly as a fix for the travis builds, or
> >> shall I take it through the qtest tree?
> >
> > I'm happy to take it directly (assuming patch 1/2 doesn't
> > have anything too big in it -- it hasn't arrived here yet.)
>
> I've been testing this in my testing/next tree and as you're not back
> until tomorrow I can have these sitting in the PR I'm pulling together
> now if that's ok.

I just got back and found that I'd put the patches through the
merge tests but hadn't done the final 'and push to master' part,
so I've done that now. I assume git will cope with them being
in your PR as well as in master...

thanks
-- PMM