Liron Ar has posted comments on this change. Change subject: core: Use import command to register a VM ......................................................................
Patch Set 15: (5 comments) partial review http://gerrit.ovirt.org/#/c/27247/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java: Line 227: if (!checkStoragePool()) { Line 228: return false; Line 229: } Line 230: Line 231: Set<Guid> destGuids = new HashSet<Guid>(imageToDestinationDomainMap.values()); do we need to check this on the register scenario? Line 232: for (Guid destGuid : destGuids) { Line 233: StorageDomain storageDomain = getStorageDomain(destGuid); Line 234: StorageDomainValidator validator = new StorageDomainValidator(storageDomain); Line 235: if (!validate(validator.isDomainExistAndActive()) || !validate(validator.domainIsValidDestination())) { Line 238: Line 239: domainsMap.put(destGuid, storageDomain); Line 240: } Line 241: Line 242: if (getParameters().isImportAsNewEntity() && !getParameters().getCopyCollapse()) { general - we should prevent copycollapse on the register scenario as it won't work (images are already on that sotrage) Line 243: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_CLONE_NOT_COLLAPSED); Line 244: } Line 245: Line 246: setSourceDomainId(getParameters().getSourceDomainId()); http://gerrit.ovirt.org/#/c/27247/15/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 145: } Line 146: return retValue; Line 147: } Line 148: Line 149: protected boolean isUnregisteredVM() { please rename to "isImagesAlreadyOnTarget()", the current name isn't clear enough imo and it's not 100% correct as it's being used also for templates which might be confusing. Line 150: return getParameters().isImagesExistOnStorageDomain(); Line 151: } Line 152: @Override Line 153: protected void setActionMessageParameters() { Line 147: } Line 148: Line 149: protected boolean isUnregisteredVM() { Line 150: return getParameters().isImagesExistOnStorageDomain(); Line 151: } add a blank line here Line 152: @Override Line 153: protected void setActionMessageParameters() { Line 154: if (getMoveOrCopyImageOperation() == ImageOperation.Move) { Line 155: addCanDoActionMessage(VdcBllMessages.VAR__ACTION__MOVE); http://gerrit.ovirt.org/#/c/27247/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyParameters.java: Line 9: private static final long serialVersionUID = 1051590893103934441L; Line 10: Line 11: private Map<Guid, Guid> imageToDestinationDomainMap; Line 12: private boolean importAsNewEntity; Line 13: private boolean imagesExistOnStorageDomain; /s/imagesExistOnTargetStorageDomain; Line 14: Line 15: public MoveOrCopyParameters(Guid containerId, Guid storageDomainId) { Line 16: super(storageDomainId); Line 17: setContainerId(containerId); -- To view, visit http://gerrit.ovirt.org/27247 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ee64651eb614feb6ac9d7fde88a4ee6348aff06 Gerrit-PatchSet: 15 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
