Maor Lipchuk has posted comments on this change. Change subject: core: Introduce TryBackToCinderSnapshotCommand. ......................................................................
Patch Set 19: (5 comments) https://gerrit.ovirt.org/#/c/42062/19/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/TryBackToCinderSnapshotCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/TryBackToCinderSnapshotCommand.java: Line 35: * Remove old image vm map. Line 36: */ Line 37: @Override Line 38: protected void processOldImageFromDb() { Line 39: updateOldImageActive(Snapshot.SnapshotType.PREVIEW, false); > supdateOldImageActive/updateOldImageAsActive Done Line 40: } Line 41: Line 42: @Override Line 43: protected void executeCommand() { Line 76: * @param active Line 77: * The active state. Line 78: */ Line 79: protected void updateOldImageActive(Snapshot.SnapshotType snapshotType, boolean active) { Line 80: Guid oldImageId = findImageForSameDrive(snapshotType); > The naming here is really confusing... maybe change to getImageIdBySnapshot done Line 81: if (oldImageId == null) { Line 82: log.error("Can't find image to update to active '{}', snapshot type '{}', original image id '{}'", Line 83: active, Line 84: snapshotType, Line 87: } Line 88: Line 89: DiskImage oldImage = getDiskImageDao().getSnapshotById(oldImageId); Line 90: oldImage.setActive(active); Line 91: getImageDao().update(oldImage.getImage()); > formatter Done Line 92: } Line 93: Line 94: private CinderDisk initializeNewCinderVolumeDisk(Guid newVolumeId) { Line 95: CinderDisk clonedDiskFromSnapshot = (CinderDisk) getDiskDao().get(getParameters().getContainerId()); Line 91: getImageDao().update(oldImage.getImage()); Line 92: } Line 93: Line 94: private CinderDisk initializeNewCinderVolumeDisk(Guid newVolumeId) { Line 95: CinderDisk clonedDiskFromSnapshot = (CinderDisk) getDiskDao().get(getParameters().getContainerId()); > same This is already fomatted, I now formatted all the class Line 96: Line 97: // override volume type and volume format to Unassigned and unassigned for Cinder. Line 98: clonedDiskFromSnapshot.setVolumeType(VolumeType.Unassigned); Line 99: clonedDiskFromSnapshot.setvolumeFormat(VolumeFormat.Unassigned); Line 93: Line 94: private CinderDisk initializeNewCinderVolumeDisk(Guid newVolumeId) { Line 95: CinderDisk clonedDiskFromSnapshot = (CinderDisk) getDiskDao().get(getParameters().getContainerId()); Line 96: Line 97: // override volume type and volume format to Unassigned and unassigned for Cinder. > please extract to CinderBroker (e.g. reuse volumeToCinderDisk if possible) I'm not sure that I understood how we can do it elegantly. We are setting values from the parameters, and also set the status of the image as locked. The API of this method will big and un-maintainable... Also why broker and not CinderStorageHelper? Line 98: clonedDiskFromSnapshot.setVolumeType(VolumeType.Unassigned); Line 99: clonedDiskFromSnapshot.setvolumeFormat(VolumeFormat.Unassigned); Line 100: clonedDiskFromSnapshot.setImageStatus(ImageStatus.LOCKED); Line 101: clonedDiskFromSnapshot.setCreationDate(new Date()); -- To view, visit https://gerrit.ovirt.org/42062 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I532fed7fe11a033c8b86251e5ad8b73230801c2d Gerrit-PatchSet: 19 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
