Antoni Segura Puimedon has posted comments on this change. Change subject: virt: migration: add monitor thread control loop ......................................................................
Patch Set 7: Code-Review-1 (2 comments) Minor comments http://gerrit.ovirt.org/#/c/25976/7/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 388: self.progress = 0 Line 389: Line 390: memSize = int(self._vm.conf['memSize']) Line 391: maxTimePerGiB = config.getint('vars', Line 392: 'migrationMaxTime_per_gib_mem') what about having maxTimePerGib be loaded after the imports as a constant MAX_TIME_PER_GIB = config.getint('vars', 'migrationprogressTimeour') Line 393: self._migrationMaxTime = (maxTimePerGiB * memSize + 1023) / 1024 Line 394: self._progressTimeout = config.getint('vars', Line 395: 'migrationprogressTimeout') Line 396: Line 391: maxTimePerGiB = config.getint('vars', Line 392: 'migrationMaxTime_per_gib_mem') Line 393: self._migrationMaxTime = (maxTimePerGiB * memSize + 1023) / 1024 Line 394: self._progressTimeout = config.getint('vars', Line 395: 'migrationprogressTimeout') Same thing here, I don't think it should be an instance variable. It is read-only and a configuration value. Line 396: Line 397: @property Line 398: def enabled(self): Line 399: return MigrationMonitorThread._MIGRATION_MONITOR_INTERVAL > 0 -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@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