Arik Hadas has posted comments on this change. Change subject: core: Add memory allocation check to hibernate VM ......................................................................
Patch Set 14: Code-Review+1 (5 comments) minor comments http://gerrit.ovirt.org/#/c/30883/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java: Line 47: memories.remove(StringUtils.EMPTY); Line 48: return memories; Line 49: } Line 50: Line 51: public static List<DiskImage> createDiskDummies(VM vm, long metadataSize) { can you replace vm with the memory size? that way you could use it from the test instead of the duplicated code which currently exists there Line 52: DiskImage memoryVolume = new DiskImage(); Line 53: memoryVolume.setDiskAlias("memory"); Line 54: memoryVolume.setvolumeFormat(VolumeFormat.RAW); Line 55: memoryVolume.setSize(vm.getTotalMemorySizeInBytes()); Line 64: dataVolume.setActualSizeInBytes(metadataSize); Line 65: dataVolume.getSnapshots().add(dataVolume); Line 66: Line 67: List<DiskImage> disksList = Arrays.asList(memoryVolume, dataVolume); Line 68: return disksList; please inline disksList Line 69: } http://gerrit.ovirt.org/#/c/30883/14/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java: Line 155: assertFalse(vm.getManagedVmDeviceMap().containsKey(snapshotDisk.getId())); Line 156: } Line 157: Line 158: @Test Line 159: public void verifyDomainForMemory() { this check should probably be split to 2 - one until line 170 and one for the second part (with and without the threshold setting). that way you could use assertEqual/assertTrue etc right? Line 160: Guid sdId = Guid.newGuid(); Line 161: List<StorageDomain> storageDomains = createStorageDomains(sdId); Line 162: int vmSpaceInBytes = SizeConverter.convert(VM_SPACE_IN_MB, SizeConverter.SizeUnit.MB, SizeConverter.SizeUnit.BYTES).intValue(); Line 163: List<DiskImage> disksList = createDisks(vmSpaceInBytes); Line 159: public void verifyDomainForMemory() { Line 160: Guid sdId = Guid.newGuid(); Line 161: List<StorageDomain> storageDomains = createStorageDomains(sdId); Line 162: int vmSpaceInBytes = SizeConverter.convert(VM_SPACE_IN_MB, SizeConverter.SizeUnit.MB, SizeConverter.SizeUnit.BYTES).intValue(); Line 163: List<DiskImage> disksList = createDisks(vmSpaceInBytes); MemoryUtils.createDisks should be used Line 164: Line 165: StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(storageDomains, disksList); Line 166: assertThat(storageDomain, notNullValue()); Line 167: if (storageDomain != null) { Line 403: // Line 404: // assertFalse(VmHandler.verifyAddVm(new ArrayList<String>(), 0, null, Guid.NewGuid(), 0)); Line 405: // } Line 406: //} Line 407: please remove -- To view, visit http://gerrit.ovirt.org/30883 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1250e6ea8d67f8026d66cf06589538343d39756a Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[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
