Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 8: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6730/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7520/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7630/ : FAILURE -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 9: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6731/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7521/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7631/ : FAILURE -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Itamar Heim
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 10: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6732/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7522/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7632/ : FAILURE -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Martin Sivák has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 10:
(3 comments)
http://gerrit.ovirt.org/#/c/17694/10/vdsm/vm.py
File vdsm/vm.py:
Line 461: class ParsedDomXML(object):
Line 462: def __init__(self, xml):
Line 463: self.set(xml)
Line 464:
Line 465: def set(self, xml):
I would probably make the class "read-only" by moving this to the constructor
and requiring the user to create a new instance of the class every time a new
XML is needed.
Line 466: self._xml = xml
Line 467: self._dom = _domParseStr(xml)
Line 468: devices = self.firstElementByTagName('devices')
Line 469: self._devices = devices
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.
I agree. Properties would make it nicer.
Line 481: return self._devices
Line 482:
Line 483: def getDeviceElements(self, tagName):
Line 484: return self._devices.getElementsByTagName(tagName)
Line 4631: return pid
Line 4632:
Line 4633: def _getUnderlyingVmInfo(self):
Line 4634: self._lastXMLDesc.set(self._dom.XMLDesc(0))
Line 4635: self._devXmlHash =
str(hash(self._lastXMLDesc.devs().toxml()))
You can add the hash to the ParsedXMLDom class as a property and compute it
when it is accessed for the first time.
Line 4636:
Line 4637: def _ejectFloppy(self):
Line 4638: if 'volatileFloppy' in self.conf:
Line 4639: utils.rmFile(self.conf['floppy'])
--
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Itamar Heim
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 11: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6741/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7531/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7641/ : FAILURE -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 12: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6743/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7533/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7643/ : SUCCESS -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 13: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6748/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7538/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7648/ : SUCCESS -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 13: Code-Review+1 -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Martin Sivák has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 13: (2 comments) http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py File vdsm/vm.py: Line 485: return self._devices.getElementsByTagName(tagName) Line 486: Line 487: @property Line 488: def hash(self): Line 489: if self.devices and not self._hash: Why not "self._hash is not None"? Line 490: self._hash = str(hash(self._devices.toxml())) Line 491: return self._hash or '0' Line 492: Line 493: 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. Line 492: Line 493: Line 494: def eventToString(event): Line 495: return _EVENT_STRINGS[event] -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 13: Code-Review-1
(8 comments)
This is a very nice patch, but it needs some more work.
http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py
File vdsm/vm.py:
Line 457: "Shutdown",
Line 458: "PM-Suspended")
Line 459:
Line 460:
Line 461: class ParsedDomXML(object):
While this name is clear, a name like LibvirtDomain or DomainDescriptor may be
nicer.
Line 462: def __init__(self, xml):
Line 463: self._hash = None
Line 464: self._xml = xml
Line 465: self._dom = _domParseStr(xml)
Line 462: def __init__(self, xml):
Line 463: self._hash = None
Line 464: self._xml = xml
Line 465: self._dom = _domParseStr(xml)
Line 466: self._devices = self.firstElementByTagName('devices')
Since we assume that self._xml, self._dom, and self._devices never change, we
can simply create self._hash here and return it in hash(). Since hashing is
cheap, this should be good enough.
Line 467:
Line 468: @property
Line 469: def xml(self):
Line 470: return self._xml
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 use
the much cheaper hash(self._xml)
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 486:
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()))
For another patch: I'm not sure how we use this hash - but it is not good for
detecting changes in the xml. For this we should use md5 or sha1.
Line 491: return self._hash or '0'
Line 492:
Line 493:
Line 494: def eventToString(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?
Line 2038: '%s' % self.id)
Line 2039: self._released = False
Line 2040: self._releaseLock = threading.Lock()
Line 2041: self.saveState()
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 dom
behind it's back, and then accessing the dom to create a new xml.
I think we should leave the old code as is, and remove the dom property, used
only by this function.
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?!
If this should not have a return value, add another patch removing the return
value and renaming this to something like initUnderlyingVmInfo.
Minor thing: it is not clear what is self._dom.XMLDesc(0). An explaining
variable may help here:
explaining_name = self._dom.XMLDesc(0)
self._lastXMLDesc = ParsedDomXML(explaining_name)
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.
Line 5113: port = graphics.getAttribute('port')
Line 5114: if port:
Line 5115: self.conf['displayPort'] = port
Line 5116: port = graphics.getAttribute('tlsPort')
--
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 13: (1 comment) http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py File vdsm/vm.py: 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. Line 3110: Line 3111: def _updateAgentChannels(self): Line 3112: """ Line 3113: We moved the naming of guest agent channel sockets. To keep backwards -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Michal Skrivanek has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 13:
(2 comments)
http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py
File vdsm/vm.py:
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.
I tend to like the name. It gives you a clear distinction from vdsm's structures
Line 3110:
Line 3111: def _updateAgentChannels(self):
Line 3112: """
Line 3113: We moved the naming of guest agent channel sockets. To keep
backwards
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?
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Itamar Heim
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
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: '%s' % 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'
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 13:
(1 comment)
http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py
File vdsm/vm.py:
Line 5108: def _getUnderlyingDisplayPort(self):
Line 5109: """
Line 5110: Obtain display port info from libvirt.
Line 5111: """
Line 5112: graphics =
self._lastXMLDesc.firstElementByTagName('graphics')
> Well and before it was an out of index error in that case. Not sure how rel
Right, so this can wait for another patch.
Line 5113: port = graphics.getAttribute('port')
Line 5114: if port:
Line 5115: self.conf['displayPort'] = port
Line 5116: port = graphics.getAttribute('tlsPort')
--
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Itamar Heim
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 13: (1 comment) http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py File vdsm/vm.py: Line 4634: pass Line 4635: return pid Line 4636: Line 4637: def _getUnderlyingVmInfo(self): Line 4638: self._lastXMLDesc = ParsedDomXML(self._dom.XMLDesc(0)) > The return value was never used anywhere, that's why I dropped it. It's not Obviously libvirt API is not clear, and we would like to have more clear code, so it is easy to work with. Of course this should be handled in another patch. Line 4639: Line 4640: def _ejectFloppy(self): Line 4641: if 'volatileFloppy' in self.conf: Line 4642: utils.rmFile(self.conf['floppy']) -- 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 Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 14: Patch set 14: just published a patch of mine on top of this one :\ -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 14: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8108/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8221/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7318/ : SUCCESS -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 14: Code-Review-1 Please address my comments from patch set 13: http://gerrit.ovirt.org/#/c/17694/13/vdsm/vm.py -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Itamar Heim has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 14: ping -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Vinzenz Feenstra has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 13:
(3 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):
> correct, it has to be renamed
Done
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 3292:
Line 3293: for snappableDiskDeviceXmlElement in
snappableDiskDeviceXmlElements:
Line 3294: self._changeDisk(snappableDiskDeviceXmlElement)
Line 3295:
Line 3296: return parsedSrcDomXML.dom.toxml()
> True, I just tried to use the convenience functions of it. It's probably no
Done
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?!
Done
Line 4639:
Line 4640: def _ejectFloppy(self):
Line 4641: if 'volatileFloppy' in self.conf:
Line 4642: utils.rmFile(self.conf['floppy'])
--
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
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 15: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9666/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10451/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1018/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10608/ : FAILURE -- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 16: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9667/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10452/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1019/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10609/ : FAILURE -- 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: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 18:
(2 comments)
Nice, needs little cleanup.
http://gerrit.ovirt.org/#/c/17694/18/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1855: self.conf['vmName'] = 'n%s' % self.id
Line 1856: self._guestSocketFile =
self._makeChannelPath(_VMCHANNEL_DEVICE_NAME)
Line 1857: self._qemuguestSocketFile =
self._makeChannelPath(_QEMU_GA_DEVICE_NAME)
Line 1858: self._lastXMLDesc = DomainDescriptor(
Line 1859: '%s' % self.id)
Lets move this hack into domaindescrioptor module and call it something like
emptyDescriptor or something like that.
Will clean the code here:
self._lastXMLDesc = DomainDescriptor.fromId(self.id)
Line 1860: self._released = False
Line 1861: self._releaseLock = threading.Lock()
Line 1862: self.saveState()
Line 1863: self._watchdogEvent = {}
Line 4659: except Exception:
Line 4660: pass
Line 4661: return pid
Line 4662:
Line 4663: def _getUnderlyingVmInfo(self):
This does not get anything.
Lets rename to setupUnderlyingVmInfo, or maye more specific like parseDomainXML
Line 4664: domainXML = self._dom.XMLDesc(0)
Line 4665: self._lastXMLDesc = DomainDescriptor(domainXML)
Line 4666:
Line 4667: def _ejectFloppy(self):
--
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: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 18: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9670/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10455/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1021/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10612/ : SUCCESS -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 17: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9668/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10453/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1020/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10610/ : FAILURE -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 18:
(2 comments)
I'm fine with the domain_descriptor.py;
but just a wild idea, maybe merge into the new xmldom module?
http://gerrit.ovirt.org/#/c/26855/2
http://gerrit.ovirt.org/#/c/17694/18/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1855: self.conf['vmName'] = 'n%s' % self.id
Line 1856: self._guestSocketFile =
self._makeChannelPath(_VMCHANNEL_DEVICE_NAME)
Line 1857: self._qemuguestSocketFile =
self._makeChannelPath(_QEMU_GA_DEVICE_NAME)
Line 1858: self._lastXMLDesc = DomainDescriptor(
Line 1859: '%s' % self.id)
> Lets move this hack into domaindescrioptor module and call it something lik
+1
Line 1860: self._released = False
Line 1861: self._releaseLock = threading.Lock()
Line 1862: self.saveState()
Line 1863: self._watchdogEvent = {}
Line 4661: return pid
Line 4662:
Line 4663: def _getUnderlyingVmInfo(self):
Line 4664: domainXML = self._dom.XMLDesc(0)
Line 4665: self._lastXMLDesc = DomainDescriptor(domainXML)
Now we need the return value: see _lookupDeviceXMLByAlias
Line 4666:
Line 4667: def _ejectFloppy(self):
Line 4668: if 'volatileFloppy' in self.conf:
Line 4669: utils.rmFile(self.conf['floppy'])
--
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: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Vinzenz Feenstra has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 18: @return value No, not any more, I just updated it -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 19: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9678/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10463/ : UNSTABLE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1028/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10620/ : SUCCESS -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 20: Code-Review-1 Verified-1 Build Unstable http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9691/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10476/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1033/ : The patch does not pass the virt functional tests http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10633/ : SUCCESS -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 21: Code-Review-1 Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9692/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10477/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1034/ : The patch does not pass the virt functional tests http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/24/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/57/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/34/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10634/ : SUCCESS -- 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: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 22: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9693/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10478/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1035/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/25/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/58/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/35/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10635/ : SUCCESS -- 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: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 22:
(2 comments)
Nice but see comments in
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:
Line 19: #
Line 20: import xml.dom.minidom
Line 21:
Line 22:
Line 23: class DomainDescriptor(object):
Blank line bellow the class is common and make it even nicer.
Line 24: def __init__(self, xmlStr):
Line 25: self._xml = xmlStr
Line 26: self._dom = xml.dom.minidom.parseString(xmlStr)
Line 27: self._devices = self.firstElementByTagName('devices')
Line 29: self._devicesHash = str(hash(self._devices)) if self._devices
else '0'
Line 30:
Line 31: @staticmethod
Line 32: def fromId(uuid):
Line 33: return DomainDescriptor('%s' %
uuid)
Should be class method:
@classmethod
def fromId(cls, uuid):
retrn cls("...")
So if you subclass this you will create your subclass, not the superclass.
Line 34:
Line 35: @property
Line 36: def xml(self):
Line 37: return self._xml
--
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Vinzenz Feenstra has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 22:
(2 comments)
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:
Line 19: #
Line 20: import xml.dom.minidom
Line 21:
Line 22:
Line 23: class DomainDescriptor(object):
> Blank line bellow the class is common and make it even nicer.
Done
Line 24: def __init__(self, xmlStr):
Line 25: self._xml = xmlStr
Line 26: self._dom = xml.dom.minidom.parseString(xmlStr)
Line 27: self._devices = self.firstElementByTagName('devices')
Line 29: self._devicesHash = str(hash(self._devices)) if self._devices
else '0'
Line 30:
Line 31: @staticmethod
Line 32: def fromId(uuid):
Line 33: return DomainDescriptor('%s' %
uuid)
> Should be class method:
Done
Line 34:
Line 35: @property
Line 36: def xml(self):
Line 37: return self._xml
--
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Ewoud Kohl van Wijngaarden has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 22:
(1 comment)
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:
Line 29: self._devicesHash = str(hash(self._devices)) if self._devices
else '0'
Line 30:
Line 31: @staticmethod
Line 32: def fromId(uuid):
Line 33: return DomainDescriptor('%s' %
uuid)
> Should be class method:
@Nir are you sure you're not confusing this with @classmethod?
https://docs.python.org/2/library/functions.html#staticmethod vs
https://docs.python.org/2/library/functions.html#classmethod
Line 34:
Line 35: @property
Line 36: def xml(self):
Line 37: return self._xml
--
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 22:
(1 comment)
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:
Line 29: self._devicesHash = str(hash(self._devices)) if self._devices
else '0'
Line 30:
Line 31: @staticmethod
Line 32: def fromId(uuid):
Line 33: return DomainDescriptor('%s' %
uuid)
> @Nir are you sure you're not confusing this with @classmethod? https://docs
I do not understand your question - Vinzenz is using a @staticmethod and I
suggested a @classmethod - where is the confusion?
Line 34:
Line 35: @property
Line 36: def xml(self):
Line 37: return self._xml
--
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Ewoud Kohl van Wijngaarden has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 22:
(1 comment)
http://gerrit.ovirt.org/#/c/17694/22/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:
Line 29: self._devicesHash = str(hash(self._devices)) if self._devices
else '0'
Line 30:
Line 31: @staticmethod
Line 32: def fromId(uuid):
Line 33: return DomainDescriptor('%s' %
uuid)
> I do not understand your question - Vinzenz is using a @staticmethod and I
I now see I caused the confusion by misreading it. My apologies.
Line 34:
Line 35: @property
Line 36: def xml(self):
Line 37: return self._xml
--
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 23: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9745/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10530/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1068/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/32/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc19_created/65/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/44/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10687/ : SUCCESS -- 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: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 23: Code-Review+1 Nice -- 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: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 23: Code-Review+1 let's have this in! It owuld be awesome to have also some numbers about the performance gains of this patch -- 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: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 24: rebased, due to vmxml.py changes introduced since the revision before. -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 24: (2 comments) some possible improvements I'd like to implement, probably better in follow up patches once this is in. http://gerrit.ovirt.org/#/c/17694/24/vdsm/virt/domain_descriptor.py File vdsm/virt/domain_descriptor.py: Line 40: @property Line 41: def dom(self): Line 42: return self._dom Line 43: Line 44: def firstElementByTagName(self, tagName): according to git grep, this can be made private Line 45: elements = self._dom.childNodes[0].getElementsByTagName(tagName) Line 46: return elements[0] if elements else None Line 47: Line 48: @property Line 49: def devices(self): Line 50: return self._devices Line 51: Line 52: def getDeviceElements(self, tagName): Line 53: return self._devices.getElementsByTagName(tagName) Since we need another round of review due to the big rebase being made, I'd like to propose a new API here. My rationale is 1 since this is new module, have an API more_pep8_friendly and goAwayFromCamelCase 2 add more convenience methods, possibly merging helpers currently in vmxml.py 3 abstract from the minidom API, in not-so-distant future we want to move to cElementTree for performance and nicer API. What about replacing the two above with @property def all_devices(self): return self._devices def devices(self, type): # klass? dev_type? kind? return self._devices.getElementsByTagName(type) - of course this will not address #3 above Line 54: Line 55: @property Line 56: def devicesHash(self): -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: fixes tests broken by rebase. Will do any other change but rebase fixes/address reviewer's comments in followup patches. let's unfreeze this change and have it in! -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change.
Change subject: Introduction for caching the parsed domain XML
..
Patch Set 25:
(2 comments)
Partial review
http://gerrit.ovirt.org/#/c/17694/25/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4489: return pid
Line 4490:
Line 4491: def _updateDomainDescriptor(self):
Line 4492: domainXML = self._dom.XMLDesc(0)
Line 4493: self._lastXMLDesc = DomainDescriptor(domainXML)
Lets clean up the ugly _lastXMLDesc - how about self._domain?
For example:
for device in self._domain.devices:
...
Or:
self._domain.xml
Line 4494:
Line 4495: def _ejectFloppy(self):
Line 4496: if 'volatileFloppy' in self.conf:
Line 4497: utils.rmFile(self.conf['floppy'])
Line 5045: def _getUnderlyingNetworkInterfaceInfo(self):
Line 5046: """
Line 5047: Obtain network interface info from libvirt.
Line 5048: """
Line 5049: # TODO use xpath instead of parseString (here and elsewhere)
This comment can be remove now.
Line 5050: for x in self._lastXMLDesc.getDeviceElements('interface'):
Line 5051: devType = x.getAttribute('type')
Line 5052: mac =
x.getElementsByTagName('mac')[0].getAttribute('address')
Line 5053: 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: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: (1 comment) http://gerrit.ovirt.org/#/c/17694/25/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4489: return pid Line 4490: Line 4491: def _updateDomainDescriptor(self): Line 4492: domainXML = self._dom.XMLDesc(0) Line 4493: self._lastXMLDesc = DomainDescriptor(domainXML) > Lets clean up the ugly _lastXMLDesc - how about self._domain? We can also do it in another patch - but this patch changes every line that touch self._lastXMLDesc since we change the type. Changing the name now will save additional verification and will save reviewers time. Line 4494: Line 4495: def _ejectFloppy(self): Line 4496: if 'volatileFloppy' in self.conf: Line 4497: utils.rmFile(self.conf['floppy']) -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Francesco Romani has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: (1 comment) http://gerrit.ovirt.org/#/c/17694/25/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4489: return pid Line 4490: Line 4491: def _updateDomainDescriptor(self): Line 4492: domainXML = self._dom.XMLDesc(0) Line 4493: self._lastXMLDesc = DomainDescriptor(domainXML) > We can also do it in another patch - but this patch changes every line that 'domain' is nicer and shorter, and as you said saves developer time. Will change. Should I also squash http://gerrit.ovirt.org/#/c/33672/1 in this? Only name are changed, non changes in functioning. Line 4494: Line 4495: def _ejectFloppy(self): Line 4496: if 'volatileFloppy' in self.conf: Line 4497: utils.rmFile(self.conf['floppy']) -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Nir Soffer has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: (1 comment) http://gerrit.ovirt.org/#/c/17694/25/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4489: return pid Line 4490: Line 4491: def _updateDomainDescriptor(self): Line 4492: domainXML = self._dom.XMLDesc(0) Line 4493: self._lastXMLDesc = DomainDescriptor(domainXML) > 'domain' is nicer and shorter, and as you said saves developer time. Will c 336721 is not related to this change, please don't squash it. Line 4494: Line 4495: def _ejectFloppy(self): Line 4496: if 'volatileFloppy' in self.conf: Line 4497: utils.rmFile(self.conf['floppy']) -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 24: Build Failed http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/414/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/431/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11766/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12710/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1709/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12555/ : SUCCESS -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
oVirt Jenkins CI Server has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: Build Failed http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/416/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/433/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11769/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12713/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1710/ : There was an infra issue, please contact in...@ovirt.org http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12558/ : SUCCESS -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introduction for caching the parsed domain XML
Dan Kenigsberg has posted comments on this change. Change subject: Introduction for caching the parsed domain XML .. Patch Set 25: (1 comment) http://gerrit.ovirt.org/#/c/17694/25/vdsm/virt/domain_descriptor.py File vdsm/virt/domain_descriptor.py: Line 40: @property Line 41: def dom(self): Line 42: return self._dom Line 43: Line 44: def firstElementByTagName(self, tagName): this should be private (unless expected to be used out of __init__ Line 45: elements = self._dom.childNodes[0].getElementsByTagName(tagName) Line 46: return elements[0] if elements else None Line 47: Line 48: @property -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ewoud Kohl van Wijngaarden Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
