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 <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
