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

[Michal Privoznik]

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 at 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.

Michal