Tal Nisan has posted comments on this change. Change subject: core: Image configuration adjustments for cloning a VM ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/35225/3//COMMIT_MSG Commit Message: Line 6: Line 7: core: Image configuration adjustments for cloning a VM Line 8: Line 9: While cloning a VM, in some cases disk image configurations must be Line 10: applied before executing the command. Specifically when cloning a Change to: Specifically when cloning a VM with a raw sparse disk on NFS domain to a block domain, the raw format needs to be changed to COW since block domains don't support raw + sparse combination Line 11: VM's snapshot with thin NFS, raw disk allocation, to a VM with thin Line 12: block disk allocation- the raw format needs to be changed to COW. Line 13: Line 14: Change-Id: Id924e9beab5d84277fad4f3a2a40b14571beaf22 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 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() && > let's export this part to a method so that this if clause will be more read or getDestintationDomainFromDisk().isBlockDomain() Sounds better to me Line 104: diskImage.getVolumeType().equals(VolumeType.Sparse) && Line 105: diskImage.getVolumeFormat().equals(VolumeFormat.RAW)) { Line 106: diskImage.setvolumeFormat(VolumeFormat.COW); Line 107: } 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) && > I'd switch the order so that this check will be first Why does it matter? Line 105: diskImage.getVolumeFormat().equals(VolumeFormat.RAW)) { Line 106: diskImage.setvolumeFormat(VolumeFormat.COW); Line 107: } Line 108: } 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)) { > I'd remove the last check and just set it You can basically cause the result will be the same if it's RAW it'll be changed to COW, if it's COW it'll just remain cow, but I think it's more clear that way that we are searching for RAW+Sparse 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
