[virt-tools-list] [PATCH 3/5] virtinst: add listen=none graphics option

[Marc-André Lureau]

Hi

----- Original Message -----
> On Thu, Apr 28, 2016 at 09:39:40AM -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > On Thu, Apr 28, 2016 at 02:22:09PM +0200, Marc-André Lureau wrote:
> > > > Add a special listen value to disable any extra display server
> > > > listening
> > > > socket. This is necessary now that qemu prevents starting a spice+virgl
> > > > VM with listening sockets (until spice allows remoting with virgl).
> > > > 
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > ---
> > > >  man/virt-install.pod                                     |  9
> > > >  +++++++--
> > > >  tests/cli-test-xml/compare/virt-install-many-devices.xml |  4 ++++
> > > >  tests/clitest.py                                         |  1 +
> > > >  virtinst/cli.py                                          | 11
> > > >  ++++++++++-
> > > >  4 files changed, 22 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/man/virt-install.pod b/man/virt-install.pod
> > > > index 0bc3d8c..0537693 100644
> > > > --- a/man/virt-install.pod
> > > > +++ b/man/virt-install.pod
> > > > @@ -964,8 +964,13 @@ Specify the spice tlsport.
> > > >  Address to listen on for VNC/Spice connections. Default is typically
> > > >  127.0.0.1
> > > >  (localhost only), but some hypervisors allow changing this globally
> > > >  (for
> > > >  example, the qemu driver default can be changed in
> > > >  /etc/libvirt/qemu.conf).
> > > > -Use 0.0.0.0 to allow access from other machines. This is use by 'vnc'
> > > > and
> > > > -'spice'
> > > > +Use 0.0.0.0 to allow access from other machines.
> > > > +
> > > > +Use 'none' to specify that the display server should not listen on any
> > > > +port. The display server can be accessed only locally through
> > > > +libvirt unix socket (virt-viewer with --attach for instance).
> > > > +
> > > > +This is used by 'vnc' and 'spice'
> > > >  
> > > >  =item B<keymap>
> > > >  
> > > > diff --git a/tests/cli-test-xml/compare/virt-install-many-devices.xml
> > > > b/tests/cli-test-xml/compare/virt-install-many-devices.xml
> > > > index e771cd3..d268cd9 100644
> > > > --- a/tests/cli-test-xml/compare/virt-install-many-devices.xml
> > > > +++ b/tests/cli-test-xml/compare/virt-install-many-devices.xml
> > > > @@ -214,6 +214,10 @@
> > > >        <gl enable="yes"/>
> > > >        <image compression="off"/>
> > > >      </graphics>
> > > > +    <graphics type="spice" autoport="no">
> > > > +      <gl enable="yes"/>
> > > > +      <image compression="off"/>
> > > > +    </graphics>
> > > 
> > > NACK, please don't do this ... it's not intuitive and it may confuse
> > > users.
> > > This wasn't designed in libvirt to start a domain with spice to not
> > > listen
> > > anywhere.  Even though it works, we shouldn't use it.  I'm working on
> > > patches,
> > > that will introduce new <listen type='fd'/> which will tell qemu that
> > > spice
> > > shouldn't listen and that a client have to pass FD in order to connect.
> > > I'll
> > > also block this particular case in libvirt while starting a domain, that
> > > no
> > > port
> > > was specified.  It's not even documented anywhere.
> > 
> > Isn't it simply a documentation issue then? I don't think listen type='fd'
> > is more intuitive either. Also changing this now will break existing
> > configuration relying on this behaviour.
> 
> The 'fd' is a working name, I'll probably change it to 'none' or something
> else.
> Well one shouldn't rely on anything that isn't mentioned anywhere and is base
> on
> testing or reviewing code.  Instead of erroring out I'll probably convert
> this
> case to <listen type='none'/> but you should wait with those patches.  I

Ok, please keep me in CC. Are you working on this for the next release? if not, I can help.

> don't
> like similar configurations anywhere, where for some values it listens on
> address but with one exception that if you set port=0 and autoport=no that it
> won't listen anywhere (even if live XML has a listen=127.0.0.1).  It's
> ambiguous
> and like I said it may confuse users.
> 
> For that XML you are using in this patch live XML looks like this:
> 
> ...
>     <graphics type='spice' autoport='no' listen='127.0.0.1'>
>       <listen type='address' address='127.0.0.1/>
>       <gl enable='yes'/>
>       <image compression='off'/>
>     </graphics>
> ...
> 
> which basically means that spice should listen on address 127.0.0.1 but no
> port
> was specified so does it really listen anywhere or what?

Isn't that a live XML "bug" instead?

> 
> I'm working on making this clear and to always represent a current state in
> the
> XML so everyone that uses libvirt knows exactly what's the configuration.

Sounds good

> 
> So in this case it should looks like this:
> 
> ...
>     <graphics type='spice'>
>       <listen type='none'/>
>       <gl enable='yes'/>
>       <image compression='off'/>
>     </graphics>
> ...

I like it, let me know how I can help. I hope we can have it in next libvirt release, since it's not possible to setup a spice+virgl VM anymore since qemu "Disallow use of gl + TCP port" now.

thanks