Liron Ar has uploaded a new change for review. Change subject: core: handle 'source' domains when adding a vm from template ......................................................................
core: handle 'source' domains when adding a vm from template When adding a vm from template we need to make sure to choose an Active domain on which the template disks stored on for the operation- 1. If there's no such domain, a CDA message should be displayed. 2. If the source disk is stored on few domains, the engine will attempt to select the same domain as the target domain for possibly faster copy. 3. For better user experience, at first we inspect if all the source disks have a copy on Active domain, later on the destination domains are being verified and then the source domains will be selected as explained in (2). Change-Id: I8d02cfded41a3588dc944d9dfcd5a3eec88c45ab Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1037439 Signed-off-by: Liron Aravot <lara...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 9 files changed, 144 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/26262/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 99ad2a0..b37ba9a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -22,8 +22,8 @@ import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; -import org.ovirt.engine.core.bll.validator.VmWatchdogValidator; import org.ovirt.engine.core.bll.validator.VmValidationUtils; +import org.ovirt.engine.core.bll.validator.VmWatchdogValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; @@ -99,8 +99,9 @@ protected Guid imageTypeId; protected ImageType imageType; private Guid vmInterfacesSourceId; - private VmTemplate vmDisksSource; + protected VmTemplate vmDisksSource; private Guid vmDevicesSourceId; + private List<StorageDomain> poolDomains; private Map<Guid, Guid> srcDiskIdToTargetDiskIdMapping = new HashMap<>(); private Map<Guid, Guid> srcVmNicIdToTargetVmNicIdMapping = new HashMap<>(); @@ -418,9 +419,12 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTER_UNDEFINED_ARCHITECTURE); } - if (!buildAndCheckDestStorageDomains()) { + if (!verifySourceDomains() || !buildAndCheckDestStorageDomains()) { return false; } + + chooseDisksSourceDomains(); + // otherwise.. storageToDisksMap = ImagesHandler.buildStorageToDiskMap(getImagesToCheckDestinationStorageDomains(), @@ -575,6 +579,12 @@ return retValue && validateIsImagesOnDomains(); } + protected boolean verifySourceDomains() { + return true; + } + + protected void chooseDisksSourceDomains() {} + protected Collection<DiskImage> getImagesToCheckDestinationStorageDomains() { return vmDisksSource.getDiskTemplateMap().values(); } @@ -613,9 +623,16 @@ return true; } + protected List<StorageDomain> getPoolDomains() { + if (poolDomains == null) { + poolDomains = getStorageDomainDAO().getAllForStoragePool(vmDisksSource.getStoragePoolId()); + } + return poolDomains; + } + protected void fillImagesMapBasedOnTemplate() { ImagesHandler.fillImagesMapBasedOnTemplate(vmDisksSource, - getStorageDomainDAO().getAllForStoragePool(vmDisksSource.getStoragePoolId()), + getPoolDomains(), diskInfoDestinationMap, destStorages, false); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java index 46f3234..a41cf0c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java @@ -1,26 +1,37 @@ package org.ovirt.engine.core.bll; +import java.util.EnumSet; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; import org.ovirt.engine.core.bll.job.ExecutionHandler; +import org.ovirt.engine.core.bll.validator.DiskImagesValidator; +import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.AddVmFromTemplateParameters; import org.ovirt.engine.core.common.action.CreateCloneOfTemplateParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskImageBase; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.utils.BusniessEntityHelper; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; -import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; @LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class AddVmFromTemplateCommand<T extends AddVmFromTemplateParameters> extends AddVmCommand<T> { + private Map<Guid, Guid> diskInfoSourceMap; + private Map<Guid, Set<Guid>> validDisksDomains; public AddVmFromTemplateCommand(T parameters) { super(parameters); @@ -100,8 +111,8 @@ DiskImageBase diskInfo = getParameters().getDiskInfoDestinationMap().get(disk.getId()); CreateCloneOfTemplateParameters params = new CreateCloneOfTemplateParameters(disk.getImageId(), getParameters().getVmStaticData().getId(), diskInfo); - params.setStorageDomainId(disk.getStorageIds().get(0)); - params.setDestStorageDomainId(diskInfoDestinationMap.get(disk.getId()).getStorageIds().get(0)); + params.setStorageDomainId(diskInfoSourceMap.get(disk.getId())); + params.setDestStorageDomainId(retrieveDestinationDomainForDisk(disk.getId())); params.setDiskAlias(diskInfoDestinationMap.get(disk.getId()).getDiskAlias()); params.setVmSnapshotId(getVmSnapshotId()); params.setParentCommand(VdcActionType.AddVmFromTemplate); @@ -145,6 +156,40 @@ } @Override + protected boolean verifySourceDomains() { + Pair<ValidationResult, Map<Guid, Set<Guid>>> result = + new DiskImagesValidator(getImagesToCheckDestinationStorageDomains()).diskImagesOnAnyApplicableDomains( + BusniessEntityHelper.buildIdMap(getPoolDomains()), + EnumSet.of(StorageDomainStatus.Active), + VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN); + + validDisksDomains = result.getSecond(); + return validate(result.getFirst()); + } + + @Override + protected void chooseDisksSourceDomains() { + diskInfoSourceMap = new HashMap<>(); + for (DiskImage disk : getImagesToCheckDestinationStorageDomains()) { + Guid diskId = disk.getId(); + Set<Guid> validDomainsForDisk = validDisksDomains.get(diskId); + Guid destinationDomain = retrieveDestinationDomainForDisk(diskId); + // if the destination domain is one of the valid source domains, we can + // choose the same domain as the source domain for + // possibly faster operation, otherwise we'll choose random valid domain as the source. + if (validDomainsForDisk.contains(destinationDomain)) { + diskInfoSourceMap.put(diskId, destinationDomain); + } else { + diskInfoSourceMap.put(diskId, validDomainsForDisk.iterator().next()); + } + } + } + + private Guid retrieveDestinationDomainForDisk(Guid id) { + return diskInfoDestinationMap.get(id).getStorageIds().get(0); + } + + @Override protected boolean isVirtioScsiEnabled() { return getParameters().isVirtioScsiEnabled() != null ? super.isVirtioScsiEnabled() : isVirtioScsiControllerAttached(getVmTemplateId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java index fd55aca..0da85a2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java @@ -53,7 +53,7 @@ Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.AddVmFromTemplate); try { vdsReturnValue = runVdsCommand(VDSCommandType.CopyImage, - new CopyImageVDSCommandParameters(storagePoolID, getDiskImage().getStorageIds().get(0), + new CopyImageVDSCommandParameters(storagePoolID, getParameters().getStorageDomainId(), getVmTemplateId(), getDiskImage().getId(), getImage().getImageId(), mNewCreatedDiskImage.getId(), getDestinationImageId(), "", getDestinationStorageDomainId(), CopyVolumeType.LeafVol, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java index 7f823b9..047a7d9 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java @@ -1,23 +1,30 @@ package org.ovirt.engine.core.bll.validator; import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.Snapshot; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.DiskImageDAO; import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.VmDAO; import org.ovirt.engine.core.dao.VmDeviceDAO; +import org.ovirt.engine.core.utils.collections.MultiValueMapUtils; /** * A validator for the {@link DiskImage} class. Since most usecases require validations of multiple {@link DiskImage}s @@ -181,6 +188,58 @@ return ValidationResult.VALID; } + /** + * checks that the given disks has no derived disks on the given storage domain. if the provided storage domain id + * is null, it will be checked that there are no derived disks on any storage domain. + */ + public Pair<ValidationResult, Map<Guid, Set<Guid>>> diskImagesOnAnyApplicableDomains(Map<Guid, StorageDomain> storageDomains, + Set<StorageDomainStatus> applicableStatuses, + VdcBllMessages message) { + + List<String> disksInfo = new LinkedList<>(); + Map<Guid, Set<Guid>> legitDomainsForDisks = new HashMap<>(); + + for (DiskImage diskImage : diskImages) { + List<String> nonApplicableStorageInfo = new LinkedList<>(); + + for (Guid storageDomainID : diskImage.getStorageIds()) { + StorageDomain domain = storageDomains.get(storageDomainID); + + if (domain == null || domain.getStatus() == null) { + throw new RuntimeException("Wrong data provided to validator"); + } + + if (applicableStatuses.contains(domain.getStatus())) { + MultiValueMapUtils.addToMap(diskImage.getId(), + domain.getId(), + legitDomainsForDisks, + new MultiValueMapUtils.SetCreator<Guid>()); + } else { + nonApplicableStorageInfo.add(String.format("%s - %s", domain.getName(), domain.getStatus() + .toString())); + } + } + + if (diskImage.getStorageIds().size() == nonApplicableStorageInfo.size()) { + disksInfo.add(String.format("%s (%s) %n", + diskImage.getDiskAlias(), + StringUtils.join(nonApplicableStorageInfo, " / "))); + } + } + + ValidationResult result = ValidationResult.VALID; + + if (!disksInfo.isEmpty()) { + result = + new ValidationResult(message, + String.format("$disksInfo %s", + String.format(StringUtils.join(disksInfo, "%n"))), + String.format("$applicableStatus %s", StringUtils.join(applicableStatuses, ","))); + } + + return new Pair<>(result, legitDomainsForDisks); + } + public ValidationResult disksInStatus(ImageStatus applicableStatus) { for (DiskImage diskImage : diskImages) { if (diskImage.getImageStatus() != applicableStatus) { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index a41ed52..d9dbb34 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -642,6 +642,7 @@ ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DESTINATION_AND_SOURCE_STORAGE_SUB_TYPES_DIFFERENT(ErrorType.CONFLICT), ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED(ErrorType.CONFLICT), + ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_IS_BEING_MIGRATED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 3986aa2..a5a913d 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -652,6 +652,9 @@ ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being moved or copied. ACTION_TYPE_FAILED_DESTINATION_AND_SOURCE_STORAGE_SUB_TYPES_DIFFERENT=Cannot ${action} ${type}. Source and target domains must both be either file domains or block domains. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. +ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN=Cannot ${action} ${type}. Can't find Domain(s) in ${applicableStatus} status for all of the Template disks.\n\ +Please make sure that there is at least one Storage Domain in applicable status for each one of the disks :\n\ +${disksInfo}. ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_VM_IS_BEING_MIGRATED=Cannot ${action} ${type}. VM ${VmName} is being migrated. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index b35b367..4aaa479 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -1789,6 +1789,11 @@ @DefaultStringValue("Cannot ${action} ${type}. Template ${TemplateName} is being exported.") String ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED(); + @DefaultStringValue("ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN=Cannot ${action} ${type}. Can't find Domain(s) in ${applicableStatus} status for all of the Template disks.\n" + + "Please make sure that there is at least one Storage Domain in applicable status for each one of the disks :\n" + + "${disksInfo}") + String ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN(); + @DefaultStringValue("Cannot ${action} ${type}. VM ${VmName} is being imported.") String ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 59a60bb..a55b286 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -632,6 +632,9 @@ ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being moved or copied. ACTION_TYPE_FAILED_DESTINATION_AND_SOURCE_STORAGE_SUB_TYPES_DIFFERENT=Cannot ${action} ${type}. Source and target domains must both be either file domains or block domains. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. +ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN=Cannot ${action} ${type}. Can't find Domain(s) in ${applicableStatus} status for all of the Template disks.\n\ +Please make sure that there is at least one Storage Domain in applicable status for each one of the disks :\n\ +${disksInfo} ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_VM_IS_BEING_MIGRATED=Cannot ${action} ${type}. VM ${VmName} is being migrated. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 2ab4611..df45cda 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -653,6 +653,9 @@ ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being moved or copied. ACTION_TYPE_FAILED_DESTINATION_AND_SOURCE_STORAGE_SUB_TYPES_DIFFERENT=Cannot ${action} ${type}. Source and target domains must both be either file domains or block domains. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. +ACTION_TYPE_FAILED_TEMPLATE_DISKS_NO_FIT_DOMAIN=Cannot ${action} ${type}. Can't find Domain(s) in ${applicableStatus} status for all of the Template disks.\n\ +Please make sure that there is at least one Storage Domain in applicable status for each one of the disks :\n\ +${disksInfo} ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_VM_IS_BEING_MIGRATED=Cannot ${action} ${type}. VM ${VmName} is being migrated. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. -- To view, visit http://gerrit.ovirt.org/26262 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8d02cfded41a3588dc944d9dfcd5a3eec88c45ab Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
