Francesco Romani has posted comments on this change. Change subject: migration: Add retry logic for incoming limit ......................................................................
Patch Set 15: (2 comments) looks like my comments are still relevant after all https://gerrit.ovirt.org/#/c/46971/15/lib/vdsm/response.py File lib/vdsm/response.py: Line 84: if err: : return code == errCode[err] : else: still relevant: I don't like this. Can't you just do expected_error = response.error('migrationLimit') res = some_call() if res == expected_error: # handle it ? https://gerrit.ovirt.org/#/c/46971/15/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 351: # so don't try again Line 352: if self._migrationCanceledEvt: Line 353: break Line 354: Line 355: SourceThread._ongoingMigrations.release() still relevant too: I understand why you do this but I really don't like it this way. Looks fragile and makes the code harder to follow. Let's think about a cleaner way, possibly involving (the least amount of) refactoring. Line 356: # the destination is busy with incoming migrations Line 357: # release semaphore and give other outgoing migrations Line 358: # a chance Line 359: time.sleep(5) -- To view, visit https://gerrit.ovirt.org/46971 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icabe79dfccb61ce43489a9a242a5390e73979649 Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[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/mailman/listinfo/vdsm-patches
