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

Re: [libvirt] [PATCH] virsh: Add domdisplay command for VNC and SPICE



On Fri, Jun 22, 2012 at 01:18:49PM +0200, Michal Privoznik wrote:
> On 22.06.2012 12:07, Daniel P. Berrange wrote:
> > On Fri, Jun 22, 2012 at 10:19:06AM +0200, Michal Privoznik wrote:
> >> On 22.06.2012 07:38, Doug Goldstein wrote:
> >>> On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe cardoe com> wrote:
> >>>> Add a new 'domdisplay' command that provides a URI for both VNC and
> >>>> SPICE connections. Presently the 'vncdisplay' command provides you with
> >>>> the port info that QEMU is listening on but there is no counterpart for
> >>>> SPICE. Additionally this provides you with the bind address as specified
> >>>> in the XML, which the existing 'vncdisplay' lacks.
> >>>>
> >>>
> >>> I also toyed with the idea of adding this instead as an API call to
> >>> libvirt called something like virDomainDisplay() and allowing other
> >>> backends to advertise their URIs to the domain's display. I believe
> >>> VMWare has VMWare View which I would assume would have a method by
> >>> which to call it and provide the machine to connect to as command line
> >>> arguments. Then virsh would simply hook the 'domdisplay' command to
> >>> virDomainDisplay() and spit out that output. It would probably be able
> >>> to be used by virt-manager and vdsm as well.
> >>>
> >>> Just a thought. Let me know.
> >>>
> >>
> >> Yeah, that's what I was thinking of when reading your patch. I think we
> >> can add a new API that would return all info needed to connect to a
> >> display. I think it could look like this:
> >>
> >> char * virDomainGetGraphicsInfo(virDomainPtr dom,
> >>                                 unsigned int flags);
> >>
> >> It would return a XML doc containing all interesting knobs:
> >>
> >> <graphics type='spice' port='5904'>
> >>   <listen type='address' address='1.2.3.4'/>
> >>   <channel name='main' mode='secure'/>
> >>   <channel name='record' mode='insecure'/>
> >>   <image compression='auto_glz'/>
> >>   <streaming mode='filter'/>
> >>   <clipboard copypaste='no'/>
> >>   <mouse mode='client'/>
> >> </graphics>
> >>
> >> yeah it's basically copy&paste-d from domain's XML, but there is huge
> >> difference. Currently vncdisplay command (and from quick look at your
> > 
> > I don't think we should be inventing a new API that just replicates
> > the data from the main XML.
> > 
> >> patch domdisplay too) don't show correct listen address if none set in
> >> domain's XML. Because in that case we take the one from qemu.conf.
> >> However, this cannot be stored into domain's XML as it would discard all
> >> future changes to vnc_listen|spice_listen in qemu.conf. For instance,
> >> user doesn't set any listenAddress in domain's XML but set
> >> vnc_listen=1.2.3.4; Then we want vncdisplay (and subsequently
> >> domdisplay) to return 1.2.3.4:<port>
> > 
> > It is possible to distinguish an original 'listen' addr from
> > an auto-added already. The auto-added one will only appear
> > in the live XML, so apps can see the original value via the
> > inactive XML.
> > 
> 
> No it's not. When building qemu cmd line:
>     listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
> ...
>     if (!listenAddr)
>         listenAddr = driver->vncListen;
> 
>     escapeAddr = strchr(listenAddr, ':') != NULL;
>     if (escapeAddr)
>         virBufferAsprintf(&opt, "[%s]", listenAddr);
>     else
>         virBufferAdd(&opt, listenAddr, -1);
> 
> And when formatting (even live) domain XML:
>     listenAddr = virDomainGraphicsListenGetAddress(def, i);
> ...
>     if (listenAddr)
>         virBufferAsprintf(buf, " listen='%s'", listenAddr);
> 
> 
> What might cause confusion are migration cookies where the listen
> address is presented every time. That's because the same command
> sequence as for building cmd line is used.
> 
> But okay, if we don't want to introduce a new API we must find a way of
> inserting an attribute just into live XML not inactive (=make info
> temporary). I guess I believe how to achieve this.

We already do this for the port number, so I don't see why the
listen addr should be any harder


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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