Vered Volansky has posted comments on this change. Change subject: core: Added VM memory validation on snapshot ......................................................................
Patch Set 4: (7 comments) http://gerrit.ovirt.org/#/c/30040/4/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 493: return false; Line 494: } Line 495: Line 496: List<DiskImage> disksList = getDisksListForChecks(); Line 497: MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(disksList); > Probably not part of this patch, but the MSD should be created from a list I think in this patch is appropriate, amended. Line 498: if (disksList.size() > 0) { Line 499: DiskImagesValidator diskImagesValidator = createDiskImageValidator(disksList); Line 500: if (!(validate(diskImagesValidator.diskImagesNotLocked()) Line 501: && validate(diskImagesValidator.diskImagesNotIllegal()) http://gerrit.ovirt.org/#/c/30040/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java: Line 20: Line 21: import java.util.ArrayList; Line 22: import java.util.Arrays; Line 23: import java.util.Collections; Line 24: import java.util.List; > please organize the imports as per project's conventions - java imports sho Done Line 25: Line 26: /** Line 27: * This builder creates the memory images for live snapshots with memory operation Line 28: */ http://gerrit.ovirt.org/#/c/30040/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java: Line 1: package org.ovirt.engine.core.bll.memory; Line 2: Line 3: import org.ovirt.engine.core.common.businessentities.DiskImage; Line 4: import java.util.List; > please organize the imports as per project's conventions. Done Line 5: Line 6: public interface MemoryImageBuilder { Line 7: /** Line 8: * Create the images http://gerrit.ovirt.org/#/c/30040/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java: Line 3: import org.apache.commons.lang.StringUtils; Line 4: import org.ovirt.engine.core.common.businessentities.DiskImage; Line 5: Line 6: import java.util.Collections; Line 7: import java.util.List; > please organize the imports as per project's conventions - java imports sho Done Line 8: Line 9: /** Line 10: * This builder is used when no memory image should be created Line 11: */ http://gerrit.ovirt.org/#/c/30040/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java: Line 5: import org.ovirt.engine.core.common.businessentities.VM; Line 6: import org.ovirt.engine.core.dal.dbbroker.DbFacade; Line 7: Line 8: import java.util.Collections; Line 9: import java.util.List; > please organize the imports as per project's conventions - java imports sho Done Line 10: Line 11: /** Line 12: * This builder is responsible to create the memory volumes for stateless snapshot - Line 13: * it just take the memory volume of the previously active snapshot http://gerrit.ovirt.org/#/c/30040/4/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 109: } Line 110: Line 111: private double getTotalSizeForNewDisks(Collection<DiskImage> diskImages) { Line 112: double totalSizeForDisks = 0.0; Line 113: if (null != diskImages) { > please put the null on the right hand side of the expression, as per the pr Done Line 114: for (DiskImage diskImage : diskImages) { Line 115: double sizeForDisk = diskImage.getSize(); Line 116: Line 117: if (diskImage.getVolumeFormat() == VolumeFormat.COW) { http://gerrit.ovirt.org/#/c/30040/4/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java: Line 299: doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotLocked(); Line 300: doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotIllegal(); Line 301: doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsExistAndActive(); Line 302: doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsWithinThresholds(); Line 303: doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForAllDisks(eq(Collections.<DiskImage>emptyList()), anyList()); Don't know how this sneaked in, going back to anyList. This is what jenkins complains about, BTW. Line 304: } Line 305: Line 306: private void setUpGeneralValidations() { Line 307: doReturn(Boolean.TRUE).when(cmd).validateVM(vmValidator); -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> Gerrit-Reviewer: Allon Mureinik <[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
