Arik Hadas has uploaded a new change for review. Change subject: core: proper error on trying to import non existing vm ......................................................................
core: proper error on trying to import non existing vm If one tried to import a VM that does not exist in the export domain, he didn't get a proper error message. This patch fixes it by raising a proper error in that case. Change-Id: I02bbca136b9c3244fbe7471ac07b6409196aedc0 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java 1 file changed, 60 insertions(+), 57 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/13/40313/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index 3d9c6e2..0791732 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -10,6 +10,7 @@ import java.util.List; import java.util.Map; import java.util.Set; + import javax.annotation.PostConstruct; import org.apache.commons.lang.NotImplementedException; @@ -303,65 +304,56 @@ } } - List<VM> vms = getVmsFromExportDomain(); - if (vms == null) { - return false; + VM vm = getVmFromExportDomain(getParameters().getVmId()); + if (vm == null) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND_ON_EXPORT_DOMAIN); } - VM vm = LinqUtils.firstOrNull(vms, new Predicate<VM>() { - @Override - public boolean eval(VM evalVm) { - return evalVm.getId().equals(getParameters().getVmId()); + // At this point we should work with the VM that was read from + // the OVF because the VM from the parameters may lack images + setVm(vm); + + // Iterate over all the VM images (active image and snapshots) + for (DiskImage image : getImages()) { + if (Guid.Empty.equals(image.getVmSnapshotId())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID); } - }); - if (vm != null) { - // At this point we should work with the VM that was read from - // the OVF - setVm(vm); - - // Iterate over all the VM images (active image and snapshots) - for (DiskImage image : getImages()) { - if (Guid.Empty.equals(image.getVmSnapshotId())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID); - } - - if (getParameters().getCopyCollapse()) { - // If copy collapse sent then iterate over the images got from the parameters, until we got - // a match with the image from the VM. - for (DiskImage p : imageList) { - // copy the new disk volume format/type if provided, - // only if requested by the user - if (p.getImageId().equals(image.getImageId())) { - if (p.getVolumeFormat() != null) { - image.setvolumeFormat(p.getVolumeFormat()); - } - if (p.getVolumeType() != null) { - image.setVolumeType(p.getVolumeType()); - } - // Validate the configuration of the image got from the parameters. - if (!validateImageConfig(canDoActionMessages, domainsMap, image)) { - return false; - } - break; + if (getParameters().getCopyCollapse()) { + // If copy collapse sent then iterate over the images got from the parameters, until we got + // a match with the image from the VM. + for (DiskImage p : imageList) { + // copy the new disk volume format/type if provided, + // only if requested by the user + if (p.getImageId().equals(image.getImageId())) { + if (p.getVolumeFormat() != null) { + image.setvolumeFormat(p.getVolumeFormat()); } + if (p.getVolumeType() != null) { + image.setVolumeType(p.getVolumeType()); + } + // Validate the configuration of the image got from the parameters. + if (!validateImageConfig(canDoActionMessages, domainsMap, image)) { + return false; + } + break; } } - - image.setStoragePoolId(getParameters().getStoragePoolId()); - // we put the source domain id in order that copy will - // work properly. - // we fix it to DestDomainId in - // MoveOrCopyAllImageGroups(); - image.setStorageIds(new ArrayList<Guid>(Arrays.asList(getSourceDomainId(image)))); } - Map<Guid, List<DiskImage>> images = ImagesHandler.getImagesLeaf(getImages()); - for (Map.Entry<Guid, List<DiskImage>> entry : images.entrySet()) { - Guid id = entry.getKey(); - List<DiskImage> diskList = entry.getValue(); - getVm().getDiskMap().put(id, getActiveVolumeDisk(diskList)); - } + image.setStoragePoolId(getParameters().getStoragePoolId()); + // we put the source domain id in order that copy will + // work properly. + // we fix it to DestDomainId in + // MoveOrCopyAllImageGroups(); + image.setStorageIds(new ArrayList<Guid>(Arrays.asList(getSourceDomainId(image)))); + } + + Map<Guid, List<DiskImage>> images = ImagesHandler.getImagesLeaf(getImages()); + for (Map.Entry<Guid, List<DiskImage>> entry : images.entrySet()) { + Guid id = entry.getKey(); + List<DiskImage> diskList = entry.getValue(); + getVm().getDiskMap().put(id, getActiveVolumeDisk(diskList)); } return true; @@ -371,17 +363,28 @@ return diskList.get(diskList.size() - 1); } + protected VM getVmFromExportDomain(Guid vmId) { + for (VM vm : getVmsFromExportDomain()) { + if (vmId.equals(vm.getId())) { + return vm; + } + } + + return null; + } + /** * Load images from Import/Export domain. - * @return A {@link List} of {@link VM}s, or <code>null</code> if the query to the export domain failed. + * @return A {@link List} of {@link VM}s from the export domain. */ + @SuppressWarnings("unchecked") protected List<VM> getVmsFromExportDomain() { - GetAllFromExportDomainQueryParameters p = - new GetAllFromExportDomainQueryParameters - (getParameters().getStoragePoolId(), getParameters().getSourceDomainId()); - VdcQueryReturnValue qRetVal = - runInternalQuery(VdcQueryType.GetVmsFromExportDomain, p); - return qRetVal.getSucceeded() ? qRetVal.<List<VM>>getReturnValue() : null; + VdcQueryReturnValue qRetVal = runInternalQuery( + VdcQueryType.GetVmsFromExportDomain, + new GetAllFromExportDomainQueryParameters( + getParameters().getStoragePoolId(), + getParameters().getSourceDomainId())); + return (List<VM>) (qRetVal.getSucceeded() ? qRetVal.getReturnValue() : Collections.emptyList()); } private boolean validateImageConfig(List<String> canDoActionMessages, -- To view, visit https://gerrit.ovirt.org/40313 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I02bbca136b9c3244fbe7471ac07b6409196aedc0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
