Francesco Romani has posted comments on this change. Change subject: migrations: change convergence schedule from time to iterations ......................................................................
Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/59219/4/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 565: iterationCount) Line 566: Line 567: lastDataRemaining = dataRemaining Line 568: Line 569: if (now - lastProgressTime) > progress_timeout: > I'm worried that we change `elif' to `if' here. It's not easy to tell wheth I agree this is worth extra care. There is an important point to consider: the old code used the 'abort' boolean to call abortJob() + stop() in a single place.here after this patch we inlined this, so we abort early. So it should be ok from this perspective. OTOH, we "just" need to do the accounting, so I can perhaps move the lines 561-567 up before the if/elif/elif chain and make the change easier to review. Line 570: # Migration is stuck, abort Line 571: self._vm.log.warn( Line 572: 'Migration is stuck: Hasn\'t progressed in %s seconds. ' Line 573: 'Aborting.' % (now - lastProgressTime)) -- To view, visit https://gerrit.ovirt.org/59219 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f87c954031842c35c99888c228a34ec7f19d800 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
