Allon Mureinik has posted comments on this change.

Change subject: core: Add space validation for AddVmFromSnapshot
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

basically looks OK, but you have debug prints in the code.

http://gerrit.ovirt.org/#/c/25773/1//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: core: Add space validation for AddVmFromSnapshot
Line 8: 
Line 9: AddVmFromSnapshotCommand didn't have storage space validation in its
Line 10: CDA. This patch adds validateSpaceRequirements() to the CDA in the
s/CDA/canDoAction()/
Line 11: parent class and a test for the above method.
Line 12: 
Line 13: Change-Id: Ic0251766df9fdfad7d8dc77bcadc7f2b8b686772
Line 14: Bug-Url: https://bugzilla.redhat.com/917682


http://gerrit.ovirt.org/#/c/25773/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java:

Line 155:                diskImage.getSnapshots().addAll(snapshots);
Line 156: 
Line 157:            }
Line 158:            if 
(!validate(storageDomainValidator.hasSpaceForClonedDisks(disksList))) {
Line 159:                
System.out.println("storageDomainValidator.hasSpaceForClonedDisks(disksList)" + 
storageDomainValidator.hasSpaceForClonedDisks(disksList) + 
disksList.toString());
System.out.println?
Line 160:                return false;
Line 161:            }
Line 162:        }
Line 163:         return true;


Line 163:         return true;
Line 164:     }
Line 165: 
Line 166:     protected List<DiskImage> getAllImageSnapshots(DiskImage 
diskImage) {
Line 167:         System.out.println("in actual getAllImageSnapshots");
same
Line 168:         return 
ImagesHandler.getAllImageSnapshots(diskImage.getImageId(), 
diskImage.getImageTemplateId());
Line 169:     }
Line 170: 
Line 171:     protected DiskImage cloneDiskImage(Guid newVmId,


http://gerrit.ovirt.org/#/c/25773/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java:

Line 37:     private static final Guid STORAGE_DOMAIN_ID_1 = Guid.newGuid();
Line 38:     private static final Guid STORAGE_DOMAIN_ID_2 = Guid.newGuid();
Line 39: 
Line 40:     private static final int NUM_OF_DISKS_1 = 3;
Line 41:     private static final int NUM_OF_DISKS_2 = 3;
perhaps call it "NUM_DISKS_STORAGE_DOMAIN_1"?
Line 42: 
Line 43:     @Mock
Line 44:     private DiskImageDAO diskImageDao;
Line 45: 


Line 99:         command.storageToDisksMap.put(STORAGE_DOMAIN_ID_1, 
generateDisksList(NUM_OF_DISKS_1));
Line 100:         command.storageToDisksMap.put(STORAGE_DOMAIN_ID_2, 
generateDisksList(NUM_OF_DISKS_2));
Line 101:     }
Line 102: 
Line 103:     private List<DiskImage> generateDisksList(int size) {
should be static
Line 104:         List<DiskImage> disksList = new ArrayList<>();
Line 105:         for (int i = 0; i < size; ++i) {
Line 106:             DiskImage diskImage = new DiskImage();
Line 107:             diskImage.setImageId(Guid.newGuid());


-- 
To view, visit http://gerrit.ovirt.org/25773
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0251766df9fdfad7d8dc77bcadc7f2b8b686772
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[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

Reply via email to