Liron Aravot 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()); possibly we could add the snapshot in ok status in case it's diskless and save a further db call (take that out of this method). consider it. though the case of diskless snapshot is rare, but if we can avoid it - it's better. 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); seems to be like this should be before the vm is unlocked in super.endVmCommand(); 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) { I'd just call the addSnapshot method instead of adding this method..this class is getting too loaded with methods for specific usecases. 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