Dan Kenigsberg has posted comments on this change. Change subject: Live Merge: Ignore libvirt block job events ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/30046/8/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 5285: 'bandwidth': 0, 'cur': '0', 'end': '0', Line 5286: 'imgUUID': storedJob['disk']['imageID']} Line 5287: Line 5288: liveInfo = None Line 5289: if 'done' not in storedJob: > It means that the job is done according to libvirt and that libvirt will no 'done'/'gone seem the same to me... I somehow missed the "continue" few lines below, and thought that we can set 'gone' even when we're not sure that the job's status in libvirt. Line 5290: try: Line 5291: liveInfo = self._dom.blockJobInfo(drive.name, 0) Line 5292: except libvirt.libvirtError: Line 5293: self.log.exception("Error getting block job info") Line 5293: self.log.exception("Error getting block job info") Line 5294: jobsRet[jobID] = entry Line 5295: continue Line 5296: Line 5297: if liveInfo: > Hmm, that's not even my code. From my understanding of the call, dom.block I'd love to get an explicit ack on this from Federico of 1ce73032d923, and a simplification of the predicate there. Line 5298: entry['bandwidth'] = liveInfo['bandwidth'] Line 5299: entry['cur'] = str(liveInfo['cur']) Line 5300: entry['end'] = str(liveInfo['end']) Line 5301: else: Line 5307: # ended. Spawn an async cleanup. Line 5308: startCleanup(storedJob, drive) Line 5309: elif cleanThread.isAlive(): Line 5310: # Let previously started cleanup thread continue Line 5311: self.log.debug("Still waiting for block job %s to be " > Once per stats collection interval (default 15s). Seems reasonable to me c I hope to silence those; if you feel that this periodical spam would be helpful to you, then ok for now. Line 5312: "cleaned up", jobID) Line 5313: elif not cleanThread.isSuccessful(): Line 5314: # At this point we know the thread is not alive and the Line 5315: # cleanup failed. Retry it with a new thread. Line 5536: self.vm = vm Line 5537: self.jobId = jobId Line 5538: self.drive = drive Line 5539: self.success = False Line 5540: > better to add it to handleBlockJobEvent as you suggest in the other patch. I did not mean to suggest that - it's better to put @utils.traceback() here since it would catch SyntaxError or whatever nonsense that run() may acquire in the future. Line 5541: def run(self): Line 5542: self.vm.log.info("Starting live merge cleanup for job %s", Line 5543: self.jobId) Line 5544: ret = self.vm.handleBlockJobEvent(self.jobId, self.drive, 'finished') -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> 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
