Maor Lipchuk has posted comments on this change.

Change subject: engine(wip): Add action type for previewed snapshot.
......................................................................


Patch Set 8:

(1 comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
Line 126:         setSucceeded(succeeded);
Line 127:     }
Line 128: 
Line 129:     private Guid getSnapshotId() {
Line 130:         if (snapshotId == null) {
When will snapshotId be null?
 >  When it is uninitialized, this is kind of a lazy initialization 
 > implelemntation

Can't we just assume that null almost means "undo preview"?
 >  I'm not sure if I understood your suggestion completly, I prefer to avoid 
 > using null values to indicate a state, it could be error prone IMHO

Actually, we don't really need the snapshot id, we know that when we get to 
this 
command then the VM is already in PREVIEW and the user can only do UNDO or 
COMMIT (or it is stateless VM which means UNDO), but to keep the change small 
for now, and fix this bug, we might do it for later phase.
Line 131:             Snapshot snapshot = null;
Line 132:             Guid snapshotId = getParameters().getDstSnapshotId();
Line 133:             if (getParameters().getSnapshotPreviewAction() == 
SnapshotPreviewAction.UNDO) {
Line 134:                 snapshot = getSnapshotDao().get(getVmId(), 
SnapshotType.PREVIEW);


-- 
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: 8
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: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[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

Reply via email to