Vinzenz Feenstra has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
......................................................................
Patch Set 13:
(8 comments)
http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py
File vdsm/vm.py:
Line 484: def getDeviceElements(self, tagName):
Line 485: return self._devices.getElementsByTagName(tagName)
Line 486:
Line 487: @property
Line 488: def hash(self):
This should be named devicesHash - this is not a hash of the xml.
Can we u
correct, it has to be renamed
Line 489: if self.devices and not self._hash:
Line 490: self._hash = str(hash(self._devices.toxml()))
Line 491: return self._hash or '0'
Line 492:
Line 487: @property
Line 488: def hash(self):
Line 489: if self.devices and not self._hash:
Line 490: self._hash = str(hash(self._devices.toxml()))
Line 491: return self._hash or '0'
Why the or in return? Just compute the hash and save it.
You
might not have devices, and this keeps the original behaviour that it is initially
returning '0'
Line 492:
Line 493:
Line 494: def eventToString(event):
Line 495: return _EVENT_STRINGS[event]
Line 2033: if 'vmName' not in self.conf:
Line 2034: self.conf['vmName'] = 'n%s' % self.id
Line 2035: self._guestSocketFile = self._makeChannelPath(_VMCHANNEL_DEVICE_NAME)
Line 2036: self._qemuguestSocketFile =
self._makeChannelPath(_QEMU_GA_DEVICE_NAME)
Line 2037: self._lastXMLDesc = ParsedDomXML(
For next patch - rename lastXMLDesc to something nicer?
Yeah, I
hate it as well ;-)
Line 2038: '<domain><uuid>%s</uuid></domain>'
% self.id)
Line 2039: self._released = False
Line 2040: self._releaseLock = threading.Lock()
Line 2041: self.saveState()
Line 3105: self._getUnderlyingWatchdogDeviceInfo()
Line 3106: self._getUnderlyingSmartcardDeviceInfo()
Line 3107: self._getUnderlyingConsoleDeviceInfo()
Line 3108: # Obtain info of all unknown devices. Must be last!
Line 3109: self._getUnderlyingUnknownDeviceInfo()
For another patch: get rid of the "Underlying" in these
names.
In the long run we want to get rid of them inside of the VM class and
refactor them out of there, then we'd get rid of this name. For now I tend to agree
with Michal
Line 3110:
Line 3111: def _updateAgentChannels(self):
Line 3112: """
Line 3113: We moved the naming of guest agent channel sockets. To keep backwards
Line 3292:
Line 3293: for snappableDiskDeviceXmlElement in snappableDiskDeviceXmlElements:
Line 3294: self._changeDisk(snappableDiskDeviceXmlElement)
Line 3295:
Line 3296: return parsedSrcDomXML.dom.toxml()
This usage is bad - we take a read-only object (ParsedDomXML), modify
it's
True, I just tried to use the convenience functions of it. It's
probably not really worth it
Line 3297:
Line 3298: def _changeDisk(self, diskDeviceXmlElement):
Line 3299: diskType = diskDeviceXmlElement.getAttribute('type')
Line 3300:
Line 4634: pass
Line 4635: return pid
Line 4636:
Line 4637: def _getUnderlyingVmInfo(self):
Line 4638: self._lastXMLDesc = ParsedDomXML(self._dom.XMLDesc(0))
This function used to return self._lastXMLDesc?!
The return
value was never used anywhere, that's why I dropped it. It's not really obvious
though.
I am not sure why I would add an explaining name for this and we use this everywhere else
without it.
XMLDesc is an API of libvirt. We don't really document that usage anywhere else
either, the only part which is not quite clear to someone might be the 0 parameter.
Line 4639:
Line 4640: def _ejectFloppy(self):
Line 4641: if 'volatileFloppy' in self.conf:
Line 4642: utils.rmFile(self.conf['floppy'])
Line 5108: def _getUnderlyingDisplayPort(self):
Line 5109: """
Line 5110: Obtain display port info from libvirt.
Line 5111: """
Line 5112: graphics = self._lastXMLDesc.firstElementByTagName('graphics')
This can return None - will raise AttributeError on next line.
Well and before it was an out of index error in that case. Not sure how relevant
that really is
Line 5113: port = graphics.getAttribute('port')
Line 5114: if port:
Line 5115: self.conf['displayPort'] = port
Line 5116: port = graphics.getAttribute('tlsPort')
Line 5120: def _getUnderlyingNetworkInterfaceInfo(self):
Line 5121: """
Line 5122: Obtain network interface info from libvirt.
Line 5123: """
Line 5124: # TODO use xpath instead of parseString (here and elsewhere)
This is not valid here anymore, is it?
Probably not
Line 5125: for x in self._lastXMLDesc.getDeviceElements('interface'):
Line 5126: devType = x.getAttribute('type')
Line 5127: mac =
x.getElementsByTagName('mac')[0].getAttribute('address')
Line 5128: alias =
x.getElementsByTagName('alias')[0].getAttribute('name')
--
To view, visit
http://gerrit.ovirt.org/17694
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e106b2f2d3f4160d4e882f1a2880cb1b52fbb22
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes