Tomas Jelinek has posted comments on this change.

Change subject: migration: added support for convergance schedule
......................................................................


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/46940/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 399:             self._vm.log.info('starting migration to %s '
Line 400:                               'with miguri %s', duri, muri)
Line 401: 
Line 402:             self._monitorThread = MonitorThread(self._vm, startTime, 
self._convergenceSchedule)
Line 403:             self._do_perform_migration(self, duri, muri)
> Disclosure: I actually missed the _do in the first review :)
hmm, not sure how though. I see this options:

1: in ctor replace the _perform_migration with the decorated version of it (can 
not really use decorators because I need access to self). so here it would be 
just self._perform_migration(duri, muri) - but it will not be too explicit that 
a different method is actually called

2: inline it (as it was before)

3: use OO way (e.g. have a base class doing the base _perform_migration() and 
two children which will extend it differently) (I guess it is not pythonic ;) )

4: use some magic I don't know :)

So I guess the 4 is the way to go. What you think?
Line 404: 
Line 405:             self.log.info("migration took %d seconds to complete",
Line 406:                           (time.time() - startTime) + 
destCreationTime)
Line 407: 


-- 
To view, visit https://gerrit.ovirt.org/46940
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I989cff12d08ef1cab36bd10df7daaa999a8dac14
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to