Maor Lipchuk has posted comments on this change. Change subject: core: Use allow snapshot when Adding a disk to VM. ......................................................................
Patch Set 3: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java Line 89: ImagesHandler.setDiskAlias(getParameters().getDiskInfo(), getVm()); Agreed, added a condition. After GUI will implement the allow snapshot field, this check should make more sence Line 142: private void setAllowSnapshotForDisk() { I agree, but since the GUI does not reflect the property now, and we always get false here there is not much logic in the condition now. But I think it still need to be in the can do action, since more validation should be added after GUI will reflect it to the user. Line 335: private void setVmSnapshotIdForDisk(AddImageFromScratchParameters parameters) { See my previous comment. For now this is the basic validation since GUI not reflecting it to the user, it should have more condition after user can pass us true or false. Line 338: } else { It is Guid.Empty by default. I prefer to initialize the value with Null since Guid.empty is more ugly IMHO. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java Line 49: mNewCreatedDiskImage.setShareable(getParameters().getDiskInfo().isShareable()); This field should be relevant when user will want to choose which disks he want to make non snapshotable. -- To view, visit http://gerrit.ovirt.org/4231 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I135669c5f7bfb85fda561a47258834cd35d7688b Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches