Liron Ar has posted comments on this change.

Change subject: core: Validate MSD when importing a VM from configuration
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/28538/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java:

Line 132:         return image.getStorageIds().get(0);
Line 133:     }
Line 134: 
Line 135:     protected void ensureDomainMap(Collection<DiskImage> images, Guid 
defaultDomainId) {
Line 136:         super.ensureDomainMap(images, defaultDomainId);
i'd replace the super method call with something else, there's no need for the 
map population that is done there
Line 137:         if (isImagesAlreadyOnTarget()) {
Line 138:             for (DiskImage image : images) {
Line 139:                 imageToDestinationDomainMap.put(image.getId(), 
image.getStorageIds().get(0));
Line 140:             }


Line 135:     protected void ensureDomainMap(Collection<DiskImage> images, Guid 
defaultDomainId) {
Line 136:         super.ensureDomainMap(images, defaultDomainId);
Line 137:         if (isImagesAlreadyOnTarget()) {
Line 138:             for (DiskImage image : images) {
Line 139:                 imageToDestinationDomainMap.put(image.getId(), 
image.getStorageIds().get(0));
replace image.getStorageIds().get.. with getSourceDomainId
Line 140:             }
Line 141:         }
Line 142:     }
Line 143: 


http://gerrit.ovirt.org/#/c/28538/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java:

Line 158:             return false;
Line 159:         }
Line 160: 
Line 161:         for (DiskImage image : getVm().getImages()) {
Line 162:             setStorageDomainId(image.getStorageIds().get(0));
1. can you elaborate why it's not taken from the ovfentitydata anymore?
2. general questions - have you tested the results of it with template that has 
disk copies on multiple domains?
Line 163:             if (!validate(new 
StorageDomainValidator(getStorageDomain()).isDomainExistAndActive())) {
Line 164:                 return false;
Line 165:             }
Line 166:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I067b6186c2a2b7d77394e9cb0a4e8f023c168e4b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[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: [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