Change in vdsm[master]: virt: migration: move threads in a separate module

nsoffer at redhat.com nsoffer at redhat.com
Sat Mar 29 17:41:38 UTC 2014


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 <fromani at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list