[Top][All Lists]

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

Re: [Qemu-devel] [PATCH 19/24] console: add and use qemu_display_find_de

From: Darren Kenny
Subject: Re: [Qemu-devel] [PATCH 19/24] console: add and use qemu_display_find_default
Date: Fri, 17 Nov 2017 14:16:50 +0000
User-agent: NeoMutt/20171027

Hi Gerd,

Thanks for clarifying things for me.

On Fri, Nov 17, 2017 at 02:24:54PM +0100, Gerd Hoffmann wrote:

> -        dpy.type = DISPLAY_TYPE_NONE;
> +        if (!qemu_display_find_default(&dpy)) {
> +            dpy.type = DISPLAY_TYPE_NONE;
> +#if defined(CONFIG_VNC)
> +            vnc_parse("localhost:0,to=99,id=default", &error_abort);
> #endif

Mostly some questions on this:

- I'm curious, why is VNC not just one of the ones searched for in

vnc is a bit different.  It's a remote display protocol, not a (local)
user interface.  There is no DISPLAY_TYPE_VNC for that reason (without
this series too).  And IMO it was a bad idea to add -display vnc=$config
(same as -vnc $config) in the first place.

There can be only one user interface instance, i.e. you can't have gtk
and sdl at the same time.  With vnc (and spice) that works though, i.e.
you can start qemu with some local user interface and enable vnc or
spice (even both) remote access at the same time.

Ah, wasn't sure about being able to do both a local UI and VNC -
I've only ever used one or the other :)

OK, so the odd thing then is the check for !remote_display earlier
on in the function (missing from the quote above) which seems to end
up initializing VNC (albeit with localhost) when CONFIG_VNC is
defined, but no other local display type has been selected.

But, I suppose it's not strictly incorrect with the 'localhost' :)

- What if there is another display type added? Would it be added to
 qemu_display_find_default() or in this if statement?

qemu_display_find_default() most likely,
but depends on what exactly it is.

OK, so neither SPICE or VNC really fit into this patchset because
they are seen as additional to the display type, not the actual
display type - SPICE is even more special too given the dependency
on a guest VM driver to really function correctly.

I wonder should there be a, similar, qemu_display_find_remote_default()
which would be used when no local display is defined. And that would
use a similar mechanism to permit selecting VNC or SPICE via
registration mechanisms over rather than the #ifdef style for remote
displays that will remain ever after this patchset (certainly could
be another patchset beyond this one).

Maybe it's not worth it, given that VNC is really he only remote UI
module you can use.

- Is there a reason that VNC doesn't follow the same registration
 mechanism you're proposing here (as in you've no changes to that
 module in this patch set).

See above ;)

- In the spirit of encapsulation, would it not make sense for the
 vnc_parse() to be in the VNC code itself as a fall back if no
 specific options are given to the -vnc option?

It's not that simple.  The default vnc server is started only in case
no (graphical) local user interface is available, so it's not something
the vnc code can figure purely on its own.

Fair enough.

Just a thought, but maybe the specific string could be defined in
vnc.h instead of a string here at least? (e.g.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]