[libvirt] [PATCH] Ignore listen attribute of <graphics> for type network listens

[Laine Stump]

On 02/26/2015 10:11 AM, Francesco Romani wrote:
> Hi,
>
> ----- Original Message -----
>> From: "Laine Stump" <laine at laine.org>
>> To: libvir-list at redhat.com
>> Cc: "Ján Tomko" <jtomko at redhat.com>, fromani at redhat.com
>> Sent: Thursday, February 26, 2015 3:57:22 PM
>> Subject: Re: [PATCH] Ignore listen attribute of <graphics> for type network listens
>>
>> On 02/26/2015 08:53 AM, Ján Tomko wrote:
>>> Commit 6992994 started filling the listen attribute
>>> of the parent <graphics> elements from type='network' listens.
>>>
>>> When this XML is passed to UpdateDevice, parsing fails:
>>> XML error: graphics listen attribute 10.20.30.40 must match
>>> address attribute of first listen element (found none)
>>
>> Note that the listen attribute of <graphics> won't be filled in if you
>> request the *inactive* xml, and so there will be no error when it is fed
>> back to updatedevice. I can't think of any examples right now, but have
>> a very definite memory that there are several items in the config like
>> this - if you feed the status xml output back to update/define "you're
>> gonna have a bad time". That's why virsh edit asks for the INACTIVE xml.
>>
>> Did you see this when trying to do an update-device manually, or as the
>> result of some management application that has forgotten to add the
>> INACTIVE flag to its request for XML?
> I don't know about Jan, but I experienced this failure myself with a quite
> recent libvirt snapshot (around Feb 20, so with the aforementioned commit)
> using oVirt/VDSM.
>
> When we connect to display through oVirt, before to fire up we set up the
> password through virDomainUpdateDeviceFlags. This fails for me on libvirt 1.2.13
> from master with Commit 6992994
>
> Let me summarize the flow:
>
> we create and feed createXML with:
>
>                 <graphics autoport="yes" keymap="en-us" passwd="*****" passwdValidTo="1970-01-01T00:00:01" port="-1" tlsPort="-1" type="spice">
>                         <channel mode="secure" name="main"/>
>                         <channel mode="secure" name="inputs"/>
>                         <channel mode="secure" name="cursor"/>
>                         <channel mode="secure" name="playback"/>
>                         <channel mode="secure" name="record"/>
>                         <channel mode="secure" name="display"/>
>                         <channel mode="secure" name="usbredir"/>
>                         <channel mode="secure" name="smartcard"/>
>                         <listen network="vdsm-ovirtmgmt" type="network"/>
>                 </graphics>
>
> we get back using domainGetXmlDesc:

This is where the error is caused - you need to add the inactive flag to
this call
>
>     <graphics type='spice' tlsPort='5900' autoport='yes' listen='192.168.1.51' keymap='en-us' passwd='*****' passwdValidTo='1970-01-01T00:00:01'>
>       <listen type='network' address='192.168.1.51' network='vdsm-ovirtmgmt'/>
>       <channel name='main' mode='secure'/>
>       <channel name='display' mode='secure'/>
>       <channel name='inputs' mode='secure'/>
>       <channel name='cursor' mode='secure'/>
>       <channel name='playback' mode='secure'/>
>       <channel name='record' mode='secure'/>
>       <channel name='smartcard' mode='secure'/>
>       <channel name='usbredir' mode='secure'/>
>     </graphics>
>
> we try to feed using virDomainUpdateDeviceFlags:
>
> <graphics autoport="yes" connected="disconnect" keymap="en-us" listen="192.168.1.51" passwd="SOMETHING" passwdValidTo="2015-02-26T09:56:40" tlsPort="5900" type="spice">
>       <listen address="192.168.1.51" network="vdsm-ovirtmgmt" type="network"/>
>       <channel mode="secure" name="main"/>
>       <channel mode="secure" name="display"/>
>       <channel mode="secure" name="inputs"/>
>       <channel mode="secure" name="cursor"/>
>       <channel mode="secure" name="playback"/>
>       <channel mode="secure" name="record"/>
>       <channel mode="secure" name="smartcard"/>
>       <channel mode="secure" name="usbredir"/>
>     </graphics>
>
> it chokes out with
>
>   File "/usr/lib64/python2.7/site-packages/libvirt.py", line 2647, in updateDeviceFlags
>     if ret == -1: raise libvirtError ('virDomainUpdateDeviceFlags() failed', dom=self)
> libvirtError: XML error: graphics listen attribute 192.168.1.51 must match address attribute of first listen element (found none)
>
> We use flags=0 here:
>
>             self._dom.updateDeviceFlags(graphics.toxml(), 0)
>
> (_dom is a virDomain, graphics.toxml() produces the above)
>
> Now, I understand that here we should use a proper flag, but I'm
> confused on which should we use. The docs
>
> http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainUpdateDeviceFlags
>
> doesn't seem to match with I just learned from your email:
>
> http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDeviceModifyFlags
>
> Which one do we need here? VIR_DOMAIN_DEVICE_MODIFY_CONFIG?

It's not the updateDevice flags that need to be changed. You need to add
VIR_DOMAIN_XML_INACTIVE to the flags for domainXmlGetDesc.

>
>> If the latter, then I guess I can live with ignoring the error in order
>> to preserve backward compatibility with the broken application.
>> Reluctant ACK.
> Well, I appreciate the backward compatibility effort, but I also
> definitely want to fix the oVirt code! :)
>
>
> Thanks,
>