Arik Hadas has uploaded a new change for review. Change subject: core: some cleanup in ImportVmCommand ......................................................................
core: some cleanup in ImportVmCommand - Remove getVmId method as it doesn't have to be overridden in addition to getVm() override - Replace AddCanDoActionMessage calls and 'return false' right after them with calls to failCanDoAction - Refactor templateExistsOnExportDomain method to reduce the number of nested levels in it - Refactor updateSnapshotsFromExport method to reduce the number of nested levels in it - Simplified canAddVm method by removing the usage of exists field Change-Id: Ibfdb7bba0758cfccb55bc237f22659cb1993cc00 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, 31 insertions(+), 50 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/96/16296/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 76e3d67..4a9b936 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 @@ -132,14 +132,6 @@ } @Override - public Guid getVmId() { - if (getParameters().isImportAsNewEntity()) { - return getParameters().getVm().getId(); - } - return super.getVmId(); - } - - @Override public VM getVm() { if (getParameters().isImportAsNewEntity()) { return getParameters().getVm(); @@ -291,11 +283,7 @@ (getParameters().getStoragePoolId(), getParameters().getSourceDomainId()); VdcQueryReturnValue qRetVal = getBackend().runInternalQuery(VdcQueryType.GetVmsFromExportDomain, p); - if (!qRetVal.getSucceeded()) { - return null; - } - - return (List<VM>) qRetVal.getReturnValue(); + return qRetVal.getSucceeded() ? (List<VM>) qRetVal.getReturnValue() : null; } private boolean validateImageConfig(List<String> canDoActionMessages, @@ -486,8 +474,7 @@ for (DiskImage diskImage : images) { if (diskImage.getDiskInterface() == DiskInterface.VirtIO_SCSI && !FeatureSupported.virtIoScsi(getVdsGroup().getcompatibility_version())) { - addCanDoActionMessage(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); - return false; + return failCanDoAction(VdcBllMessages.VIRTIO_SCSI_INTERFACE_IS_NOT_AVAILABLE_FOR_CLUSTER_LEVEL); } } @@ -531,28 +518,25 @@ } private boolean templateExistsOnExportDomain() { - boolean retVal = false; - if (!VmTemplateHandler.BlankVmTemplateId.equals(getParameters().getVm().getVmtGuid())) { - GetAllFromExportDomainQueryParameters tempVar = new GetAllFromExportDomainQueryParameters(getParameters() - .getStoragePoolId(), getParameters().getSourceDomainId()); - VdcQueryReturnValue qretVal = Backend.getInstance().runInternalQuery( - VdcQueryType.GetTemplatesFromExportDomain, tempVar); + if (VmTemplateHandler.BlankVmTemplateId.equals(getParameters().getVm().getVmtGuid())) { + return true; + } - if (qretVal.getSucceeded()) { - Map templates = (Map) qretVal.getReturnValue(); + GetAllFromExportDomainQueryParameters tempVar = new GetAllFromExportDomainQueryParameters(getParameters() + .getStoragePoolId(), getParameters().getSourceDomainId()); + VdcQueryReturnValue qretVal = Backend.getInstance().runInternalQuery( + VdcQueryType.GetTemplatesFromExportDomain, tempVar); - for (Object template : templates.keySet()) { - if (getParameters().getVm().getVmtGuid().equals(((VmTemplate) template).getId())) { - retVal = true; - break; - } + if (qretVal.getSucceeded()) { + Map<VmTemplate, ?> templates = (Map) qretVal.getReturnValue(); + + for (VmTemplate template : templates.keySet()) { + if (getParameters().getVm().getVmtGuid().equals(template.getId())) { + return true; } } - } else { - retVal = true; } - return retVal; - + return false; } protected boolean checkTemplateInStorageDomain() { @@ -571,8 +555,7 @@ }); if (Collections.disjoint(domainsId, imageToDestinationDomainMap.values())) { - retValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN); } } return retValue; @@ -580,8 +563,7 @@ private boolean templateExists() { if (getVmTemplate() == null && !getParameters().getCopyCollapse()) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); - return false; + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); } return true; } @@ -603,8 +585,7 @@ imageGUID)); if (Boolean.FALSE.equals(retValue.getReturnValue())) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST); - return false; + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST); } } return true; @@ -612,12 +593,10 @@ protected boolean canAddVm() { // Checking if a desktop with same name already exists - boolean exists = VmHandler.isVmWithSameNameExistStatic(getVm().getName()); - - if (exists) { - addCanDoActionMessage(VdcBllMessages.VM_CANNOT_IMPORT_VM_NAME_EXISTS); + if (VmHandler.isVmWithSameNameExistStatic(getVm().getName())) { + return failCanDoAction(VdcBllMessages.VM_CANNOT_IMPORT_VM_NAME_EXISTS); } - return !exists; + return true; } @Override @@ -936,13 +915,15 @@ * images), it will be updated. If it doesn't exist, it will be saved. */ private void updateSnapshotsFromExport() { - if (getVm().getSnapshots() != null) { - for (Snapshot snapshot : getVm().getSnapshots()) { - if (getSnapshotDao().exists(getVm().getId(), snapshot.getId())) { - getSnapshotDao().update(snapshot); - } else { - getSnapshotDao().save(snapshot); - } + if (getVm().getSnapshots() == null) { + return; + } + + for (Snapshot snapshot : getVm().getSnapshots()) { + if (getSnapshotDao().exists(getVm().getId(), snapshot.getId())) { + getSnapshotDao().update(snapshot); + } else { + getSnapshotDao().save(snapshot); } } } -- To view, visit http://gerrit.ovirt.org/16296 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibfdb7bba0758cfccb55bc237f22659cb1993cc00 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
