Adam Litke has posted comments on this change. Change subject: Live Merge: Ignore libvirt block job events ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/30046/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5627: liveInfo = None Line 5628: try: Line 5629: liveInfo = self._dom.blockJobInfo(drive.name, 0) Line 5630: except libvirt.libvirtError: Line 5631: self.log.exception("Error getting block job info") > If we get an expected exception (job ended) we may not want to log a traceb Correct, but a missing blockJob results in a None return value not an exception. Therefore, I think the code here is correct. Line 5632: Line 5633: if liveInfo: Line 5634: entry['bandwidth'] = liveInfo['bandwidth'] Line 5635: entry['cur'] = str(liveInfo['cur']) Line 5635: entry['cur'] = str(liveInfo['cur']) Line 5636: entry['end'] = str(liveInfo['end']) Line 5637: else: Line 5638: self.log.info("Block Job %s has ended", jobID) Line 5639: storedJob['done'] = True > NOTE: This must not be persisted in the recovery because on vdsm restart we Yes. I think it's worth adding a comment to this effect here. Line 5640: LiveMergeCleanupThread(self, jobID, drive, Line 5641: mode='finished').start() Line 5642: jobsRet[jobID] = entry Line 5643: return jobsRet Line 5636: entry['end'] = str(liveInfo['end']) Line 5637: else: Line 5638: self.log.info("Block Job %s has ended", jobID) Line 5639: storedJob['done'] = True Line 5640: LiveMergeCleanupThread(self, jobID, drive, > It seems that in general we prefer: I can definitely change the class to a global method or vm class method if everyone prefers it. I was modeling this after the Migration threads and allowing for expansion of capabilities in the future if needed (see next paragraph). Regarding lack of references to these threads... We could easily add a dict in the VM that tracks the threads by jobID. Right now we are not doing any monitoring of these threads but if we want to in the future it would be easy to add this tracking later. Line 5641: mode='finished').start() Line 5642: jobsRet[jobID] = entry Line 5643: return jobsRet Line 5644: -- To view, visit http://gerrit.ovirt.org/30046 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibbcdca4c0c0e45e9323ecfef9ce2fce10d8451a5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[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
