Nir Soffer has posted comments on this change.
Change subject: Live Merge: Get volume chain for multiple drives
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.ovirt.org/#/c/31366/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 5424:
Line 5425: # Check that libvirt exposes full volume chain information
Line 5426: try:
Line 5427: self._driveGetActualVolumeChain([drive])[drive['alias']]
Line 5428: except KeyError:
This is not clear, and you may be mishandling a KeyError raised in
_driveGetActualVolumeChain()
Please separate the call and the check:
chains = self._driveGetActualVolumeChain([drive])
if drive['alias'] not in chains:
handle error...
Line 5429: self.log.error("merge: libvirt does not support volume chain
"
Line 5430: "monitoring. Unable to perform live
merge.")
Line 5431: return errCode['mergeErr']
Line 5432:
Line 5500: self._vmStats.sampleVmJobs()
Line 5501:
Line 5502: return {'status': doneCode}
Line 5503:
Line 5504: def _driveGetActualVolumeChain(self, drives):
Why not use *drives? Can cleanup the caller code.
Line 5505: def lookupDeviceXMLByAlias(domXML, targetAlias):
Line 5506: for deviceXML, alias in _devicesWithAlias(domXML):
Line 5507: if alias == targetAlias:
Line 5508: return deviceXML
Line 5519: if child.nodeName == name:
Line 5520: return child
Line 5521: return None
Line 5522:
Line 5523: ret = {}
Rename to driveInfo?
Line 5524: domXML = self._getUnderlyingVmInfo()
Line 5525: for drive in drives:
Line 5526: alias = drive['alias']
Line 5527: diskXML = lookupDeviceXMLByAlias(domXML, alias)
Line 5559:
Line 5560: curVols = [x['volumeID'] for x in drive.volumeChain]
Line 5561: try:
Line 5562: chain =
self._driveGetActualVolumeChain([drive])[drive['alias']]
Line 5563: except KeyError:
Same issue as the previous call, same solution?
Line 5564: self.log.debug("Unable to determine volume chain. Skipping
volume "
Line 5565: "chain synchronization for drive %s",
drive.name)
Line 5566: return
Line 5567:
--
To view, visit
http://gerrit.ovirt.org/31366
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93d5a641b0814e3764c70bea8a6c1910a821adcc
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(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: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes