Change in vdsm[master]: migration: Add retry logic for incoming limit

fromani at redhat.com fromani at redhat.com
Mon Oct 5 14:20:11 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: migration: Add retry logic for incoming limit
......................................................................


Patch Set 1:

(2 comments)

initial review

https://gerrit.ovirt.org/#/c/46971/1/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 328:                     dev._deviceXML, self._vm.conf, dev.custom)
Line 329:             hooks.before_vm_migrate_source(self._vm._dom.XMLDesc(0),
Line 330:                                            self._vm.conf)
Line 331: 
Line 332:             while not self._migrationCanceledEvt:
I'd reverse the loop logic.

Let's assume we want this retry logic (I'm still not sold, but for the sake of this patch let's assume we want it).

So we need to retry N times, being N="forever" by default (we may want to allow Engine to limit the retries in a future patch, that could be a nice compromise).

Do, I think it should go like this:

  while True:
    result = migrationCreate()  # always at least one try, like before

    if response.success(result):
      break
    elif result == response.error('migrationLimit'):
      if not self.migrationCanceledEvt:
         # we will retry. A log would be nice
         continue
      else:
         # log this
         break
    elif response.is_error(result):
      # generic error: log this and
      break

You can rearrange the checks as you find nice, but my point here is that we should somehow pack all the cycle breaker conditions together.
Line 333:                 # Do not measure the time spent for creating the VM on the
Line 334:                 # destination. In some cases some expensive operations can
Line 335:                 # cause the migration to get cancelled right after the
Line 336:                 # transfer started.


Line 346:                     # so don't try again
Line 347:                     if self._migrationCanceledEvt:
Line 348:                         break
Line 349: 
Line 350:                     SourceThread._ongoingMigrations.release()
I'm not sure we need this management here. Will get back on this later.
Line 351:                     # the destination is busy with incoming migrations
Line 352:                     # release semaphore and give other outgoing migrations
Line 353:                     # a chance
Line 354:                     time.sleep(5)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icabe79dfccb61ce43489a9a242a5390e73979649
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list