Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: Refactoring of retrieving device info from xml
......................................................................
Patch Set 7: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/17694/7//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2013-08-05 10:36:28 +0200
Line 4: Commit: Gerrit Code Review <gerrit2(a)gerrit.ovirt.org>
Line 5: CommitDate: 2013-10-01 03:56:14 -0400
Line 6:
Line 7: vdsm: Refactoring of retrieving device info from xml
Could you explain the benefit of the new code? it is not self-evident (at least not to
me). I would not mind adding as self._lastXMLDescParsed for efficiency, and I would not
mind creating a class wrapping the two together, something like
class ParsedDomXML(object):
def __init__(self, text):
self.text = text
self.dom = _domParseStr(text)
self.devs = self.dom.childNodes[0].getElementsByTagName('devices')[0]
Line 8:
Line 9: Reworked a bit the retrieval of device info from the libvirt domain xml.
Line 10: Now VDSM won't parse the code in lastXmlDesc every time and the retrieval
Line 11: of elements from the domain xml has been a bit abstracted.
--
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: 7
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: oVirt Jenkins CI Server
Gerrit-HasComments: Yes