Change in vdsm[master]: vm: detect migration completed on recovery

danken at redhat.com danken at redhat.com
Fri Jun 27 22:21:53 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: vm: detect migration completed on recovery
......................................................................


Patch Set 9: Code-Review-1

(4 comments)

minor questions

http://gerrit.ovirt.org/#/c/28511/9//COMMIT_MSG
Commit Message:

Line 17: 
Line 18: To handle this situation, we
Line 19: - connect early to the domain, on recovery. We know
Line 20:   this is safe because on recovery we iterate on the very
Line 21:   domain list libvirt provided to us moments before,
but in what way this is "safe"? it might be only a problematic choice of words.

I think that you mean is that
- now, we wait for VMs from a list produced by the former vdsm process, which may have died hour ago.
- with this patch, the list of VMs is much more fresh, limiting the danger of needless wait.

am I right?
Line 22:   so the domain will be present
Line 23: - inspect the domain state *before* waitingr migration
Line 24:   termination, and skip the wait if the domain is detected
Line 25:   running.


Line 19: - connect early to the domain, on recovery. We know
Line 20:   this is safe because on recovery we iterate on the very
Line 21:   domain list libvirt provided to us moments before,
Line 22:   so the domain will be present
Line 23: - inspect the domain state *before* waitingr migration
waitingr -> waiting for
Line 24:   termination, and skip the wait if the domain is detected
Line 25:   running.
Line 26: 
Line 27: A nice side-effect, this patch also tries to clarify a


http://gerrit.ovirt.org/#/c/28511/9/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 3010:         if self.recovering:
Line 3011:             self._dom = NotifyingVirDomain(
Line 3012:                 self._connection.lookupByUUIDString(self.id),
Line 3013:                 self._timeoutExperienced)
Line 3014:         elif 'migrationDest' in self.conf:
that's simply "not initDomain", which I find clearer.
Line 3015:             pass  # self._dom will be None until migration ends.
Line 3016:         elif 'restoreState' in self.conf:
Line 3017:             fromSnapshot = self.conf.get('restoreFromSnapshot', False)
Line 3018:             srcDomXML = self.conf.pop('_srcDomXML')


Line 3587:                 try:
Line 3588:                     if self._isDomainRunning():
Line 3589:                         waitMigration = False
Line 3590:                         self.log.info('migration completed while recovering!')
Line 3591:                 except (libvirt.libvirtError, AttributeError):
what is this evil AttributeError? due to self._dom being set to None asynchronously?
Line 3592:                     self.log.exception('migration failed while recovering!')
Line 3593:                     self.setDownStatus(ERROR, vmexitreason.MIGRATION_FAILED)
Line 3594:                     return
Line 3595:             if waitMigration:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I451c2a940693842e9bf7c63ccc117e75026bb11b
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list