Maor Lipchuk has posted comments on this change. Change subject: core: Introduce restore all cinder snapshots command. ......................................................................
Patch Set 3: (4 comments) https://gerrit.ovirt.org/#/c/42338/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RestoreAllCinderSnapshotsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RestoreAllCinderSnapshotsCommand.java: Line 30: Line 31: @Override Line 32: protected void executeVmCommand() { Line 33: for (CinderDisk cinderDisk : getParameters().getCinderDisks()) { Line 34: ImagesContainterParametersBase params = > consider extracting to create params method Done Line 35: new RestoreFromSnapshotParameters(cinderDisk.getImageId(), Line 36: getParameters().getVmId(), Line 37: getParameters().getSnapshot(), Line 38: getParameters().getRemovedSnapshotId()); Line 38: getParameters().getRemovedSnapshotId()); Line 39: params.setParentCommand(getActionType()); Line 40: params.setStorageDomainId(cinderDisk.getStorageIds().get(0)); Line 41: params.setParentParameters(getParameters()); Line 42: if (getParameters().getSnapshot().getType() != Snapshot.SnapshotType.REGULAR) { > please add a comment in which flow it could be REGULAR Done Line 43: cinderDisk.setActive(true); Line 44: getImageDao().update(cinderDisk.getImage()); Line 45: } Line 46: restoreCinderDisk(cinderDisk, params); Line 40: params.setStorageDomainId(cinderDisk.getStorageIds().get(0)); Line 41: params.setParentParameters(getParameters()); Line 42: if (getParameters().getSnapshot().getType() != Snapshot.SnapshotType.REGULAR) { Line 43: cinderDisk.setActive(true); Line 44: getImageDao().update(cinderDisk.getImage()); > do we have some rollback for this in case of failure? no, since once you started to delete volumes there is no turning back. I could mark the disk as ILLEGAL though, but lets discuss on this on another patch Line 45: } Line 46: restoreCinderDisk(cinderDisk, params); Line 47: } Line 48: setSucceeded(true); Line 56: new SubjectEntity(VdcObjectType.Storage, cinderDisk.getStorageIds().get(0))); Line 57: try { Line 58: return future.get(); Line 59: } catch (InterruptedException | ExecutionException e) { Line 60: e.printStackTrace(); > maybe worth adding some log.. Done Line 61: } Line 62: return null; Line 63: } Line 64: -- To view, visit https://gerrit.ovirt.org/42338 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3189bd1c586957800eb5784877cd605ae9f847fc Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
