Liron Ar has posted comments on this change.
Change subject: engine(wip): Add action type for previewed snapshot.
......................................................................
Patch Set 8: Code-Review-1
(8 comments)
few issues inside, giving -1 so it will be seen.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
Line 87: freeLock();
Line 88: }
Line 89: }
Line 90:
Line 91: // The snapshot being restored to
there is no need to load the snapshot multiple times, it's already loaded in
the method that you added
Line 92: Snapshot targetSnapshot =
getSnapshotDao().get(getSnapshotId());
Line 93:
Line 94: if (targetSnapshot == null) {
Line 95: throw new VdcBLLException(VdcBllErrors.ENGINE, "Can't find
target snapshot by id: "
Line 129: private Guid getSnapshotId() {
Line 130: if (snapshotId == null) {
Line 131: Snapshot snapshot = null;
Line 132: Guid snapshotId = getParameters().getDstSnapshotId();
Line 133: if (getParameters().getSnapshotPreviewAction() ==
SnapshotPreviewAction.UNDO) {
please change to switch
Line 134: snapshot = getSnapshotDao().get(getVmId(),
SnapshotType.PREVIEW);
Line 135: } else if (getParameters().getSnapshotPreviewAction() ==
SnapshotPreviewAction.COMMIT) {
Line 136: snapshot = getSnapshotDao().get(getVmId(),
SnapshotType.ACTIVE);
Line 137: }
Line 132: Guid snapshotId = getParameters().getDstSnapshotId();
Line 133: if (getParameters().getSnapshotPreviewAction() ==
SnapshotPreviewAction.UNDO) {
Line 134: snapshot = getSnapshotDao().get(getVmId(),
SnapshotType.PREVIEW);
Line 135: } else if (getParameters().getSnapshotPreviewAction() ==
SnapshotPreviewAction.COMMIT) {
Line 136: snapshot = getSnapshotDao().get(getVmId(),
SnapshotType.ACTIVE);
what about statelss snapshot? won't work now..
Line 137: }
Line 138:
Line 139: if (snapshot != null) {
Line 140: snapshotId = snapshot.getId();
Line 136: snapshot = getSnapshotDao().get(getVmId(),
SnapshotType.ACTIVE);
Line 137: }
Line 138:
Line 139: if (snapshot != null) {
Line 140: snapshotId = snapshot.getId();
in that case it will be better to set it to Guid.Empty, that way you won't
perform the db queries multiple times and also all won't need to change the
checks to check for null instead of Guid.EMpty
Line 141: }
Line 142: }
Line 143: return snapshotId;
Line 144: }
Line 328: protected VdcActionType getChildActionType() {
Line 329: return VdcActionType.RestoreFromSnapshot;
Line 330: }
Line 331:
Line 332: private List<DiskImage> getImagesList() {
it will be null now
Line 333: if (getParameters().getImagesList() == null &&
!getSnapshotId().equals(Guid.Empty)) {
Line 334: getParameters().setImagesList(
Line 335:
getDiskImageDao().getAllSnapshotsForVmSnapshot(getSnapshotId()));
Line 336: }
Line 368:
Line 369: if (!canRunActionOnNonManagedVm()) {
Line 370: return false;
Line 371: }
Line 372:
it won't be Guid.Empty ever now
Line 373: if (Guid.Empty.equals(getSnapshotId())) {
Line 374: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID);
Line 375: }
Line 376:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreStatelessVmCommand.java
Line 42: if (imagesList != null && imagesList.size() > 0) {
Line 43: /**
Line 44: * restore all snapshots
Line 45: */
Line 46: RestoreAllSnapshotsParameters tempVar = new
RestoreAllSnapshotsParameters(getVm().getId(), snapshotId,
SnapshotPreviewAction.COMMIT);
why? isn't it supposed to undo?
Line 47: tempVar.setShouldBeLogged(false);
Line 48: tempVar.setImagesList(imagesList);
Line 49: VdcReturnValueBase vdcReturn =
Line 50:
Backend.getInstance().runInternalAction(VdcActionType.RestoreAllSnapshots,
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java
Line 259: STATELESS,
Line 260: PREVIEW
Line 261: }
Line 262:
Line 263: public enum SnapshotPreviewAction {
1. please rename the enum, as it's not always preview.
2. the enum isn't related to the snapshot class.
3. what about Commot.gwt.xml?
Line 264: UNDO,
Line 265: COMMIT,
Line 266: NONE
Line 267: }
--
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: Michael Pasternak <[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
