Dan Kenigsberg has posted comments on this change. Change subject: virt: migration: always run migration monitor ......................................................................
Patch Set 12: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/25975/12//COMMIT_MSG Commit Message: Line 9: Before this patch the migration monitor thread is always Line 10: run, unless the migration monitor interval configuration option Line 11: is explicitely set to zero. Line 12: Line 13: As far as we know, this is rarely done, and there are hidden could you un-hide such an assumption? Line 14: assumptions scattered in the code about the monitor thread Line 15: always be present. Line 16: Line 17: This patch enforce those assumptions, and let this thread always http://gerrit.ovirt.org/#/c/25975/12/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 394: def run(self): Line 395: if self.enabled: Line 396: self.monitor_migration() Line 397: else: Line 398: self.idle() Could you suggest at least a single case where it is anyhow helpful to have this not-so-idle loop running? Line 399: Line 400: def monitor_migration(self): Line 401: def calculateProgress(remaining, total): Line 402: if remaining == 0: Line 464: (timeElapsed / 1000, self.progress)) Line 465: Line 466: def idle(self): Line 467: while not self._stop.isSet(): Line 468: time.sleep(1.0) how about self._stop.wait(forever) ? (if we really need idle() at all) Line 469: Line 470: def stop(self): Line 471: self._vm.log.debug('stopping migration monitor thread') -- To view, visit http://gerrit.ovirt.org/25975 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4953ddda5a5c6c0ecd7ea0f95377309e18f771a Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[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
