Maor Lipchuk has posted comments on this change.

Change subject: core: fail deletion of template disk with derived disks
......................................................................


Patch Set 2:

(1 comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
Line 200:     private DiskImagesValidator createDiskImagesValidator(DiskImage 
disk) {
Line 201:       return new DiskImagesValidator(Arrays.asList(disk));
Line 202:     }
Line 203: 
Line 204:     private boolean checkDerivedDisksFromDiskNotExist(DiskImage 
diskImage) {
The only reason this method is being called in that class is for a template 
reason, and I think it should not be used for any other reason.
I would add Template in the method name to avoid confusion.

I'm not sure what was wrong with my first suggestion (why "has no" isn't 
correct), if you don't like the "has no", perhaps consider to change the 
validation to return diskImagesHaveDerivedDisks instead... other then that I 
will be happy to hear other suggestion.
Line 205:         return 
validate(createDiskImagesValidator(diskImage).diskImagesHaveNoDerivedDisks(getParameters().getStorageDomainId()));
Line 206:     }
Line 207: 
Line 208:     private List<String> getNamesOfDerivedVmsFromTemplate(DiskImage 
diskImage) {


-- 
To view, visit http://gerrit.ovirt.org/22428
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca48993f40f3c5f7b603f33add7049e78e6c4e9
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@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

Reply via email to