Change in vdsm[ovirt-3.6]: vm: safer removal of the recovery file

fromani at redhat.com fromani at redhat.com
Mon Jan 25 13:58:48 UTC 2016


Francesco Romani has submitted this change and it was merged.

Change subject: vm: safer removal of the recovery file
......................................................................


vm: safer removal of the recovery file

If a user shutdowns a VM from inside the guest, Vdsm leaves
a stale recovery file.
The cause is a race between onQemuDeath, being asynchronous, and
Vm.destroy().
The actual culprit is the ordering of events between onQemuDeath and
destroy(), and the shutdown from within the guest is just the easiest
way to trigger this issue.

In the QEMU death path, Vdsm calls releaseVm, which wipes the recovery
file, and after a succesful completion does saveState, which recreates
the recovery file.
Later, on destroy(), Vdsm tries to do releaseVm again, but releaseVm
will not run, because of the flag that the previous succesful
call set. releaseVm must run exactly once.

Correct sequence:
- destroy()
-- vm._destroyed = True
-- releaseVm() - done (vm._released == False)
-- vm._released = True
[later]
- onQemuDeath
-- releaseVm() - skipped (vm._released == True)
-- saveState() - skipped (vm._destroyed == True)

Faulty sequence:
- onQemuDeath()
-- releaseVm() - done (vm._released == False)
-- vm._released = True
-- saveState() - done <BUG!> (vm._destroyed == False)
[later]
- destroy()
-- vm._destroyed = True
-- releaseVm() - skipped (vm._released == True)

The fix is to protect the recoveryFile handling with a lock.

Change-Id: I9a8d2b34599da17bee35c9bb694122ecd9d58068
Backport-To: 3.6
Backport-To: 3.5
Bug-Url: https://bugzilla.redhat.com/1253043
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/51387
Continuous-Integration: Jenkins CI
Reviewed-by: Milan Zamazal <mzamazal at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/52515
---
M vdsm/virt/vm.py
1 file changed, 17 insertions(+), 10 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified; Looks good to me, approved
  Milan Zamazal: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I9a8d2b34599da17bee35c9bb694122ecd9d58068
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>


More information about the vdsm-patches mailing list