Martin Betak has posted comments on this change. Change subject: migration: Add incoming migration semaphore ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/45954/2/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 1271: else: Line 1272: yield Line 1273: Line 1274: Line 1275: @contextmanager > See comment in api.py Done Line 1276: def releaseOnError(resource): Line 1277: try: Line 1278: yield Line 1279: except Exception: https://gerrit.ovirt.org/#/c/45954/2/vdsm/API.py File vdsm/API.py: Line 574: Line 575: if not migration.incomingMigrations.acquire(False): Line 576: return response.error('migrationLimit') Line 577: Line 578: with utils.releaseOnError(migration.incomingMigrations): > This feels unpythonic. You should create a contextmanager function that is Done Line 579: params['vmId'] = self._UUID Line 580: result = self.create(params) Line 581: if result['status']['code']: Line 582: self.log.debug('Migration create - Failed') https://gerrit.ovirt.org/#/c/45954/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 54: VIR_MIGRATE_PARAM_GRAPHICS_URI = 'graphics_uri' Line 55: Line 56: Line 57: mig = min(config.getint('vars', 'max_incoming_migrations'), Line 58: caps.CpuTopology().cores()) > should be exposed as a configuration imho I wanted to remain consistent with the handling of outgoing migrations which use the same logic. Also added to config.py.in Line 59: Line 60: incomingMigrations = threading.BoundedSemaphore(mig) Line 61: Line 62: -- To view, visit https://gerrit.ovirt.org/45954 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8952f732033ed160292b11fbc0c4deac099b2b3e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
