Change in vdsm[master]: virt: migration: always run migration monitor

danken at redhat.com danken at redhat.com
Thu Apr 3 10:13:38 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: virt: migration: always run migration monitor
......................................................................


Patch Set 12: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/25975/12//COMMIT_MSG
Commit Message:

Line 9: Before this patch the migration monitor thread is always
Line 10: run, unless the migration monitor interval configuration option
Line 11: is explicitely set to zero.
Line 12: 
Line 13: As far as we know, this is rarely done, and there are hidden
could you un-hide such an assumption?
Line 14: assumptions scattered in the code about the monitor thread
Line 15: always be present.
Line 16: 
Line 17: This patch enforce those assumptions, and let this thread always


http://gerrit.ovirt.org/#/c/25975/12/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 394:     def run(self):
Line 395:         if self.enabled:
Line 396:             self.monitor_migration()
Line 397:         else:
Line 398:             self.idle()
Could you suggest at least a single case where it is anyhow helpful to have this not-so-idle loop running?
Line 399: 
Line 400:     def monitor_migration(self):
Line 401:         def calculateProgress(remaining, total):
Line 402:             if remaining == 0:


Line 464:                               (timeElapsed / 1000, self.progress))
Line 465: 
Line 466:     def idle(self):
Line 467:         while not self._stop.isSet():
Line 468:             time.sleep(1.0)
how about self._stop.wait(forever)  ?
(if we really need idle() at all)
Line 469: 
Line 470:     def stop(self):
Line 471:         self._vm.log.debug('stopping migration monitor thread')


-- 
To view, visit http://gerrit.ovirt.org/25975
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4953ddda5a5c6c0ecd7ea0f95377309e18f771a
Gerrit-PatchSet: 12
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: Francesco Romani <fromani 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