Nir Soffer has posted comments on this change. Change subject: virt: migration: move threads in a separate module ......................................................................
Patch Set 7: (2 comments) Possible improvement. http://gerrit.ovirt.org/#/c/25970/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1575: elif 'restoreState' in self.conf: Line 1576: self._lastStatus = vmstatus.RESTORING_STATE Line 1577: else: Line 1578: self._lastStatus = vmstatus.WAIT_FOR_LAUNCH Line 1579: self._migrationSourceThread = migration.MigrationSourceThread(self) This would be nicer if we remove the "Migration" prefix. This is common issue in Python, and many times can be fixed easily. self._migrationSourceThread = migration.SourceThread(self) This change also make it much harder to write bad code like: from migration import * Or: from migration import SourceThread Because the names may conflict with other modules using this naming convention. This conflict is good and enforce importing only modules, which should have been our coding style. Line 1580: self._kvmEnable = self.conf.get('kvmEnable', 'true') Line 1581: self._guestSocketFile = constants.P_VDSM_RUN + self.conf['vmId'] + \ Line 1582: '.guest.socket' Line 1583: self._incomingMigrationFinished = threading.Event() Line 2512: # taken self Down Line 2513: if self._lastStatus == vmstatus.DOWN: Line 2514: return errCode['noVM'] Line 2515: self._migrationSourceThread = migration.MigrationSourceThread( Line 2516: self, **params) Here is may also improve indentation: self._migrationSourceThread = migration.SourceThread(self, **params) Line 2517: self._migrationSourceThread.start() Line 2518: self._migrationSourceThread.getStat() Line 2519: return self._migrationSourceThread.status Line 2520: finally: -- To view, visit http://gerrit.ovirt.org/25970 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia69c7448b66417c7bba8ae3d301e7d777ca88067 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@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: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@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
