Francesco Romani has posted comments on this change.

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


Patch Set 4:

(3 comments)

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)
> actually... :) the _do_perform_migration is assigned to self in the constru
Disclosure: I actually missed the _do in the first review :)
But still, let's try to make this more readable
Line 404: 
Line 405:             self.log.info("migration took %d seconds to complete",
Line 406:                           (time.time() - startTime) + 
destCreationTime)
Line 407: 


Line 594:         self._vm.log.debug('stopping migration monitor thread')
Line 595:         self._stop.set()
Line 596: 
Line 597:     def _next_action(self, stalling):
Line 598:         head, tail = self._convSchedule[0], self._convSchedule[1:]
> I need to assign the tail to the conv_schedule only if head.stallingLimit <
Right, I overlooked the code. Then maybe something like

  head = self._convSchedule[0]  # peek the head
  # log
  if head.stallingLimit < stalling:
    self._execute_next_step(stalling, head.actionWithParams)
    # log
    self._convSchedule.pop(0)  # consume the head
Line 599:         self._vm.log.debug('Stalling for %d seconds, '
Line 600:                            'trying to make next action: '
Line 601:                            '%s, tail: %s', stalling, head, tail)
Line 602:         if head.stallingLimit < stalling:


Line 605:             self._convSchedule = tail
Line 606: 
Line 607:     def _execute_next_step(self, stalling, actionWithParams):
Line 608:         action = str(actionWithParams.action)
Line 609:         if action == CONVERGENCE_SCHEDULE_SET_DOWNTIME:
> hm, do you mean to provide some map like convergance_actions which would ma
Yes, this is what I meant.
I don't have strong feelings here, we can do this in a later patch if we find a 
good way. I just wanted to make sure you considered this option, it is fine if 
you did and choose something else.
Line 610:             downtime = actionWithParams.params[0]
Line 611:             self.log.warn('Stalling for %d, setting downtime to %d',
Line 612:                            stalling, downtime)
Line 613:             self._vm._dom.migrateSetMaxDowntime(int(downtime), 0)


-- 
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