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

Hi, ----- Original Message ----- > From: "Laine Stump" <laine(a)laine.org&gt; > To: libvir-list(a)redhat.com > Cc: "Ján Tomko" <jtomko(a)redhat.com&gt;, fromani(a)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#virDomainUpdateDevice... doesn't seem to match with I just learned from your email: http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDeviceModify... 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,