Change in vdsm[master]: virt: Correct VM state before vm.cont() in _recover()

Shmuel Leib Melamud has posted comments on this change.

Change subject: virt: Correct VM state before vm.cont() in _recover()
......................................................................


Patch Set 1:

(1 comment)

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

Line 212:             except Exception:
Line 213:                 self.log.exception("Failed to destroy remote VM")
Line 214:         # if the guest was stopped before migration, we need to cont it
Line 215:         if self.hibernating:
Line 216:             self._vm.lastStatus = vmstatus.PAUSED
>  IMO better to reset this status when/if the hook raises, with a
> comment tha 
This may happen not only when the hook raises. self._vm._dom.save(fname)
may also raise an exception and result will be the same. Thus, the exceptions must be
catched at higher level than _startUnderlyingMigration() - in run().

But exceptions are already catched in run() and _recover() is used in all cases to reset
VM to the state it had before migration started. IMHO it is better to keep all recovery
code in one place.

Also, _vm.pause(SAVING_STATE) is invoked in _prepareGuest() called from run(). It is
logical to change the state back on the same level.

But I agree, it is not the best place. The best place should be _vm.cont(). We have
_vm.pause(SAVING_STATE) that means "pause the VM and put it into SAVING_STATE instead
of PAUSED". So we need also the opposite - _vm.cont(beforeState=SAVING_STATE) that
means "continue the VM that is currently in SAVING_STATE instead of PAUSED".

Or we can just take away the status check from _vm.cont().
Line 217:             self._vm.cont()
Line 218:         # either way, migration has finished
Line 219:         self._vm.lastStatus = vmstatus.UP
Line 220:         self._vm.send_status_event()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b1c7b4eecacf87ece48dc563fd2da294af0510b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud <smelamud(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Shmuel Leib Melamud <smelamud(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes