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

[Francesco Romani]

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:

    <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?

> 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,
 
-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani