Dan Kenigsberg has posted comments on this change.

Change subject: virt: migration: merge monitor and downtime thread
......................................................................


Patch Set 24: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/25977/24/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 377: 
Line 378:         self._downtime = downTime
Line 379:         self._wait = (
Line 380:             self._DELAY_PER_GIB * max(memSize, 2048) + 1023) / 1024
Line 381:         self._downtimeInterval = self._wait / self._DOWNTIME_STEPS
the down side of merging the two threads into one, is that now it is harder to 
understand that these data members are specific to  the setting of down time. 
You may want to have a _downtime qualifier shared between them all.

But I would find it nicer if we can cosolidate this logic with 
AdvancedStatsFunc, where each Func has its own fancy object with its own data 
members.
Line 382:         self._downtimeStep = 0
Line 383: 
Line 384:     @property
Line 385:     def enabled(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac66331b44435a9cffeb9de1454db6843245979
Gerrit-PatchSet: 24
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

Reply via email to