Vered Volansky has posted comments on this change. Change subject: core: Add memory allocation check to hibernate VM ......................................................................
Patch Set 14: (5 comments) Addressing comments in the next patchset 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 Done 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 Done 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 t The threshold is set no matter what, I just changed it. Allon prefers the assertThat way, I can use assertTrue either way. I can separate it into two tests anyway. 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 Done 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 I got checkstyle without it... -- 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
