Gilad Chaplik has uploaded a new change for review. Change subject: core: RunVmCommand.canDoAction clean-up (6) ......................................................................
core: RunVmCommand.canDoAction clean-up (6) Check for vm images. Change-Id: I297c9afba655c38ffdd870f61b1967732a712912 Signed-off-by: Gilad Chaplik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java 5 files changed, 164 insertions(+), 119 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/13402/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index 02a8bb9..629e86a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -19,6 +19,7 @@ import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaVdsDependent; import org.ovirt.engine.core.bll.quota.QuotaVdsGroupConsumptionParameter; +import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.RunVmValidator; @@ -688,6 +689,14 @@ vmDisks)) && validate(getVmValidator(vm).vmNotLocked()) && validate(getSnapshotsValidator().vmNotDuringSnapshot(vm.getId())) && + getRunVmValidator().validateImagesForRunVm(vm, + vmDisks, + isInternalExecution(), + messages) && + validate(new StoragePoolValidator(getStoragePoolDAO().get(vm.getStoragePoolId())).isUp()) && + validate(getRunVmValidator().validateIsoPath(vm.isAutoStartup(), vm.getStoragePoolId(), + getParameters().getDiskPath(), + getParameters().getFloppyPath())) && canRunVm(vm) && validateNetworkInterfaces(); if (!canDoAction) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java index feabbec..4206664 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java @@ -270,7 +270,11 @@ return getRunVmValidator().canRunVm(vm, messages, getDiskDao().getAllForVm(vm.getId(), true), - runVmParams.getBootSequence()) + runVmParams.getBootSequence(), + false, + fetchStoragePool(vm.getStoragePoolId()), + runVmParams.getDiskPath(), + runVmParams.getFloppyPath()) && VmRunHandler.getInstance().canRunVm(vm, messages, @@ -283,6 +287,10 @@ return DbFacade.getInstance().getDiskDao(); } + private static storage_pool fetchStoragePool(Guid storagePoolId) { + return DbFacade.getInstance().getStoragePoolDao().get(storagePoolId); + } + private static RunVmValidator getRunVmValidator() { return new RunVmValidator(); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java index b80dc31..24cc402 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java @@ -7,18 +7,14 @@ import java.util.Map; import java.util.Map.Entry; -import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; -import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.common.VdcActionUtils; import org.ovirt.engine.core.common.action.RunVmParams; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.ImageType; -import org.ovirt.engine.core.common.businessentities.RepoFileMetaData; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; @@ -26,12 +22,8 @@ import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; -import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; -import org.ovirt.engine.core.common.queries.GetImagesListParameters; -import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; -import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.common.vdscommands.IsVmDuringInitiatingVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; @@ -65,61 +57,40 @@ boolean retValue = true; List<Disk> vmDisks = getDiskDao().getAllForVm(vm.getId(), true); - List<DiskImage> vmImages = ImagesHandler.filterImageDisks(vmDisks, true, false); - if (retValue && !vmImages.isEmpty()) { - storage_pool sp = getStoragePoolDAO().get(vm.getStoragePoolId()); - ValidationResult spUpResult = new StoragePoolValidator(sp).isUp(); - if (!spUpResult.isValid()) { - message.add(spUpResult.getMessage().name()); + if (retValue) { + boolean isVmDuringInit = ((Boolean) getBackend() + .getResourceManager() + .RunVdsCommand(VDSCommandType.IsVmDuringInitiating, + new IsVmDuringInitiatingVDSCommandParameters(vm.getId())) + .getReturnValue()).booleanValue(); + if (vm.isRunning() || (vm.getStatus() == VMStatus.NotResponding) || isVmDuringInit) { retValue = false; + message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString()); + } else if (vm.getStatus() == VMStatus.Paused && vm.getRunOnVds() != null) { + VDS vds = DbFacade.getInstance().getVdsDao().get( + new Guid(vm.getRunOnVds().toString())); + if (vds.getStatus() != VDSStatus.Up) { + retValue = false; + message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString()); + message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString()); + } } - if (retValue) { - retValue = performImageChecksForRunningVm(vm, message, runParams, vmImages); + boolean isStatelessVm = shouldVmRunAsStateless(runParams, vm); + + if (retValue && isStatelessVm && !snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) { + retValue = false; + message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString()); } - // Check if iso and floppy path exists - if (retValue && !vm.isAutoStartup() - && !validateIsoPath(getIsoDomainListSyncronizer() - .findActiveISODomain(vm.getStoragePoolId()), - runParams, - message)) { + // if the VM itself is stateless or run once as stateless + if (retValue && isStatelessVm && vm.isAutoStartup()) { retValue = false; - } else if (retValue) { - boolean isVmDuringInit = ((Boolean) getBackend() - .getResourceManager() - .RunVdsCommand(VDSCommandType.IsVmDuringInitiating, - new IsVmDuringInitiatingVDSCommandParameters(vm.getId())) - .getReturnValue()).booleanValue(); - if (vm.isRunning() || (vm.getStatus() == VMStatus.NotResponding) || isVmDuringInit) { - retValue = false; - message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString()); - } else if (vm.getStatus() == VMStatus.Paused && vm.getRunOnVds() != null) { - VDS vds = DbFacade.getInstance().getVdsDao().get( - new Guid(vm.getRunOnVds().toString())); - if (vds.getStatus() != VDSStatus.Up) { - retValue = false; - message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString()); - message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString()); - } - } + message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString()); + } - boolean isStatelessVm = shouldVmRunAsStateless(runParams, vm); - - if (retValue && isStatelessVm && !snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) { - retValue = false; - message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString()); - } - - // if the VM itself is stateless or run once as stateless - if (retValue && isStatelessVm && vm.isAutoStartup()) { - retValue = false; - message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString()); - } - - if (retValue && isStatelessVm && !hasSpaceForSnapshots(vm, message)) { - return false; - } + if (retValue && isStatelessVm && !hasSpaceForSnapshots(vm, message)) { + return false; } } @@ -226,62 +197,6 @@ } } return isoGuid; - } - - @SuppressWarnings("unchecked") - private boolean validateIsoPath(Guid storageDomainId, - RunVmParams runParams, - ArrayList<String> messages) { - if (!StringUtils.isEmpty(runParams.getDiskPath())) { - if (storageDomainId == null) { - messages.add(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO.toString()); - return false; - } - boolean retValForIso = false; - VdcQueryReturnValue ret = - getBackend().runInternalQuery(VdcQueryType.GetImagesList, - new GetImagesListParameters(storageDomainId, ImageType.ISO)); - if (ret != null && ret.getReturnValue() != null && ret.getSucceeded()) { - List<RepoFileMetaData> repoFileNameList = (List<RepoFileMetaData>) ret.getReturnValue(); - if (repoFileNameList != null) { - for (RepoFileMetaData isoFileMetaData : (List<RepoFileMetaData>) ret.getReturnValue()) { - if (isoFileMetaData.getRepoFileName().equals(runParams.getDiskPath())) { - retValForIso = true; - break; - } - } - } - } - if (!retValForIso) { - messages.add(VdcBllMessages.ERROR_CANNOT_FIND_ISO_IMAGE_PATH.toString()); - return false; - } - } - - if (!StringUtils.isEmpty(runParams.getFloppyPath())) { - boolean retValForFloppy = false; - VdcQueryReturnValue ret = - getBackend().runInternalQuery(VdcQueryType.GetImagesList, - new GetImagesListParameters(storageDomainId, ImageType.Floppy)); - if (ret != null && ret.getReturnValue() != null && ret.getSucceeded()) { - List<RepoFileMetaData> repoFileNameList = (List<RepoFileMetaData>) ret.getReturnValue(); - if (repoFileNameList != null) { - - for (RepoFileMetaData isoFileMetaData : (List<RepoFileMetaData>) ret.getReturnValue()) { - if (isoFileMetaData.getRepoFileName().equals(runParams.getFloppyPath())) { - retValForFloppy = true; - break; - } - } - } - } - if (!retValForFloppy) { - messages.add(VdcBllMessages.ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH.toString()); - return false; - } - } - - return true; } public boolean shouldVmRunAsStateless(RunVmParams param, VM vm) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java index dd73d27..541abe0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java @@ -2,13 +2,25 @@ import java.util.List; +import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.bll.Backend; +import org.ovirt.engine.core.bll.ImagesHandler; import org.ovirt.engine.core.bll.IsoDomainListSyncronizer; import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.VmHandler; +import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.common.businessentities.BootSequence; import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageType; +import org.ovirt.engine.core.common.businessentities.RepoFileMetaData; import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.businessentities.storage_pool; +import org.ovirt.engine.core.common.queries.GetImagesListParameters; +import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; +import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; @@ -61,6 +73,81 @@ } + public boolean validateImagesForRunVm(VM vm, + List<Disk> vmDisks, + boolean isInternal, + List<String> messages) { + List<DiskImage> images = ImagesHandler.filterImageDisks(vmDisks, true, false); + if (!ImagesHandler.PerformImagesChecks(messages, + vm.getStoragePoolId(), Guid.Empty, !vm.isAutoStartup(), + true, false, false, + !vm.isAutoStartup() || !isInternal, + !vm.isAutoStartup() || !isInternal, + images)) { + return false; + } + return true; + } + + public ValidationResult validateIsoPath(boolean vmHA, Guid storageDomainId, + String diskPath, + String floppyPath) { + if (vmHA) { + return ValidationResult.VALID; + } + if (!StringUtils.isEmpty(diskPath)) { + if (storageDomainId == null) { + return new ValidationResult(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO); + } + boolean retValForIso = false; + VdcQueryReturnValue ret = + getBackend().runInternalQuery(VdcQueryType.GetImagesList, + new GetImagesListParameters(storageDomainId, ImageType.ISO)); + if (ret != null && ret.getReturnValue() != null && ret.getSucceeded()) { + List<RepoFileMetaData> repoFileNameList = (List<RepoFileMetaData>) ret.getReturnValue(); + if (repoFileNameList != null) { + for (RepoFileMetaData isoFileMetaData : (List<RepoFileMetaData>) ret.getReturnValue()) { + if (isoFileMetaData.getRepoFileName().equals(diskPath)) { + retValForIso = true; + break; + } + } + } + } + if (!retValForIso) { + return new ValidationResult(VdcBllMessages.ERROR_CANNOT_FIND_ISO_IMAGE_PATH); + } + } + + if (!StringUtils.isEmpty(floppyPath)) { + boolean retValForFloppy = false; + VdcQueryReturnValue ret = + getBackend().runInternalQuery(VdcQueryType.GetImagesList, + new GetImagesListParameters(storageDomainId, ImageType.Floppy)); + if (ret != null && ret.getReturnValue() != null && ret.getSucceeded()) { + List<RepoFileMetaData> repoFileNameList = (List<RepoFileMetaData>) ret.getReturnValue(); + if (repoFileNameList != null) { + + for (RepoFileMetaData isoFileMetaData : (List<RepoFileMetaData>) ret.getReturnValue()) { + if (isoFileMetaData.getRepoFileName().equals(floppyPath)) { + retValForFloppy = true; + break; + } + } + } + } + if (!retValForFloppy) { + return new ValidationResult(VdcBllMessages.ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH); + } + } + + return ValidationResult.VALID; + } + + private BackendInternal getBackend() { + return Backend.getInstance(); + } + protected VmNetworkInterfaceDao getVmNetworkInterfaceDao() { return DbFacade.getInstance().getVmNetworkInterfaceDao(); } @@ -75,7 +162,14 @@ // Compatibility method for static VmPoolCommandBase.canRunPoolVm // who uses the same validation as runVmCommand - public boolean canRunVm(VM vm, List<String> messages, List<Disk> vmDisks, BootSequence bootSequence) { + public boolean canRunVm(VM vm, + List<String> messages, + List<Disk> vmDisks, + BootSequence bootSequence, + boolean isInternalExecution, + storage_pool storagePool, + String diskPath, + String floppyPath) { if (!validateVmProperties(vm, messages)) { return false; } @@ -94,6 +188,21 @@ messages.add(result.getMessage().toString()); return false; } + if (validateImagesForRunVm(vm, vmDisks, isInternalExecution, messages)) { + return false; + } + result = new StoragePoolValidator(storagePool).isUp(); + if (!result.isValid()) { + messages.add(result.getMessage().toString()); + return false; + } + if (!vm.isAutoStartup()) { + result = validateIsoPath(!vm.isAutoStartup(), vm.getStoragePoolId(), diskPath, floppyPath); + if (!result.isValid()) { + messages.add(result.getMessage().toString()); + return false; + } + } return true; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java index e2c15d7..f4fb61c 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java @@ -324,11 +324,6 @@ } protected void mockVmRunHandler() { - storage_pool sp = new storage_pool(); - sp.setstatus(StoragePoolStatus.Up); - when(spDao.get(any(Guid.class))).thenReturn(sp); - doReturn(spDao).when(vmRunHandler).getStoragePoolDAO(); - doReturn(vmRunHandler).when(command).getVmRunHandler(); doNothing().when(isoDomainListSyncronizer).init(); @@ -463,6 +458,15 @@ RunVmValidator runVmValidator = mock(RunVmValidator.class); when(runVmValidator.validateVmProperties(any(VM.class), Matchers.anyListOf(String.class))).thenReturn(true); when(runVmValidator.validateBootSequence(any(VM.class), any(BootSequence.class), Matchers.anyListOf(Disk.class))).thenReturn(ValidationResult.VALID); + when(runVmValidator.validateImagesForRunVm(any(VM.class), + Matchers.anyListOf(Disk.class), + anyBoolean(), + Matchers.anyListOf(String.class))).thenReturn(true); + storage_pool sp = new storage_pool(); + sp.setstatus(StoragePoolStatus.Up); + when(spDao.get(any(Guid.class))).thenReturn(sp); + doReturn(spDao).when(command).getStoragePoolDAO(); + when(runVmValidator.validateIsoPath(anyBoolean(), any(Guid.class), any(String.class), any(String.class))).thenReturn(ValidationResult.VALID); doReturn(runVmValidator).when(command).getRunVmValidator(); return runVmValidator; } -- To view, visit http://gerrit.ovirt.org/13402 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I297c9afba655c38ffdd870f61b1967732a712912 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
