Dan Kenigsberg has posted comments on this change. Change subject: virt: migration: add monitor thread control loop ......................................................................
Patch Set 23: Code-Review-1 (2 comments) http://gerrit.ovirt.org/#/c/25976/23/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 387: self._stop.set() Line 388: Line 389: Line 390: class MonitorThread(threading.Thread): Line 391: _MONITOR_TICK = 1 # unit: seconds. must be integer > 0. no one should ever change _MONITOR_TICK... If set to 2, (step % self._MONITOR_INTERVAL) would never get to zero. I believe that "step" should increase by exactly one step per iteration. _MONITOR_TICK should be eliminated. Line 392: _MONITOR_INTERVAL = config.getint( Line 393: 'vars', 'migration_monitor_interval') # unit: seconds Line 394: _MAX_TIME_PER_GIB = config.getint( Line 395: 'vars', 'migration_max_time_per_gib_mem') Line 426: while not self._stop.isSet(): Line 427: self._stop.wait(self._MONITOR_TICK) Line 428: for action in actions: Line 429: action(step) Line 430: step += self._MONITOR_TICK I have an issue with limitless integers, despite the fact of a year-long migration is not a very frequent sight, and is going to get only to step=31 million. Line 431: Line 432: self._vm.log.debug('migration monitor thread exiting') Line 433: Line 434: def monitor_migration(self, step): -- To view, visit http://gerrit.ovirt.org/25976 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie422bead060c8ba2bfd4bfada522b91d56697841 Gerrit-PatchSet: 23 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: Francesco Romani <from...@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
