Alissa Bonas has posted comments on this change.
Change subject: core: Unfilter ImagesHandler.PerformImagesChecks
......................................................................
Patch Set 6: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
Line 436: boolean checkImagesLocked,
Line 437: boolean checkImagesIllegal,
Line 438: boolean checkImagesExist,
Line 439: boolean checkIsValid,
Line 440: List<DiskImage> diskImageList) {
Isn't it better to use a Set here? Or the disks can repeat themselves?
Line 441:
Line 442: boolean returnValue = true;
Line 443: if (checkImagesLocked) {
Line 444: returnValue = checkImagesLocked(messages, diskImageList);
Line 445: }
Line 446:
Line 447: if (returnValue && checkIsValid) {
Line 448: if (diskImageList.size() > 0) {
Line 449: returnValue = returnValue &&
there's no need here to do returnValue&& ... If statement was entered, thus it
means returnValue is true. (if you are going to touch this code again consider
changing it, I know it was not part of your change)
Line 450: checkDiskImages(messages,
Line 451: storagePoolId,
Line 452: checkImagesIllegal,
Line 453: checkImagesExist,
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
Line 78: true,
Line 79: true,
Line 80: true,
Line 81: true,
Line 82:
ImagesHandler.filterImageDisks(diskImages, true, false));
I see that filtering is done here twice - here and couple of rows above. Is it
correct? Isn't it better to do the filtering once?
Line 83:
Line 84: ensureDomainMap(diskImages,
getParameters().getStorageDomainId());
Line 85: for(DiskImage disk : diskImages) {
Line 86: imageFromSourceDomainMap.put(disk.getId(), disk);
--
To view, visit http://gerrit.ovirt.org/12321
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc93d1bb72daed053f0f37ae3c3f12efef915669
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
