Hello Dan Kenigsberg, Michal Skrivanek,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/31669
to review the following change.
Change subject: vm: split migration completion in smaller methods
......................................................................
vm: split migration completion in smaller methods
Refactor _waitForIncomingMigrationFinish in smaller
methods for clarity and to prepare for better
timeout handling. The method was renamed for better
clarity: in the case of dehibernation it wasn't
waiting for anything.
No functional changes.
Change-Id: Ic469814afc607951870f4171499cef958855fc54
Signed-off-by: Francesco Romani <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/28509
Reviewed-by: Michal Skrivanek <[email protected]>
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M vdsm/virt/vm.py
1 file changed, 43 insertions(+), 37 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/31669/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 03c8a91..729b469 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2288,7 +2288,7 @@
if ('migrationDest' in self.conf or 'restoreState' in self.conf) \
and self.lastStatus != vmstatus.DOWN:
- self._waitForIncomingMigrationFinish()
+ self._completeIncomingMigration()
self.lastStatus = vmstatus.UP
if self._initTimePauseCode:
@@ -3943,7 +3943,7 @@
else:
self._monitorResponse = 0
- def _waitForIncomingMigrationFinish(self):
+ def _completeIncomingMigration(self):
if 'restoreState' in self.conf:
self.cont()
del self.conf['restoreState']
@@ -3951,40 +3951,9 @@
hooks.after_vm_dehibernate(self._dom.XMLDesc(0), self.conf,
{'FROM_SNAPSHOT': fromSnapshot})
elif 'migrationDest' in self.conf:
- timeout = config.getint('vars', 'migration_destination_timeout')
- self.log.debug("Waiting %s seconds for end of migration", timeout)
- self._incomingMigrationFinished.wait(timeout)
-
- try:
- # Would fail if migration isn't successful,
- # or restart vdsm if connection to libvirt was lost
- self._dom = NotifyingVirDomain(
- self._connection.lookupByUUIDString(self.id),
- self._timeoutExperienced)
-
- if not self._incomingMigrationFinished.isSet():
- state = self._dom.state(0)
- if state[0] == libvirt.VIR_DOMAIN_PAUSED:
- if state[1] == libvirt.VIR_DOMAIN_PAUSED_MIGRATION:
- raise MigrationError("Migration Error - Timed out "
- "(did not receive success "
- "event)")
- self.log.debug("NOTE: incomingMigrationFinished event has "
- "not been set and wait timed out after %d "
- "seconds. Current VM state: %d, reason %d. "
- "Continuing with VM initialization anyway.",
- timeout, state[0], state[1])
- except libvirt.libvirtError as e:
- if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
- if not self._incomingMigrationFinished.isSet():
- newMsg = ('%s - Timed out '
- '(did not receive success event)' %
- (e.args[0] if len(e.args) else
- 'Migration Error'))
- e.args = (newMsg,) + e.args[1:]
- raise MigrationError(e.get_error_message())
- raise
-
+ usedTimeout = self._waitForUnderlyingMigration()
+ self._attachLibvirtDomainAfterMigration(
+ self._incomingMigrationFinished.isSet(), usedTimeout)
self._domDependentInit()
del self.conf['migrationDest']
hooks.after_vm_migrate_destination(self._dom.XMLDesc(0), self.conf)
@@ -4001,6 +3970,43 @@
del self.conf['username']
self.saveState()
self.log.debug("End of migration")
+
+ def _waitForUnderlyingMigration(self):
+ timeout = config.getint('vars', 'migration_destination_timeout')
+ self.log.debug("Waiting %s seconds for end of migration", timeout)
+ self._incomingMigrationFinished.wait(timeout)
+ return timeout
+
+ def _attachLibvirtDomainAfterMigration(self, migrationFinished, timeout):
+ try:
+ # Would fail if migration isn't successful,
+ # or restart vdsm if connection to libvirt was lost
+ self._dom = NotifyingVirDomain(
+ self._connection.lookupByUUIDString(self.id),
+ self._timeoutExperienced)
+
+ if not migrationFinished:
+ state = self._dom.state(0)
+ if state[0] == libvirt.VIR_DOMAIN_PAUSED:
+ if state[1] == libvirt.VIR_DOMAIN_PAUSED_MIGRATION:
+ raise MigrationError("Migration Error - Timed out "
+ "(did not receive success "
+ "event)")
+ self.log.debug("NOTE: incomingMigrationFinished event has "
+ "not been set and wait timed out after %d "
+ "seconds. Current VM state: %d, reason %d. "
+ "Continuing with VM initialization anyway.",
+ timeout, state[0], state[1])
+ except libvirt.libvirtError as e:
+ if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
+ if not migrationFinished:
+ newMsg = ('%s - Timed out '
+ '(did not receive success event)' %
+ (e.args[0] if len(e.args) else
+ 'Migration Error'))
+ e.args = (newMsg,) + e.args[1:]
+ raise MigrationError(e.get_error_message())
+ raise
def _underlyingCont(self):
hooks.before_vm_cont(self._dom.XMLDesc(0), self.conf)
@@ -5420,7 +5426,7 @@
# after RESUMED/RESUMED_MIGRATED (when VM status is PAUSED
# when migration completes, see qemuMigrationFinish function).
# In this case self._dom is None because the function
- # _waitForIncomingMigrationFinish didn't update it yet.
+ # _completeIncomingMigration didn't update it yet.
try:
domxml = self._dom.XMLDesc(0)
except AttributeError:
--
To view, visit http://gerrit.ovirt.org/31669
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic469814afc607951870f4171499cef958855fc54
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
