diff mbox series

[v1,1/3] package/qemu: add spice support

Message ID 20200629205002.8087-1-jared.bents@rockwellcollins.com
State New
Headers show
Series [v1,1/3] package/qemu: add spice support | expand

Commit Message

Jared Bents June 29, 2020, 8:50 p.m. UTC
update to add qemu spice support

Signed-off-by: Jared Bents <jared.bents@rockwellcollins.com>
---
 package/qemu/Config.in | 9 +++++++++
 package/qemu/qemu.mk   | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni June 29, 2020, 8:59 p.m. UTC | #1
On Mon, 29 Jun 2020 15:50:00 -0500
Jared Bents <jared.bents@rockwellcollins.com> wrote:

> update to add qemu spice support
> 
> Signed-off-by: Jared Bents <jared.bents@rockwellcollins.com>
> ---
>  package/qemu/Config.in | 9 +++++++++
>  package/qemu/qemu.mk   | 8 +++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/package/qemu/Config.in b/package/qemu/Config.in
> index 33d4cccd7b..3de73b69ec 100644
> --- a/package/qemu/Config.in
> +++ b/package/qemu/Config.in
> @@ -127,6 +127,15 @@ config BR2_PACKAGE_QEMU_SDL
>  comment "SDL frontend needs a toolchain w/ dynamic library"
>  	depends on BR2_STATIC_LIBS
>  
> +config BR2_PACKAGE_QEMU_SPICE
> +	bool "Enable Spice frontend"
> +	depends on BR2_PACKAGE_SPICE

I am wondering why for this one we are adding a sub-option and not
using automatic detection of BR2_PACKAGE_SPICE=y in the .mk file, like
you've done for VNC and usb-redir.

On the other hand, we do have already a bunch of sub-options to
configure Qemu. We're not very consistent here :/

Thomas
Jared Bents June 29, 2020, 9:05 p.m. UTC | #2
Hi Thomas,


On Mon, Jun 29, 2020 at 3:59 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 29 Jun 2020 15:50:00 -0500
> Jared Bents <jared.bents@rockwellcollins.com> wrote:
>
> > update to add qemu spice support
> >
> > Signed-off-by: Jared Bents <jared.bents@rockwellcollins.com>
> > ---
> >  package/qemu/Config.in | 9 +++++++++
> >  package/qemu/qemu.mk   | 8 +++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/qemu/Config.in b/package/qemu/Config.in
> > index 33d4cccd7b..3de73b69ec 100644
> > --- a/package/qemu/Config.in
> > +++ b/package/qemu/Config.in
> > @@ -127,6 +127,15 @@ config BR2_PACKAGE_QEMU_SDL
> >  comment "SDL frontend needs a toolchain w/ dynamic library"
> >       depends on BR2_STATIC_LIBS
> >
> > +config BR2_PACKAGE_QEMU_SPICE
> > +     bool "Enable Spice frontend"
> > +     depends on BR2_PACKAGE_SPICE
>
> I am wondering why for this one we are adding a sub-option and not
> using automatic detection of BR2_PACKAGE_SPICE=y in the .mk file, like
> you've done for VNC and usb-redir.
>
> On the other hand, we do have already a bunch of sub-options to
> configure Qemu. We're not very consistent here :/
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

I based the handling of the --enable-spice option off of an old
upstream attempt from Yann that I had found at
http://lists.buildroot.org/pipermail/buildroot/2012-December/063058.html
. I don't have a preference either way so let me know if you'd prefer
a suboption or have it automatically enabled if BR2_PACKAGE_SPICE is
enabled similarly to the other options I've added.

Thank you,
Jared
Romain Naour July 7, 2020, 1:30 p.m. UTC | #3
Hi Jared, Thomas,

