Liron Aravot has posted comments on this change.

Change subject: core: Image configuration adjustments for cloning a VM
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.ovirt.org/#/c/35225/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java:

Line 91:                     
ImagesHandler.filterImageDisks(vmFromConfiguration.getDiskMap().values(),
Line 92:                             false,
Line 93:                             true,
Line 94:                             true);
Line 95:             adjustDisksImageConfiguration(diskImagesFromConfiguration);
> Practically you right, logically and from code design point of view I think
you are right, but currently that kind of operations are being done in the CDA. 
having it here makes it different from other flows in the system - i think that 
keeping the current "way" we do it is important, because otherwise it might 
confuse developer that'll later on will try to change something here.
Line 96:         }
Line 97:         return diskImagesFromConfiguration;
Line 98:     }
Line 99: 


Line 100:     private void adjustDisksImageConfiguration(Collection<DiskImage> 
diskImages) {
Line 101:         for (DiskImage diskImage : diskImages) {
Line 102:             // Adjust disk image configuration if needed.
Line 103:             if 
(destStorages.get(diskInfoDestinationMap.get(diskImage.getId()).getStorageIds().get(0)).getStorageStaticData().getStorageType().isBlockDomain()
 &&
Line 104:                     
diskImage.getVolumeType().equals(VolumeType.Sparse) &&
> Why does it matter?
I didn't say that it matter.
Line 105:                     
diskImage.getVolumeFormat().equals(VolumeFormat.RAW)) {
Line 106:                 diskImage.setvolumeFormat(VolumeFormat.COW);
Line 107:             }
Line 108:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id924e9beab5d84277fad4f3a2a40b14571beaf22
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Amit Aviram <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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