Arik Hadas has uploaded a new change for review.
Change subject: core: make RunVmValidator stateful
......................................................................
core: make RunVmValidator stateful
RunVmValidator, as implied by its name, contains validations that should
be checked before running a VM. The relevant information for those
validations is the VM which is going to be run, the parameters for the
run command and whether the run command is internal or not.
This patch changes RunVmValidator to take the information it needs,
which is mentioned above, at creation time (i.e as arguments in its
constructor) and save it as part of the instance state. That way, we
won't need to pass it on each invocation from outside of this class.
Change-Id: I5f2f0ce4a0a92ff610f5c2a8be1e084f0c188f77
Signed-off-by: Arik Hadas <aha...@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/validator/RunVmValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
4 files changed, 35 insertions(+), 50 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/39/18239/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 efd0bc3..1b98f29 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
@@ -84,7 +84,6 @@
public class RunVmCommand<T extends RunVmParams> extends RunVmCommandBase<T>
implements QuotaVdsDependent {
- private static final RunVmValidator runVmValidator = new RunVmValidator();
private boolean mResume;
/** Note: this field should not be used directly, use {@link
#isVmRunningStateless()} instead */
private Boolean cachedVmIsRunningStateless;
@@ -752,27 +751,24 @@
return false;
}
+ RunVmValidator runVmValidator = getRunVmValidator();
+
if (vm.getStatus() == VMStatus.Paused) {
// if VM is paused, it was already checked before that it is
capable to run
return true;
}
- if (!getRunVmValidator().canRunVm(vm,
+ if (!runVmValidator.canRunVm(
getReturnValue().getCanDoActionMessages(),
getDiskDao().getAllForVm(vm.getId(), true),
- getParameters().getBootSequence(),
getStoragePool(),
- isInternalExecution(),
- getParameters().getDiskPath(),
- getParameters().getFloppyPath(),
- getParameters().getRunAsStateless(),
getRunVdssList(),
getDestinationVds() != null ? getDestinationVds().getId() :
null,
getVdsGroup())) {
return false;
}
- if (!validate(getRunVmValidator().validateNetworkInterfaces(vm))) {
+ if (!validate(runVmValidator.validateNetworkInterfaces())) {
return false;
}
@@ -803,7 +799,7 @@
}
protected RunVmValidator getRunVmValidator() {
- return runVmValidator;
+ return new RunVmValidator(getVm(), getParameters(),
isInternalExecution());
}
@Override
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 f15b454..21efc28 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
@@ -2,6 +2,7 @@
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
@@ -19,7 +20,6 @@
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
import org.ovirt.engine.core.common.businessentities.StoragePool;
-import org.ovirt.engine.core.common.businessentities.VDSGroup;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VmPool;
import org.ovirt.engine.core.common.businessentities.VmPoolMap;
@@ -34,7 +34,6 @@
import org.ovirt.engine.core.dao.VmPoolDAO;
public abstract class VmPoolCommandBase<T extends VmPoolParametersBase>
extends CommandBase<T> {
- private static final RunVmValidator runVmValidator = new RunVmValidator();
private static OsRepository osRepository =
SimpleDependecyInjector.getInstance().get(OsRepository.class);
private VmPool mVmPool;
@@ -219,7 +218,6 @@
messages.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND.name());
return false;
}
- VDSGroup vdsGroup =
DbFacade.getInstance().getVdsGroupDao().get(vm.getVdsGroupId());
// TODO: This is done to keep consistency with VmDAO.getById.
// It can probably be removed, but that requires some more research
@@ -228,18 +226,13 @@
RunVmParams runVmParams = new RunVmParams(vmId);
runVmParams.setUseVnc(osRepository.isLinux(vm.getVmOsId()) ||
vm.getVmType() == VmType.Server);
- return getRunVmValidator().canRunVm(vm,
+ return new RunVmValidator(vm, runVmParams, false).canRunVm(
messages,
getDiskDao().getAllForVm(vm.getId(), true),
- runVmParams.getBootSequence(),
fetchStoragePool(vm.getStoragePoolId()),
- false,
- runVmParams.getDiskPath(),
- runVmParams.getFloppyPath(),
- runVmParams.getRunAsStateless(),
- new ArrayList<Guid>(),
+ Collections.<Guid>emptyList(),
null,
- vdsGroup);
+
DbFacade.getInstance().getVdsGroupDao().get(vm.getVdsGroupId()));
}
private static DiskDao getDiskDao() {
@@ -248,10 +241,6 @@
private static StoragePool fetchStoragePool(Guid storagePoolId) {
return DbFacade.getInstance().getStoragePoolDao().get(storagePoolId);
- }
-
- private static RunVmValidator getRunVmValidator() {
- return runVmValidator;
}
@Override
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 7c8a625..df9944f 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
@@ -19,6 +19,7 @@
import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.common.FeatureSupported;
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.BootSequence;
import org.ovirt.engine.core.common.businessentities.Disk;
@@ -54,6 +55,22 @@
import org.ovirt.engine.core.utils.customprop.VmPropertiesUtils;
public class RunVmValidator {
+
+ private VM vm;
+ private RunVmParams runVmParam;
+ private boolean isInternalExecution;
+
+ public RunVmValidator(VM vm, RunVmParams rumVmParam, boolean
isInternalExecution) {
+ this.vm = vm;
+ this.runVmParam = rumVmParam;
+ this.isInternalExecution = isInternalExecution;
+ }
+
+ /**
+ * Used for testings
+ */
+ protected RunVmValidator() {
+ }
public boolean validateVmProperties(VM vm, List<String> messages) {
List<ValidationError> validationErrors =
@@ -321,42 +338,32 @@
/**
* A general method for run vm validations. used in runVmCommand and in
VmPoolCommandBase
- * @param vm
* @param messages
* @param vmDisks
- * @param bootSequence
- * @param isInternalExecution
* @param storagePool
- * @param diskPath
- * @param floppyPath
- * @param runAsStateless
- * @param vdsSelector
* @param vdsBlackList
* - hosts that we already tried to run on
+ * @param destVds
+ * @param vdsGroup
* @return
*/
- public boolean canRunVm(VM vm,
+ public boolean canRunVm(
List<String> messages,
List<Disk> vmDisks,
- BootSequence bootSequence,
StoragePool storagePool,
- boolean isInternalExecution,
- String diskPath,
- String floppyPath,
- Boolean runAsStateless,
List<Guid> vdsBlackList,
Guid destVds,
VDSGroup vdsGroup) {
if (!validateVmProperties(vm, messages) ||
- !validate(validateBootSequence(vm, bootSequence, vmDisks),
messages) ||
+ !validate(validateBootSequence(vm,
runVmParam.getBootSequence(), vmDisks), messages) ||
!validate(new VmValidator(vm).vmNotLocked(), messages) ||
!validate(getSnapshotValidator().vmNotDuringSnapshot(vm.getId()), messages) ||
!validate(validateVmStatusUsingMatrix(vm), messages) ||
- !validate(validateIsoPath(vm, diskPath, floppyPath), messages)
||
+ !validate(validateIsoPath(vm, runVmParam.getDiskPath(),
runVmParam.getFloppyPath()), messages) ||
!validate(vmDuringInitialization(vm), messages) ||
!validate(validateVdsStatus(vm), messages) ||
- !validate(validateStatelessVm(vm, vmDisks, runAsStateless),
messages)) {
+ !validate(validateStatelessVm(vm, vmDisks,
runVmParam.getRunAsStateless()), messages)) {
return false;
}
@@ -379,7 +386,7 @@
/**
* @return true if all VM network interfaces are valid
*/
- public ValidationResult validateNetworkInterfaces(VM vm) {
+ public ValidationResult validateNetworkInterfaces() {
Map<String, VmNetworkInterface> interfaceNetworkMap =
Entities.vmInterfacesByNetworkName(vm.getInterfaces());
Set<String> interfaceNetworkNames = interfaceNetworkMap.keySet();
List<Network> clusterNetworks =
getNetworkDao().getAllForCluster(vm.getVdsGroupId());
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 7867e3d..48f0c52 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
@@ -3,7 +3,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
@@ -31,7 +30,6 @@
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.validator.RunVmValidator;
import org.ovirt.engine.core.common.action.RunVmParams;
-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.IVdsAsyncCommand;
@@ -351,19 +349,14 @@
private RunVmValidator mockSuccessfulRunVmValidator() {
RunVmValidator runVmValidator = mock(RunVmValidator.class);
- when(runVmValidator.canRunVm(any(VM.class),
+ when(runVmValidator.canRunVm(
Matchers.anyListOf(String.class),
Matchers.anyListOf(Disk.class),
- any(BootSequence.class),
any(StoragePool.class),
- anyBoolean(),
- anyString(),
- anyString(),
- anyBoolean(),
Matchers.anyListOf(Guid.class),
any(Guid.class),
any(VDSGroup.class))).thenReturn(true);
-
when(runVmValidator.validateNetworkInterfaces(any(VM.class))).thenReturn(ValidationResult.VALID);
+
when(runVmValidator.validateNetworkInterfaces()).thenReturn(ValidationResult.VALID);
doReturn(runVmValidator).when(command).getRunVmValidator();
return runVmValidator;
}
--
To view, visit http://gerrit.ovirt.org/18239
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f2f0ce4a0a92ff610f5c2a8be1e084f0c188f77
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
