Allon Mureinik has posted comments on this change. Change subject: core: Added VM memory validation on snapshot ......................................................................
Patch Set 4: (7 comments) Generally +1, see minor comments inside. Also, jenkins seems to complain the test fails - can you take a look please? 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 that contains diskList and cloneDiskList below 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 should be on the top. 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. 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 should be on the top. 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 should be on the top. 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 project conventions. Line 114: for (DiskImage diskImage : diskImages) { Line 115: double sizeForDisk = diskImage.getSize(); Line 116: Line 117: if (diskImage.getVolumeFormat() == VolumeFormat.COW) { Line 130: } Line 131: Line 132: private double getTotalSizeForClonedDisks(Collection<DiskImage> diskImages) { Line 133: double totalSizeForDisks = 0.0; Line 134: if (null != diskImages) { please put the null on the right hand side of the expression, as per the project conventions. Line 135: for (DiskImage diskImage : diskImages) { Line 136: double diskCapacity = diskImage.getSize(); Line 137: double sizeForDisk = diskCapacity; Line 138: -- 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 <vvola...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
