Federico Simoncelli has uploaded a new change for review.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
vm: snapshot transient disk check should be per disk
Live snapshot is allowed if all the disks in the snapshot request are not transient.
Change-Id: I3775e2218186e56e99740c6f91c8c98484529892 Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/vm.py 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/24867/1
diff --git a/vdsm/vm.py b/vdsm/vm.py index d3b4e71..edf7da3 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -3941,9 +3941,6 @@ if self.isMigrating(): return errCode['migInProgress']
- if self.hasTransientDisks(): - return errCode['transientErr'] - for drive in snapDrives: baseDrv, tgetDrv = _normSnapDriveParams(drive)
@@ -3966,8 +3963,13 @@ return errCode['snapshotErr']
if vmDrive.hasVolumeLeases: + self.log.error('disk %s has volume leases', vmDrive.name) return errCode['noimpl']
+ if vmDrive.transientDisk: + self.log.error('disk %s is a transient disk', vmDrive.name) + return errCode['transientErr'] + vmDevName = vmDrive.name
newDrives[vmDevName] = tgetDrv.copy()
Federico Simoncelli has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 1: Verified+1
Verified (with http://gerrit.ovirt.org/20029 ): successful live snapshot and failed prepareVolumePath (rollback).
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6452/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7236/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7354/ : SUCCESS
Federico Simoncelli has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2: Verified+1
Verified (with http://gerrit.ovirt.org/20029 ): successful live snapshot and failed prepareVolumePath (rollback).
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6454/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7238/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7356/ : SUCCESS
Daniel Erez has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2: Code-Review+2
Liron Ar has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/24867/2/vdsm/vm.py File vdsm/vm.py:
Line 3955: self.log.debug("The volume is already in use: %s", tgetDrv) Line 3956: continue # Next drive Line 3957: Line 3958: try: Line 3959: vmDrive = self._findDriveByUUIDs(baseDrv) question - for transient won't we fail here already (before the added check) with lookup eror? as the engine doesn't know the volume id of the currently "transient" volume. Line 3960: except LookupError: Line 3961: # The volume we want to snapshot doesn't exist Line 3962: self.log.error("The base volume doesn't exist: %s", baseDrv) Line 3963: return errCode['snapshotErr']
Liron Ar has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2: Code-Review-1
Federico Simoncelli has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/24867/2/vdsm/vm.py File vdsm/vm.py:
Line 3955: self.log.debug("The volume is already in use: %s", tgetDrv) Line 3956: continue # Next drive Line 3957: Line 3958: try: Line 3959: vmDrive = self._findDriveByUUIDs(baseDrv)
question - for transient won't we fail here already (before the added check
We don't have an additional volume on the vdsm side. In fact when you request the transient disk hotunplug we do the exact same thing (see line 3657). Line 3960: except LookupError: Line 3961: # The volume we want to snapshot doesn't exist Line 3962: self.log.error("The base volume doesn't exist: %s", baseDrv) Line 3963: return errCode['snapshotErr']
Liron Ar has posted comments on this change.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
Patch Set 2: -Code-Review
Federico Simoncelli has submitted this change and it was merged.
Change subject: vm: snapshot transient disk check should be per disk ......................................................................
vm: snapshot transient disk check should be per disk
Live snapshot is allowed if all the disks in the snapshot request are not transient.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1065886 Change-Id: I3775e2218186e56e99740c6f91c8c98484529892 Signed-off-by: Federico Simoncelli fsimonce@redhat.com Reviewed-on: http://gerrit.ovirt.org/24867 Reviewed-by: Daniel Erez derez@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/vm.py 1 file changed, 5 insertions(+), 3 deletions(-)
Approvals: Federico Simoncelli: Verified Daniel Erez: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved
vdsm-patches@lists.fedorahosted.org