Francesco Romani has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
......................................................................
Patch Set 10:
(2 comments)
just a couple of nits. I'm really looking forward to have this merged!
http://gerrit.ovirt.org/#/c/17694/10/vdsm/vm.py
File vdsm/vm.py:
Line 464:
Line 465: def set(self, xml):
Line 466: self._xml = xml
Line 467: self._dom = _domParseStr(xml)
Line 468: devices = self.firstElementByTagName('devices')
Unneeded temporary?
Line 469: self._devices = devices
Line 470:
Line 471: def xml(self):
Line 472: return self._xml
Line 476:
Line 477: def firstElementByTagName(self, tagName):
Line 478: return self._dom.childNodes[0].getElementsByTagName(tagName)[0]
Line 479:
Line 480: def devs(self):
nit: 'devices' is better IMO.
Also wondering if we make a property of this one, 'dom' and 'xml'. This
will lead to clearer code and save us a temporary (line 3281) and a few parens here and
there.
Line 481: return self._devices
Line 482:
Line 483: def getDeviceElements(self, tagName):
Line 484: return self._devices.getElementsByTagName(tagName)
--
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: 10
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: 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