Daniel Erez has posted comments on this change. Change subject: engine: Add action type for snapshot. ......................................................................
Patch Set 14: (3 comments) http://gerrit.ovirt.org/#/c/20420/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java: Line 124: if (snapshot == null) { Line 125: switch (getParameters().getSnapshotAction()) { Line 126: case UNDO: Line 127: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.PREVIEW); Line 128: break; > isn't that wrong? should be fine as 'restoreSnapshotAndRemoveObsoleteSnapshots' method executes the 'undo' flow when SnapshotType is PREVIEW (the preview snapshot is created only for undo reasons so it makes sense to undo into it - since we return to the state that we took the preview and not to the 'regular' snapshot). Line 129: case COMMIT: Line 130: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.REGULAR, SnapshotStatus.IN_PREVIEW); Line 131: break; Line 132: case RESTORE_STATELESS: Line 126: case UNDO: Line 127: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.PREVIEW); Line 128: break; Line 129: case COMMIT: Line 130: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.REGULAR, SnapshotStatus.IN_PREVIEW); > this part seems wrong, should be fine as 'restoreSnapshotAndRemoveObsoleteSnapshots' method executes the 'commit' flow when SnapshotStatus is IN_PREVIEW. But, probably worth changing since it was done just because the UI sent "in preview" snapshot to restore when "Commit" is pressed. (as commented in line 256). I.e. simply change to Active? Line 131: break; Line 132: case RESTORE_STATELESS: Line 133: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.STATELESS); Line 134: break; Line 130: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.REGULAR, SnapshotStatus.IN_PREVIEW); Line 131: break; Line 132: case RESTORE_STATELESS: Line 133: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.STATELESS); Line 134: break; > this part seems to be wrong, again, confusing, but it's used similarly in RestoreStatelessVmCommand (as we return to the state when we took the stateless snapshot). Line 135: default: Line 136: log.errorFormat("The Snapshot Action {0} is not valid", getParameters().getSnapshotAction()); Line 137: } Line 138: -- To view, visit http://gerrit.ovirt.org/20420 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If877befc5058c3412ae3d3731d5beacbc09e5c97 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Michael Pasternak <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
