Gilad Chaplik has uploaded a new change for review.
Change subject: core: RunVmCommand.canDoAction clean-up (11)
......................................................................
core: RunVmCommand.canDoAction clean-up (11)
Handle stateless vm.
Change-Id: Ie887a05e17dd6a59b4e5d2df83a9aa7e357526dc
Signed-off-by: Gilad Chaplik <gchap...@redhat.com>
---
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
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
6 files changed, 182 insertions(+), 150 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/05/13405/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 f6acd1c..8f91e85 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
@@ -528,8 +528,8 @@
// Clear the first user:
getVm().setConsoleUserId(null);
-
-
getParameters().setRunAsStateless(getVmRunHandler().shouldVmRunAsStateless(getParameters(),
getVm()));
+
getParameters().setRunAsStateless(getParameters().getRunAsStateless() != null ?
getParameters().getRunAsStateless()
+ : getVm().isStateless());
getVm().setDisplayType(getParameters().getUseVnc() == null ?
getVm().getDefaultDisplayType() :
@@ -699,6 +699,10 @@
getParameters().getFloppyPath())) &&
validate(getRunVmValidator().vmDuringInitialization(vm)) &&
getRunVmValidator().validateVdsStatus(vm,
messages) &&
+
validate(getRunVmValidator().validateStatelessVm(vm,
+ vmDisks,
+ getParameters().getRunAsStateless()))
+ &&
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 4206664..7e5f110 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
@@ -274,7 +274,8 @@
false,
fetchStoragePool(vm.getStoragePoolId()),
runVmParams.getDiskPath(),
- runVmParams.getFloppyPath())
+ runVmParams.getFloppyPath(),
+ runVmParams.getRunAsStateless())
&&
VmRunHandler.getInstance().canRunVm(vm,
messages,
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 f1ccf73..008ff83 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
@@ -2,14 +2,10 @@
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
import org.ovirt.engine.core.bll.interfaces.BackendInternal;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
-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;
@@ -19,8 +15,6 @@
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.VM;
-import org.ovirt.engine.core.common.config.Config;
-import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.VdcBllMessages;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
@@ -53,22 +47,6 @@
List<Disk> vmDisks = getDiskDao().getAllForVm(vm.getId(), true);
if (retValue) {
- 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;
- }
}
retValue = retValue == false ? retValue :
vdsSelector.canFindVdsToRunOn(message, false);
@@ -89,59 +67,6 @@
return IsoDomainListSyncronizer.getInstance();
}
- /**
- * check that we can create snapshots for all disks
- *
- * @param vm
- * @return true if all storage domains have enough space to create
snapshots for this VM plugged disks
- */
- public boolean hasSpaceForSnapshots(VM vm, ArrayList<String> message) {
- Integer minSnapshotSize = Config.<Integer>
GetValue(ConfigValues.InitStorageSparseSizeInGB);
- for (Entry<StorageDomain, Integer> e :
mapStorageDomainsToNumOfDisks(vm).entrySet()) {
- if (!destinationHasSpace(e.getKey(), minSnapshotSize *
e.getValue(), message)) {
- return false;
- }
- }
- return true;
- }
-
- private boolean destinationHasSpace(StorageDomain storageDomain, long
sizeRequested, ArrayList<String> message) {
- return validate(new
StorageDomainValidator(storageDomain).isDomainHasSpaceForRequest(sizeRequested),
message);
- }
-
- protected boolean validate(ValidationResult validationResult,
ArrayList<String> message) {
- if (!validationResult.isValid()) {
- message.add(validationResult.getMessage().name());
- if (validationResult.getVariableReplacements() != null) {
- for (String variableReplacement :
validationResult.getVariableReplacements()) {
- message.add(variableReplacement);
- }
- }
- }
- return validationResult.isValid();
- }
-
- /**
- * map the VM number of pluggable and snapable disks from their domain.
- *
- * @param vm
- * @return
- */
- public Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks(VM vm) {
- Map<StorageDomain, Integer> map = new HashMap<StorageDomain,
Integer>();
- for (Disk disk : getDiskDao().getAllForVm(vm.getId(), true)) {
- if (disk.isAllowSnapshot()) {
- for (StorageDomain domain :
getStorageDomainDAO().getAllStorageDomainsByImageId(((DiskImage)
disk).getImageId())) {
- map.put(domain, map.containsKey(domain) ?
Integer.valueOf(map.get(domain) + 1) : Integer.valueOf(1));
- }
- }
- }
- return map;
- }
-
- /**
- * Check isValid, storageDomain and diskSpace only if VM is not HA VM
- */
protected boolean performImageChecksForRunningVm
(VM vm, List<String> message, RunVmParams runParams,
List<DiskImage> vmDisks) {
return ImagesHandler.PerformImagesChecks(message,
@@ -174,13 +99,6 @@
}
}
return isoGuid;
- }
-
- public boolean shouldVmRunAsStateless(RunVmParams param, VM vm) {
- if (param.getRunAsStateless() != null) {
- return param.getRunAsStateless();
- }
- return vm.isStateless();
}
protected BackendInternal getBackend() {
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 d576368..5eb81df 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
@@ -1,6 +1,9 @@
package org.ovirt.engine.core.bll.validator;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
import org.apache.commons.lang.StringUtils;
import org.ovirt.engine.core.bll.Backend;
@@ -16,11 +19,14 @@
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.VDS;
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;
@@ -29,6 +35,7 @@
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.VdcBllMessages;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
+import org.ovirt.engine.core.dao.StorageDomainDAO;
import org.ovirt.engine.core.dao.VdsDAO;
import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao;
import org.ovirt.engine.core.utils.vmproperties.VmPropertiesUtils;
@@ -175,16 +182,82 @@
return true;
}
- protected BackendInternal getBackend() {
- return Backend.getInstance();
+ public ValidationResult validateStatelessVm(VM vm, List<Disk> plugDisks,
Boolean stateless) {
+ boolean isStatelessVm = stateless != null ? stateless :
vm.isStateless();
+ if (!isStatelessVm) {
+ return ValidationResult.VALID;
+ }
+
+ ValidationResult previewValidation =
getSnapshotValidator().vmNotInPreview(vm.getId());
+ if (!previewValidation.isValid()) {
+ return previewValidation;
+ }
+
+ // if the VM itself is stateless or run once as stateless
+ if (vm.isAutoStartup()) {
+ return new
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA);
+ }
+
+ ValidationResult hasSpaceValidation = hasSpaceForSnapshots(vm,
plugDisks);
+ if (!hasSpaceValidation.isValid()) {
+ return hasSpaceValidation;
+ }
+ return ValidationResult.VALID;
+ }
+
+ protected SnapshotsValidator getSnapshotValidator() {
+ return new SnapshotsValidator();
+ }
+
+ /**
+ * check that we can create snapshots for all disks
+ * @param vm
+ * @return true if all storage domains have enough space to create
snapshots for this VM plugged disks
+ */
+ public ValidationResult hasSpaceForSnapshots(VM vm, List<Disk> plugDisks) {
+ Integer minSnapshotSize = Config.<Integer>
GetValue(ConfigValues.InitStorageSparseSizeInGB);
+ Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks =
mapStorageDomainsToNumOfDisks(vm, plugDisks);
+ for (Entry<StorageDomain, Integer> e :
mapStorageDomainsToNumOfDisks.entrySet()) {
+ ValidationResult validationResult =
+ new
StorageDomainValidator(e.getKey()).isDomainHasSpaceForRequest(minSnapshotSize *
e.getValue());
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+ }
+ return ValidationResult.VALID;
+ }
+
+ /**
+ * map the VM number of pluggable and snapable disks from their domain.
+ * @param vm
+ * @return
+ */
+ public Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks(VM vm,
List<Disk> plugDisks) {
+ Map<StorageDomain, Integer> map = new HashMap<StorageDomain,
Integer>();
+ for (Disk disk : plugDisks) {
+ if (disk.isAllowSnapshot()) {
+ for (StorageDomain domain :
getStorageDomainDAO().getAllStorageDomainsByImageId(((DiskImage)
disk).getImageId())) {
+ map.put(domain, map.containsKey(domain) ?
Integer.valueOf(map.get(domain) + 1) : Integer.valueOf(1));
+ }
+ }
+ }
+ return map;
}
protected VdsDAO getVdsDao() {
return DbFacade.getInstance().getVdsDao();
}
+ protected BackendInternal getBackend() {
+ return Backend.getInstance();
+ }
+
protected VmNetworkInterfaceDao getVmNetworkInterfaceDao() {
return DbFacade.getInstance().getVmNetworkInterfaceDao();
+ }
+
+ protected StorageDomainDAO getStorageDomainDAO() {
+ return DbFacade.getInstance().getStorageDomainDao();
}
protected IsoDomainListSyncronizer getIsoDomainListSyncronizer() {
@@ -204,7 +277,8 @@
boolean isInternalExecution,
storage_pool storagePool,
String diskPath,
- String floppyPath) {
+ String floppyPath,
+ Boolean runAsStateless) {
if (!validateVmProperties(vm, messages)) {
return false;
}
@@ -218,7 +292,7 @@
messages.add(result.getMessage().toString());
return false;
}
- result = new SnapshotsValidator().vmNotDuringSnapshot(vm.getId());
+ result = getSnapshotValidator().vmNotDuringSnapshot(vm.getId());
if (!result.isValid()) {
messages.add(result.getMessage().toString());
return false;
@@ -246,6 +320,11 @@
if (!validateVdsStatus(vm, messages)) {
return false;
}
+ result = validateStatelessVm(vm, vmDisks, runAsStateless);
+ 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 923114a..d2812a7 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
@@ -352,66 +352,6 @@
assertTrue(command.getReturnValue().getCanDoActionMessages().isEmpty());
}
- private void canRunStatelessVmTest(boolean autoStartUp,
- boolean isVmStateless,
- Boolean isStatelessParam,
- boolean shouldPass) {
- final ArrayList<Disk> disks = new ArrayList<Disk>();
- final DiskImage diskImage = createImage();
- disks.add(diskImage);
-
- final VdsSelector vdsSelector = mock(VdsSelector.class);
- when(vdsSelector.canFindVdsToRunOn(anyListOf(String.class),
anyBoolean())).thenReturn(true);
- doReturn(vdsSelector).when(command).getVdsSelector();
-
- VDSReturnValue vdsReturnValue = new VDSReturnValue();
- vdsReturnValue.setReturnValue(false);
-
when(vdsBrokerFrontend.RunVdsCommand(eq(VDSCommandType.IsVmDuringInitiating),
any(VDSParametersBase.class))).thenReturn(vdsReturnValue);
- initDAOMocks(disks);
-
- final VM vm = new VM();
- // set stateless and HA
- vm.setStateless(isVmStateless);
- vm.setAutoStartup(autoStartUp);
-
- command.getParameters().setRunAsStateless(isStatelessParam);
- boolean canRunVm = command.canRunVm(vm);
-
- final List<String> messages =
command.getReturnValue().getCanDoActionMessages();
- assertEquals(shouldPass, canRunVm);
- assertEquals(shouldPass,
!messages.contains("VM_CANNOT_RUN_STATELESS_HA"));
- }
-
- @Test
- public void canRunVmFailStatelessWhenVmHA() {
- canRunStatelessVmTest(true, false, Boolean.TRUE, false);
- }
-
- @Test
- public void canRunVmPassStatelessWhenVmHAandStatelessFalse() {
- canRunStatelessVmTest(true, true, Boolean.FALSE, true);
- }
-
- @Test
- public void canRunVmFailStatelessWhenVmHAwithNullStatelessParam() {
- canRunStatelessVmTest(true, true, null, false);
- }
-
- @Test
- public void canRunVmPassStatelessWhenVmHAwithNullStatelessParam() {
- canRunStatelessVmTest(true, false, null, true);
- }
-
- @Test
- public void canRunVmPassStatelessWhenVmHAwithNegativeStatelessParam() {
- canRunStatelessVmTest(true, false, Boolean.FALSE, true);
- }
-
- @Test
- public void canRunVmPassStatelessWhenVmNotHAwithNegativeStatelessParam() {
- canRunStatelessVmTest(false, false, Boolean.TRUE, true);
- }
-
/**
* @param disks
* @param guid
@@ -452,6 +392,7 @@
when(runVmValidator.validateIsoPath(anyBoolean(), any(Guid.class),
any(String.class), any(String.class))).thenReturn(ValidationResult.VALID);
when(runVmValidator.vmDuringInitialization(any(VM.class))).thenReturn(ValidationResult.VALID);
when(runVmValidator.validateVdsStatus(any(VM.class),
Matchers.anyListOf(String.class))).thenReturn(true);
+ when(runVmValidator.validateStatelessVm(any(VM.class),
Matchers.anyListOf(Disk.class),
anyBoolean())).thenReturn(ValidationResult.VALID);
doReturn(runVmValidator).when(command).getRunVmValidator();
return runVmValidator;
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
index 0a0f3ce..6578efd 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
@@ -8,6 +8,7 @@
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
+import java.util.Random;
import java.util.Set;
import org.junit.Before;
@@ -16,6 +17,7 @@
import org.ovirt.engine.core.bll.IsoDomainListSyncronizer;
import org.ovirt.engine.core.bll.VDSBrokerFrontendImpl;
import org.ovirt.engine.core.bll.ValidationResult;
+import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.common.businessentities.BootSequence;
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
@@ -164,9 +166,10 @@
protected IsoDomainListSyncronizer getIsoDomainListSyncronizer() {
return new IsoDomainListSyncronizer() {
@Override
- protected void init(){
+ protected void init() {
// no impl
}
+
@Override
public Guid findActiveISODomain(Guid storagePoolId) {
return new Guid();
@@ -195,6 +198,92 @@
VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING);
}
+ @Test
+ public void passNotStatelessVM() {
+ Random rand = new Random();
+ canRunVmAsStateless(rand.nextBoolean(), rand.nextBoolean(), false,
false, true, null);
+ canRunVmAsStateless(rand.nextBoolean(), rand.nextBoolean(), false,
null, true, null);
+ }
+
+ @Test
+ public void failRunStatelessSnapshotInPreview() {
+ Random rand = new Random();
+ canRunVmAsStateless(rand.nextBoolean(),
+ true,
+ true,
+ true,
+ false,
+ VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW);
+ canRunVmAsStateless(rand.nextBoolean(),
+ true,
+ true,
+ null,
+ false,
+ VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW);
+ canRunVmAsStateless(rand.nextBoolean(),
+ true,
+ false,
+ true,
+ false,
+ VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW);
+ }
+
+ @Test
+ public void failRunStatelessHA_VM() {
+ canRunVmAsStateless(true,
+ false,
+ true,
+ true,
+ false,
+ VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA);
+ canRunVmAsStateless(true,
+ false,
+ true,
+ null,
+ false,
+ VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA);
+ canRunVmAsStateless(true,
+ false,
+ false,
+ true,
+ false,
+ VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA);
+ }
+
+ private void canRunVmAsStateless(boolean autoStartUp,
+ final boolean vmInPreview,
+ boolean isVmStateless,
+ Boolean isStatelessParam,
+ boolean shouldPass,
+ VdcBllMessages message) {
+ runVmValidator = new RunVmValidator() {
+ @Override
+ protected SnapshotsValidator getSnapshotValidator() {
+ return new SnapshotsValidator() {
+ @Override
+ public ValidationResult vmNotInPreview(Guid vmId) {
+ if (vmInPreview) {
+ return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW);
+ }
+ return ValidationResult.VALID;
+ };
+ };
+ };
+
+ @Override
+ public ValidationResult hasSpaceForSnapshots(VM vm, List<Disk>
plugDisks) {
+ return ValidationResult.VALID;
+ }
+ };
+ VM vm = new VM();
+ vm.setAutoStartup(autoStartUp);
+ vm.setStateless(isVmStateless);
+ List<Disk> disks = new ArrayList<Disk>();
+ validateResult(runVmValidator.validateStatelessVm(vm, disks,
isStatelessParam),
+ shouldPass,
+ message);
+ }
+
private VmPropertiesUtils mockVmPropertiesUtils() {
VmPropertiesUtils utils = new VmPropertiesUtils() {
@Override
--
To view, visit http://gerrit.ovirt.org/13405
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie887a05e17dd6a59b4e5d2df83a9aa7e357526dc
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
