Arik Hadas has posted comments on this change.
Change subject: core: lock active snapshot during undo preview process
......................................................................
Patch Set 2: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
Line 226: targetSnapshot.getId(),
Line 227: getCompensationContext());
Line 228: getSnapshotDao().remove(targetSnapshot.getId());
Line 229: // add active snapshot with status locked, so that other
commands that depend on the VM's snapshots won't run in parallel
Line 230: snapshotsManager.addActiveSnapshot(targetSnapshot.getId(),
getVm(), SnapshotStatus.LOCKED, getCompensationContext());
yeah I also thought about it - the code is simpler that way, and as you were
saying, the case of diskless snapshot is rare and don't worth it in my opinion..
Line 231: }
Line 232:
Line 233: /**
Line 234: * All snapshots who derive from the snapshot which is {@link
SnapshotStatus#IN_PREVIEW}, up to it (excluding), will
Line 381: protected void endVmCommand() {
Line 382: super.endVmCommand();
Line 383:
Line 384: // if we got here, the target snapshot exists for sure
Line 385:
getSnapshotDao().updateStatus(getParameters().getDstSnapshotId(),
SnapshotStatus.OK);
right, will change
Line 386: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
Line 84: */
Line 85: public Snapshot addActiveSnapshot(Guid snapshotId,
Line 86: VM vm,
Line 87: SnapshotStatus snapshotStatus,
Line 88: final CompensationContext compensationContext) {
no, I disagree - it's better to have one place that contain common stuff
instead of spreading it in multiple places (Don't Repeat Yourself principle)
such that if we'll want to change something (such as the description of the
active snapshot) we'll need to change only in one place
Line 89: return addSnapshot(snapshotId,
Line 90: "Active VM",
Line 91: snapshotStatus,
Line 92: SnapshotType.ACTIVE,
--
To view, visit http://gerrit.ovirt.org/10845
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7a2064d5130a6fa49360a8ad8e9a1004fc44da2
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
