Omer Frenkel has posted comments on this change. Change subject: core: Added VM memory validation on snapshot ......................................................................
Patch Set 6: (2 comments) just some minor comments http://gerrit.ovirt.org/#/c/30040/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java: Line 145: return toReturn; Line 146: } Line 147: Line 148: private boolean validateStorageDomains(List<DiskImage> newDisksList) { Line 149: List<DiskImage> cloneDisksList = getMemoryImageBuilder().getDisksToBeCreated(); why did you name it cloneDisks? its memoryDisks, no? Line 150: List<DiskImage> disksList = getAllDisks(newDisksList, cloneDisksList); Line 151: MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(disksList); Line 152: return validate(sdValidator.allDomainsExistAndActive()) Line 153: && validate(sdValidator.allDomainsWithinThresholds()) http://gerrit.ovirt.org/#/c/30040/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java: Line 107: private static Integer getLowDiskSpaceThreshold() { Line 108: return Config.<Integer> getValue(ConfigValues.FreeSpaceCriticalLowInGB); Line 109: } Line 110: Line 111: private double getTotalSizeForNewDisks(Collection<DiskImage> diskImages) { maybe add some doc on these 2 methods, i dont understand why we need different cases for vm disks and memory ("cloned") disks? Line 112: double totalSizeForDisks = 0.0; Line 113: if (diskImages != null) { Line 114: for (DiskImage diskImage : diskImages) { Line 115: double sizeForDisk = diskImage.getSize(); -- To view, visit http://gerrit.ovirt.org/30040 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8fc127147bdea8737fecb034c88079521b8a8bd6 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
