Sergey Gotliv has posted comments on this change. Change subject: engine: Add action type for previewed snapshot. ......................................................................
Patch Set 11: (5 comments) http://gerrit.ovirt.org/#/c/20420/11//COMMIT_MSG Commit Message: Line 6: Line 7: engine: Add action type for previewed snapshot. Line 8: Line 9: Adding an enum which distinguish between Undo, Commit Line 10: and Restore from stateless operations What do you mean by stateless operations? Line 11: on snapshot. Line 12: Line 13: Change-Id:If877befc5058c3412ae3d3731d5beacbc09e5c97 Line 14: Related to Bug-Url: https://bugzilla.redhat.com/970115 http://gerrit.ovirt.org/#/c/20420/11/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 141: case RESTORE_STATELESS: Line 142: snapshot = getSnapshotDao().get(getVmId(), SnapshotType.STATELESS); Line 143: break; Line 144: default: Line 145: log.error("No valid Snapshot Action has been transferred"); I would throw an exception here, because all possible enum values are listed here. Bu t at least please, print the action in the log message Line 146: } Line 147: Line 148: if (snapshot != null) { Line 149: snapshotId = snapshot.getId(); Line 422: !validate(storageValidator.allDomainsWithinThresholds()) || Line 423: !performImagesChecks() || Line 424: !validate(vmValidator.vmDown()) || Line 425: // if the user choose to commit a snapshot - the vm cant have disk snapshots attached to other vms. Line 426: (getSnapshotDao().get(getSnapshotId()).getType() == SnapshotType.REGULAR && !validate(vmValidator.vmNotHavingDeviceSnapshotsAttachedToOtherVms(false)))) { Don't you already fetched the snapshot in the previous method? Line 427: return false; Line 428: } Line 429: Line 430: Snapshot snapshot = getSnapshotDao().get(getSnapshotId()); http://gerrit.ovirt.org/#/c/20420/11/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RestoreAllSnapshotsParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RestoreAllSnapshotsParameters.java: Line 6: import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotAction; Line 7: import org.ovirt.engine.core.compat.Guid; Line 8: Line 9: public class RestoreAllSnapshotsParameters extends VmOperationParameterBase implements java.io.Serializable { Line 10: private static final long serialVersionUID = -8756081739745132849L; Why we need this if we don't regenerate it after the class was changed? Line 11: Line 12: private List<DiskImage> images; Line 13: private SnapshotAction snapshotAction; Line 14: Line 21: return images; Line 22: } Line 23: Line 24: public void setImages(List<DiskImage> images) { Line 25: this.images = images; Is this method in use? I am asking because you removed the constructor with images. Line 26: } Line 27: Line 28: public RestoreAllSnapshotsParameters() { Line 29: } -- 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: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mishka8...@yahoo.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