Le 29/06/2020 à 23:05, Jared Bents a écrit :
> Hi Thomas,
> 
> 
> On Mon, Jun 29, 2020 at 3:59 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
>>
>> On Mon, 29 Jun 2020 15:50:00 -0500
>> Jared Bents <jared.bents@rockwellcollins.com> wrote:
>>
>>> update to add qemu spice support
>>>
>>> Signed-off-by: Jared Bents <jared.bents@rockwellcollins.com>
>>> ---
>>>  package/qemu/Config.in | 9 +++++++++
>>>  package/qemu/qemu.mk   | 8 +++++++-
>>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/package/qemu/Config.in b/package/qemu/Config.in
>>> index 33d4cccd7b..3de73b69ec 100644
>>> --- a/package/qemu/Config.in
>>> +++ b/package/qemu/Config.in
>>> @@ -127,6 +127,15 @@ config BR2_PACKAGE_QEMU_SDL
>>>  comment "SDL frontend needs a toolchain w/ dynamic library"
>>>       depends on BR2_STATIC_LIBS
>>>
>>> +config BR2_PACKAGE_QEMU_SPICE
>>> +     bool "Enable Spice frontend"
>>> +     depends on BR2_PACKAGE_SPICE
>>
>> I am wondering why for this one we are adding a sub-option and not
>> using automatic detection of BR2_PACKAGE_SPICE=y in the .mk file, like
>> you've done for VNC and usb-redir.
>>
>> On the other hand, we do have already a bunch of sub-options to
>> configure Qemu. We're not very consistent here :/

How to enable systems emulation or user-land emulation without sub-options?
What about removing BR2_PACKAGE_QEMU_SDL and BR2_PACKAGE_QEMU_FDT options and
handle the dependencies automatically?

>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
> 
> I based the handling of the --enable-spice option off of an old
> upstream attempt from Yann that I had found at
> http://lists.buildroot.org/pipermail/buildroot/2012-December/063058.html
> . I don't have a preference either way so let me know if you'd prefer
> a suboption or have it automatically enabled if BR2_PACKAGE_SPICE is
> enabled similarly to the other options I've added.

I guess we should enable spice only when spice package is enabled by the user
(automatic detection).

Best regards,
Romain

> 
> Thank you,
> Jared
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
diff mbox series

Patch

diff --git a/package/qemu/Config.in b/package/qemu/Config.in
index 33d4cccd7b..3de73b69ec 100644
--- a/package/qemu/Config.in
+++ b/package/qemu/Config.in
@@ -127,6 +127,15 @@  config BR2_PACKAGE_QEMU_SDL
 comment "SDL frontend needs a toolchain w/ dynamic library"
 	depends on BR2_STATIC_LIBS
 
+config BR2_PACKAGE_QEMU_SPICE
+	bool "Enable Spice frontend"
+	depends on BR2_PACKAGE_SPICE
+	help
+	  Say 'y' here to have QEMU support Spice as a (VNC-like) frontend.
+
+comment "Spice support requires spice-server"
+	depends on !BR2_PACKAGE_SPICE
+
 comment "Misc. features"
 
 config BR2_PACKAGE_QEMU_FDT
diff --git a/package/qemu/qemu.mk b/package/qemu/qemu.mk
index cb138fd488..0b76e432e0 100644
--- a/package/qemu/qemu.mk
+++ b/package/qemu/qemu.mk
@@ -112,6 +112,13 @@  else
 QEMU_OPTS += --disable-numa
 endif
 
+ifeq ($(BR2_PACKAGE_QEMU_SPICE),y)
+QEMU_OPTS += --enable-spice
+QEMU_DEPENDENCIES += spice
+else
+QEMU_OPTS += --disable-spice
+endif
+
 # Override CPP, as it expects to be able to call it like it'd
 # call the compiler.
 define QEMU_CONFIGURE_CMDS
@@ -143,7 +150,6 @@  define QEMU_CONFIGURE_CMDS
 			--disable-linux-io-uring \
 			--disable-cap-ng \
 			--disable-docs \
-			--disable-spice \
 			--disable-rbd \
 			--disable-libiscsi \
 			--disable-usb-redir \