Allon Mureinik has posted comments on this change. Change subject: core: deleting template with shared volumes should fail ......................................................................
Patch Set 5: Code-Review+1 (2 comments) +1 on the code, please see comments on the commit message. @Cherryn - can you please take a look at the error message at backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties ? @Omer/Roy - any insight from a virt perspective? .................................................... Commit Message Line 7: core: deleting template with shared volumes should fail Line 8: Line 9: When deleting a template that has disks that other disks were created Line 10: from, the operation should fail to avoid possible deletion a part Line 11: of the template or an error on the delete attempt as the volume is shared. This sentence isn't very clear. Consider the following: "When deleting a template that contain disks which have VM disks based upon them the operation should fail on the engine side. If it is sent to VDSM, it would fail with an error on deleting a shared volume, which may leave the template in an illegal state." Line 12: Line 13: Change-Id: If781238d7be4768b1086645c64b4ccf807dc6108 Line 14: Bug-Url: https://bugzilla.redhat.com/953492 .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java Line 157: return checkNoDisksBasedOnTemplateDisks(); Line 158: } Line 159: Line 160: private boolean checkNoDisksBasedOnTemplateDisks() { Line 161: List<String> disksInfo = null; I'd just initialize an empty list here and avoid the hassle, but it's a matter of taste, I guess. Line 162: for (DiskImage diskImage : imageTemplates) { Line 163: List<DiskImage> basedDisks = getDiskImageDAO().getAllSnapshotsForParent(diskImage.getImageId()); Line 164: for (DiskImage basedDisk : basedDisks) { Line 165: if (disksInfo == null) { -- To view, visit http://gerrit.ovirt.org/22162 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If781238d7be4768b1086645c64b4ccf807dc6108 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Cheryn Tan <cheryn...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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
