[Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
Currently, virgl support has to go through a local unix socket, trying
to connect to a VM using -spice gl through spice://localhost:5900 will
only result in a black screen.
This commit errors out when the user tries to start a VM with both GL
support and a port/tls-port set.
This would fit better in spice-server, but currently QEMU does not call
into spice-server when parsing 'gl' on its command line, so we have to
do this check in QEMU instead.
Signed-off-by: Christophe Fergeau
---
ui/spice-core.c | 4
1 file changed, 4 insertions(+)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 7987a4e..94f3236 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -844,6 +844,10 @@ void qemu_spice_init(void)
#ifdef HAVE_SPICE_GL
if (qemu_opt_get_bool(opts, "gl", 0)) {
+if ((port != 0) || (tls_port != 0)) {
+error_report("SPICE GL support is local-only for now and
incompatible with -spice port/tls-port");
+exit(1);
+}
if (egl_rendernode_init() == 0) {
display_opengl = 1;
}
--
2.5.0
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Mon, Mar 14, 2016 at 12:41 PM, Christophe Fergeau
wrote:
> Currently, virgl support has to go through a local unix socket, trying
> to connect to a VM using -spice gl through spice://localhost:5900 will
> only result in a black screen.
> This commit errors out when the user tries to start a VM with both GL
> support and a port/tls-port set.
> This would fit better in spice-server, but currently QEMU does not call
> into spice-server when parsing 'gl' on its command line, so we have to
> do this check in QEMU instead.
>
> Signed-off-by: Christophe Fergeau
Reviewed-by: Marc-André Lureau
> ---
> ui/spice-core.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 7987a4e..94f3236 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -844,6 +844,10 @@ void qemu_spice_init(void)
>
> #ifdef HAVE_SPICE_GL
> if (qemu_opt_get_bool(opts, "gl", 0)) {
> +if ((port != 0) || (tls_port != 0)) {
> +error_report("SPICE GL support is local-only for now and
> incompatible with -spice port/tls-port");
> +exit(1);
> +}
> if (egl_rendernode_init() == 0) {
> display_opengl = 1;
> }
> --
> 2.5.0
>
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Mo, 2016-03-14 at 12:41 +0100, Christophe Fergeau wrote: > Currently, virgl support has to go through a local unix socket, trying > to connect to a VM using -spice gl through spice://localhost:5900 will > only result in a black screen. > This commit errors out when the user tries to start a VM with both GL > support and a port/tls-port set. > This would fit better in spice-server, but currently QEMU does not call > into spice-server when parsing 'gl' on its command line, so we have to > do this check in QEMU instead. Hmm. It's something which we want support long-term though, by encoding those dma-bufs as video stream and send them off over tcp. I don't think this is a good idea long-term. And even as temporary stopgap: Can libvirt + virt-manager handle this? cheers, Gerd
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
Hi, On Mon, Mar 14, 2016 at 04:00:11PM +0100, Gerd Hoffmann wrote: > On Mo, 2016-03-14 at 12:41 +0100, Christophe Fergeau wrote: > > Currently, virgl support has to go through a local unix socket, trying > > to connect to a VM using -spice gl through spice://localhost:5900 will > > only result in a black screen. > > This commit errors out when the user tries to start a VM with both GL > > support and a port/tls-port set. > > This would fit better in spice-server, but currently QEMU does not call > > into spice-server when parsing 'gl' on its command line, so we have to > > do this check in QEMU instead. > > Hmm. It's something which we want support long-term though, by encoding > those dma-bufs as video stream and send them off over tcp. > > I don't think this is a good idea long-term. Yes, long-term we will want to remove this once spice gets support for this. Currently however, this imo makes it a bit easier to understand how everything is setup. Otherwise I expect people are going to just take their existing SPICE libvirt configuration listening on the network, add to it, try remote-viewer spice://localhost:5900 and wonder why they get a black screen, and not know whether this is because their guest does not support virgl, or their host, or because of some other issue, ... Having this as a stopgap ensures that they at least be informed this is not a valid usecase. > And even as temporary stopgap: Can libvirt + virt-manager handle this? libvirt can, I haven't tried virt-manager (I believe all the patches from Marc-André haven't been pushed yet) starts QEMU with -spice port=0 Christophe signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On 03/14/2016 05:41 AM, Christophe Fergeau wrote:
> Currently, virgl support has to go through a local unix socket, trying
> to connect to a VM using -spice gl through spice://localhost:5900 will
> only result in a black screen.
> This commit errors out when the user tries to start a VM with both GL
> support and a port/tls-port set.
> This would fit better in spice-server, but currently QEMU does not call
> into spice-server when parsing 'gl' on its command line, so we have to
> do this check in QEMU instead.
>
> Signed-off-by: Christophe Fergeau
> ---
> ui/spice-core.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 7987a4e..94f3236 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -844,6 +844,10 @@ void qemu_spice_init(void)
>
> #ifdef HAVE_SPICE_GL
> if (qemu_opt_get_bool(opts, "gl", 0)) {
> +if ((port != 0) || (tls_port != 0)) {
Overparenthesized; you could write:
if (port || tls_port) {
for the same effect with less typing.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Mon, Mar 14, 2016 at 09:41:34AM -0600, Eric Blake wrote:
> On 03/14/2016 05:41 AM, Christophe Fergeau wrote:
> > Currently, virgl support has to go through a local unix socket, trying
> > to connect to a VM using -spice gl through spice://localhost:5900 will
> > only result in a black screen.
> > This commit errors out when the user tries to start a VM with both GL
> > support and a port/tls-port set.
> > This would fit better in spice-server, but currently QEMU does not call
> > into spice-server when parsing 'gl' on its command line, so we have to
> > do this check in QEMU instead.
> >
> > Signed-off-by: Christophe Fergeau
> > ---
> > ui/spice-core.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/ui/spice-core.c b/ui/spice-core.c
> > index 7987a4e..94f3236 100644
> > --- a/ui/spice-core.c
> > +++ b/ui/spice-core.c
> > @@ -844,6 +844,10 @@ void qemu_spice_init(void)
> >
> > #ifdef HAVE_SPICE_GL
> > if (qemu_opt_get_bool(opts, "gl", 0)) {
> > +if ((port != 0) || (tls_port != 0)) {
>
> Overparenthesized; you could write:
>
> if (port || tls_port) {
>
> for the same effect with less typing.
Yeah I know I'm overly verbose with these tests, the parentheses make it
explicit that there are no operator priority issues, the != 0 emphasize
it's an integer type which is being handled.
I'll change it to your recommendation before sending a v2.
Christophe
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
Hi, > Yes, long-term we will want to remove this once spice gets support for > this. How can qemu figure whenever spice supports gl+tcp or not? cheers, Gerd
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Tue, Mar 15, 2016 at 02:09:47PM +0100, Gerd Hoffmann wrote: > Hi, > > > Yes, long-term we will want to remove this once spice gets support for > > this. > > How can qemu figure whenever spice supports gl+tcp or not? gl support is already enabled through a spice-server version check #if defined(CONFIG_OPENGL_DMABUF) # if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */ # define HAVE_SPICE_GL 1 # include "ui/egl-helpers.h" # include "ui/egl-context.h" # endif #endif We can do something similar once gl+tcp is available. Christophe signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Di, 2016-03-15 at 15:17 +0100, Christophe Fergeau wrote: > On Tue, Mar 15, 2016 at 02:09:47PM +0100, Gerd Hoffmann wrote: > > Hi, > > > > > Yes, long-term we will want to remove this once spice gets support for > > > this. > > > > How can qemu figure whenever spice supports gl+tcp or not? > > gl support is already enabled through a spice-server version check > > #if defined(CONFIG_OPENGL_DMABUF) > # if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */ > # define HAVE_SPICE_GL 1 > # include "ui/egl-helpers.h" > # include "ui/egl-context.h" > # endif > #endif Sure, qemu needed to figure whenever spice-server provides the gl API. > We can do something similar once gl+tcp is available. I don't expect adding gl+tcp support to spice needs changes in the spice-server API and qemu. So ifdef'ing on the spice-server version is bogous, especially as the version you compiled qemu against can be newer or older as the version used to run qemu. cheers, Gerd
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Tue, Mar 15, 2016 at 03:32:31PM +0100, Gerd Hoffmann wrote: > > We can do something similar once gl+tcp is available. > > I don't expect adding gl+tcp support to spice needs changes in the > spice-server API and qemu. So ifdef'ing on the spice-server version is > bogous, Hmm, I expected some changes, at least wrt options if the user needs to tweak the format of the video stream, hence the suggestion :) Things get trickier then. Adding a runtime spice_get_version() would not be that great either as the check would have to be updated with each spice-server release. I'd still like to have some failure when people try such configurations, gathering all the pieces is complicated enough, better to let people know when they try doing something that won't work. Christophe signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Fri, Mar 18, 2016 at 09:17:53AM +0100, Gerd Hoffmann wrote:
> On Mo, 2016-03-14 at 12:41 +0100, Christophe Fergeau wrote:
> > Currently, virgl support has to go through a local unix socket, trying
> > to connect to a VM using -spice gl through spice://localhost:5900 will
> > only result in a black screen.
> > This commit errors out when the user tries to start a VM with both GL
> > support and a port/tls-port set.
> > This would fit better in spice-server, but currently QEMU does not call
> > into spice-server when parsing 'gl' on its command line, so we have to
> > do this check in QEMU instead.
> >
> > Signed-off-by: Christophe Fergeau
>
> Picked up for ui patch queue.
Great thanks!
Fwiw, I've changed it locally to address Eric's comments, but did not
send it as v2 since a discussion was ongoing, here is what it looks in
my local clone now (but I'm fine with either version):
commit c0e10fb17a45a9da14d068a58af3a00e78f82403
Author: Christophe Fergeau
Date: Mon Mar 14 12:37:50 2016 +0100
spice: Disallow use of gl + TCP port
Currently, virgl support has to go through a local unix socket, trying
to connect to a VM using -spice gl through spice://localhost:5900 will
only result in a black screen.
This commit errors out when the user tries to start a VM with both GL
support and a port/tls-port set.
This would fit better in spice-server, but currently QEMU does not call
into spice-server when parsing 'gl' on its command line, so we have to
do this check in QEMU instead.
Signed-off-by: Christophe Fergeau
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 7987a4e..83950fe 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -844,6 +844,10 @@ void qemu_spice_init(void)
#ifdef HAVE_SPICE_GL
if (qemu_opt_get_bool(opts, "gl", 0)) {
+if (port || tls_port) {
+error_report("SPICE GL support is local-only for now and
incompatible with -spice port/tls-port");
+exit(1);
+}
if (egl_rendernode_init() == 0) {
display_opengl = 1;
}
Christophe
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH] spice: Disallow use of gl + TCP port
On Mo, 2016-03-14 at 12:41 +0100, Christophe Fergeau wrote: > Currently, virgl support has to go through a local unix socket, trying > to connect to a VM using -spice gl through spice://localhost:5900 will > only result in a black screen. > This commit errors out when the user tries to start a VM with both GL > support and a port/tls-port set. > This would fit better in spice-server, but currently QEMU does not call > into spice-server when parsing 'gl' on its command line, so we have to > do this check in QEMU instead. > > Signed-off-by: Christophe Fergeau Picked up for ui patch queue. thanks, Gerd
