Amit Aviram 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);
> I don't think that this call should be 'hidden' here, it will make debuggin
Practically you right, logically and from code design point of view I think 
that here is the best place to fit this part- if you want to get the disks 
configured for the snapshot cloning- this function will adjust the result for 
you to get what you've asked.. 
otherwise you change the command in the canDoAction which for me looks a bit 
not related to the context of the method..
Line 96:         }
Line 97:         return diskImagesFromConfiguration;
Line 98:     }
Line 99: 


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) &&
Line 105:                     
diskImage.getVolumeFormat().equals(VolumeFormat.RAW)) {
> You can basically cause the result will be the same if it's RAW it'll be ch
So I'll keep it for now
Line 106:                 diskImage.setvolumeFormat(VolumeFormat.COW);
Line 107:             }
Line 108:         }
Line 109:     }


-- 
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