Maor Lipchuk has posted comments on this change.

Change subject: core: Add support for copied disk template
......................................................................


Patch Set 9:

(5 comments)

http://gerrit.ovirt.org/#/c/35933/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateFromConfigurationCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateFromConfigurationCommand.java:

Line 113:         setActionReturnValue(getVmTemplate().getId());
Line 114:         setSucceeded(true);
Line 115:     }
Line 116: 
Line 117:     private void insertCopiedImagesToDb() {
> /s/insertCopiedImagesToDb/findAndSaveDiskCopies
done
Line 118:         List<OvfEntityData> ovfEntityDataList =
Line 119:                 
getUnregisteredOVFDataDao().getByEntityIdAndStorageDomain(ovfEntityData.getEntityId(),
 null);
Line 120:         List<image_storage_domain_map> copiedTemplateDisks = new 
ArrayList<image_storage_domain_map>();
Line 121:         for (OvfEntityData ovfEntityDataFetched : ovfEntityDataList) {


Line 116: 
Line 117:     private void insertCopiedImagesToDb() {
Line 118:         List<OvfEntityData> ovfEntityDataList =
Line 119:                 
getUnregisteredOVFDataDao().getByEntityIdAndStorageDomain(ovfEntityData.getEntityId(),
 null);
Line 120:         List<image_storage_domain_map> copiedTemplateDisks = new 
ArrayList<image_storage_domain_map>();
> 1. I'd prefer it to be a linkedList
done

2. I think that the copiedTemplateDisks sound better
Line 121:         for (OvfEntityData ovfEntityDataFetched : ovfEntityDataList) {
Line 122:             
initStorageDomainMapWithCopiedTemplateImages(copiedTemplateDisks,
Line 123:                     getImages(),
Line 124:                     ovfEntityDataFetched.getStorageDomainId());


Line 125:         }
Line 126:         insertNewCopiedDiskImage(copiedTemplateDisks);
Line 127:     }
Line 128: 
Line 129:     private void 
initStorageDomainMapWithCopiedTemplateImages(List<image_storage_domain_map> 
copiedTemplateDisks,
> /s/initStorageDomainMapWithCopiedTemplateImages/findDisksCopies
changed to populateDisksCopies
Line 130:             List<DiskImage> originaTemplatelmages,
Line 131:             Guid storageDomainId) {
Line 132:         List<Guid> imagesContainedInStorageDomain = 
getImagesGuidFromStorage(storageDomainId, getStoragePoolId());
Line 133:         for (DiskImage templateDiskImage : getImages()) {


Line 130:             List<DiskImage> originaTemplatelmages,
Line 131:             Guid storageDomainId) {
Line 132:         List<Guid> imagesContainedInStorageDomain = 
getImagesGuidFromStorage(storageDomainId, getStoragePoolId());
Line 133:         for (DiskImage templateDiskImage : getImages()) {
Line 134:             if 
(storageDomainId.equals(templateDiskImage.getStorageIds().get(0))) {
> lets add a comment here explaining why we continue on that scenario
done
Line 135:                 continue;
Line 136:             }
Line 137:             if 
(imagesContainedInStorageDomain.contains(templateDiskImage.getId())) {
Line 138:                 log.info("Found a copied image of '{}' on Storage 
Domain id '{}'",


Line 166:         }
Line 167:         return imagesList;
Line 168:     }
Line 169: 
Line 170:     private void insertNewCopiedDiskImage(final 
List<image_storage_domain_map> copiedTemplateDisks) {
> /s/insert/SaveImageStorageDomainMapList
done
Line 171:         TransactionSupport.executeInNewTransaction(new 
TransactionMethod<Void>() {
Line 172:             @Override
Line 173:             public Void runInTransaction() {
Line 174:                 for (image_storage_domain_map imageStorageDomainMap : 
copiedTemplateDisks) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7514146d26792a361e47d1de820f5233ff9bf40
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to