Federico Simoncelli has uploaded a new change for review. Change subject: vm: pre-validate disks for snapshot ...................................................................... vm: pre-validate disks for snapshot This patch divides the snapshot disks validation from the path preparation in order to limit the rollback scenarios. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1065886 Change-Id: I4fb507c2f7268bb688af5fd187dcd033d0a068a2 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com&gt; Reviewed-on: http://gerrit.ovirt.org/20029 Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt; (cherry picked from commit a826455fbb70ba21e2e3240382a4293eca2bf661) --- M vdsm/vm.py 1 file changed, 15 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/25006/1 diff --git a/vdsm/vm.py b/vdsm/vm.py index 8b74187..dbe77db 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -3910,7 +3910,6 @@ vmDrive = self._findDriveByUUIDs(baseDrv) except LookupError: # The volume we want to snapshot doesn't exist - _rollbackDrives(newDrives) self.log.error("The base volume doesn't exist: %s", baseDrv) return errCode['snapshotErr'] @@ -3924,22 +3923,29 @@ newDrives[vmDevName]["name"] = vmDevName newDrives[vmDevName]["format"] = "cow" + # If all the drives are the current ones, return success + if len(newDrives) == 0: + self.log.debug('all the drives are already in use, success') + return {'status': doneCode} + + preparedDrives = {} + + for vmDevName, vmDevice in newDrives.iteritems(): + # Adding the device before requesting to prepare it as we want + # to be sure to teardown it down even when prepareVolumePath + # failed for some unknown issue that left the volume active. + preparedDrives[vmDevName] = vmDevice try: newDrives[vmDevName]["path"] = \ self.cif.prepareVolumePath(newDrives[vmDevName]) except Exception: - _rollbackDrives(newDrives) - self.log.error("Unable to prepare the volume path " - "for the disk: %s", vmDevName, exc_info=True) + self.log.exception('unable to prepare the volume path for ' + 'disk %s', vmDevName) + _rollbackDrives(preparedDrives) return errCode['snapshotErr'] snapelem = _diskSnapshot(vmDevName, newDrives[vmDevName]["path"]) disks.appendChild(snapelem) - - # If all the drives are the current ones, return success - if len(newDrives) == 0: - self.log.debug("All the drives are already in use, success") - return {'status': doneCode} snap.appendChild(disks) -- To view, visit http://gerrit.ovirt.org/25006 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4fb507c2f7268bb688af5fd187dcd033d0a068a2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;