Gilad Chaplik has uploaded a new change for review.
Change subject: core: runVmCommand.canDoAction clean up (13)
......................................................................
core: runVmCommand.canDoAction clean up (13)
- Vm status validtion
- Removal of RunVmHandler
Change-Id: I6e2ca6ca3d1410f857591956a6e2b50dde3153dc
Signed-off-by: Gilad Chaplik <gchap...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
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/RunVmCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
D
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
7 files changed, 23 insertions(+), 161 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/07/13407/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
index a2df4ad..b787548 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
@@ -228,6 +228,10 @@
ImagesHandler.filterImageDisks(pluggedDisks, false, true)));
return null;
}
+
+ private List<Disk> getPluggedDisks(VM vm) {
+ return null;
+ }
});
} catch (VdcBLLException e) {
handleVdsLiveSnapshotFailure(e);
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 f5deb9e..847ac5b 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
@@ -704,7 +704,7 @@
getParameters().getRunAsStateless()))
&&
getVdsSelector().canFindVdsToRunOn(messages,
false) &&
- canRunVm(vm) &&
+
validate(getRunVmValidator().validateVmStatusUsingMatrix(vm)) &&
validateNetworkInterfaces();
if (!canDoAction) {
return false;
@@ -735,14 +735,6 @@
protected RunVmValidator getRunVmValidator() {
return runVmValidator;
- }
-
- protected boolean canRunVm(VM vm) {
- return getVmRunHandler().canRunVm(vm,
- getReturnValue().getCanDoActionMessages(),
- getParameters(),
- getVdsSelector(),
- getSnapshotsValidator());
}
@Override
@@ -917,10 +909,6 @@
ActionGroup.CHANGE_VM_CUSTOM_PROPERTIES));
}
return permissionList;
- }
-
- protected VmRunHandler getVmRunHandler() {
- return VmRunHandler.getInstance();
}
private static final Log log = LogFactory.getLog(RunVmCommand.class);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
index bdfb32a..5712e0b 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
@@ -78,10 +78,6 @@
return snapshotsValidator;
}
- public void setSnapshotsValidator(SnapshotsValidator snapshotsValidator) {
- this.snapshotsValidator = snapshotsValidator;
- }
-
/**
* List on all vdss, vm run on. In the case of problem to run vm will be
more then one
*/
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 e464631..e4e3ef6 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
@@ -276,13 +276,7 @@
runVmParams.getDiskPath(),
runVmParams.getFloppyPath(),
runVmParams.getRunAsStateless(),
- vdsSelector)
- &&
- VmRunHandler.getInstance().canRunVm(vm,
- messages,
- runVmParams,
- vdsSelector,
- new SnapshotsValidator());
+ vdsSelector);
}
private static DiskDao getDiskDao() {
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
deleted file mode 100644
index 34a908c..0000000
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
+++ /dev/null
@@ -1,112 +0,0 @@
-package org.ovirt.engine.core.bll;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import org.ovirt.engine.core.bll.interfaces.BackendInternal;
-import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
-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.DiskImage;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
-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.compat.Guid;
-import org.ovirt.engine.core.dal.VdcBllMessages;
-import org.ovirt.engine.core.dal.dbbroker.DbFacade;
-import org.ovirt.engine.core.dao.DiskDao;
-import org.ovirt.engine.core.dao.StorageDomainDAO;
-import org.ovirt.engine.core.dao.StoragePoolDAO;
-
-/** A utility class for verifying running a vm*/
-public class VmRunHandler {
- private static final VmRunHandler instance = new VmRunHandler();
-
- public static VmRunHandler getInstance() {
- return instance;
- }
-
- /**
- * This method checks whether the given VM is capable to run.
- *
- * @param vm not null {@link VM}
- * @param message
- * @param runParams
- * @param vdsSelector
- * @param snapshotsValidator
- * @param vmPropsUtils
- * @return true if the given VM can run with the given properties, false
otherwise
- */
- public boolean canRunVm(VM vm, ArrayList<String> message, RunVmParams
runParams,
- VdsSelector vdsSelector, SnapshotsValidator snapshotsValidator) {
- boolean retValue = true;
-
- /**
- * only if can do action ok then check with actions matrix that status
is valid for this action
- */
- if (retValue
- && !VdcActionUtils.CanExecute(Arrays.asList(vm), VM.class,
- VdcActionType.RunVm)) {
- retValue = false;
-
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL.toString());
- }
- return retValue;
- }
-
- protected IsoDomainListSyncronizer getIsoDomainListSyncronizer() {
- return IsoDomainListSyncronizer.getInstance();
- }
-
- protected boolean performImageChecksForRunningVm
- (VM vm, List<String> message, RunVmParams runParams,
List<DiskImage> vmDisks) {
- return ImagesHandler.PerformImagesChecks(message,
- vm.getStoragePoolId(), Guid.Empty, !vm.isAutoStartup(),
- true, false, false,
- !vm.isAutoStartup() || !runParams.getIsInternal(),
- !vm.isAutoStartup() || !runParams.getIsInternal(),
- vmDisks);
- }
-
- /**
- * Checks if there is an active ISO domain in the storage pool. If so
returns the Iso Guid, otherwise returns null.
- *
- * @param storagePoolId
- * The storage pool id.
- * @return Iso Guid of active Iso, and null if not.
- */
- public Guid findActiveISODomain(Guid storagePoolId) {
- Guid isoGuid = null;
- List<StorageDomain> domains =
getStorageDomainDAO().getAllForStoragePool(
- storagePoolId);
- for (StorageDomain domain : domains) {
- if (domain.getStorageDomainType() == StorageDomainType.ISO) {
- StorageDomain sd =
getStorageDomainDAO().getForStoragePool(domain.getId(),
- storagePoolId);
- if (sd != null && sd.getStatus() ==
StorageDomainStatus.Active) {
- isoGuid = sd.getId();
- break;
- }
- }
- }
- return isoGuid;
- }
-
- protected BackendInternal getBackend() {
- return Backend.getInstance();
- }
-
- protected DiskDao getDiskDao() {
- return DbFacade.getInstance().getDiskDao();
- }
-
- protected StorageDomainDAO getStorageDomainDAO() {
- return DbFacade.getInstance().getStorageDomainDao();
- }
-
- protected StoragePoolDAO getStoragePoolDAO() {
- return DbFacade.getInstance().getStoragePoolDao();
- }
-}
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 92bfeb0..b9dfab1 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,5 +1,6 @@
package org.ovirt.engine.core.bll.validator;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -15,6 +16,8 @@
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.VdcActionUtils;
+import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.businessentities.BootSequence;
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
@@ -206,6 +209,14 @@
return ValidationResult.VALID;
}
+ public ValidationResult validateVmStatusUsingMatrix(VM vm) {
+ if (!VdcActionUtils.CanExecute(Arrays.asList(vm), VM.class,
+ VdcActionType.RunVm)) {
+ return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL);
+ }
+ return ValidationResult.VALID;
+ }
+
protected SnapshotsValidator getSnapshotValidator() {
return new SnapshotsValidator();
}
@@ -329,6 +340,11 @@
if (!vdsSelector.canFindVdsToRunOn(messages, false)) {
return false;
}
+ result = validateVmStatusUsingMatrix(vm);
+ 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 9d26ff9..e8a8d24 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
@@ -4,7 +4,6 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
-import static org.mockito.Matchers.anyListOf;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
@@ -25,7 +24,6 @@
import org.junit.runner.RunWith;
import org.mockito.Matchers;
import org.mockito.Mock;
-import org.mockito.Spy;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
@@ -79,12 +77,6 @@
@Mock
private StoragePoolDAO spDao;
- @Spy
- private final VmRunHandler vmRunHandler = VmRunHandler.getInstance();
-
- @Mock
- private RunVmValidator runVmValidator;
-
@Mock
private BackendInternal backend;
@@ -98,7 +90,6 @@
public void mockBackend() {
doReturn(backend).when(command).getBackend();
- doReturn(backend).when(vmRunHandler).getBackend();
VDSReturnValue vdsReturnValue = new VDSReturnValue();
vdsReturnValue.setReturnValue(true);
@@ -310,7 +301,6 @@
RunVmParams param = new RunVmParams(Guid.NewGuid());
command = spy(new RunVmCommand<RunVmParams>(param));
mockIsoDomainListSyncronizer();
- mockVmRunHandler();
mockSuccessfulRunVmValidator();
mockSuccessfulSnapshotValidator();
mockBackend();
@@ -319,18 +309,6 @@
private void mockIsoDomainListSyncronizer() {
doNothing().when(isoDomainListSyncronizer).init();
doReturn(isoDomainListSyncronizer).when(command).getIsoDomainListSyncronizer();
- }
-
- protected void mockVmRunHandler() {
- doReturn(vmRunHandler).when(command).getVmRunHandler();
-
- doNothing().when(isoDomainListSyncronizer).init();
-
doReturn(isoDomainListSyncronizer).when(vmRunHandler).getIsoDomainListSyncronizer();
-
-
doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(any(VM.class),
- anyListOf(String.class),
- any(RunVmParams.class),
- anyListOf(DiskImage.class));
}
@Test
@@ -346,7 +324,6 @@
doReturn(true).when(vdsSelector).canFindVdsToRunOn(Matchers.anyList(),
anyBoolean());
doReturn(vdsSelector).when(command).getVdsSelector();
doReturn(vm).when(command).getVm();
- doReturn(true).when(command).canRunVm(vm);
doReturn(true).when(command).validateNetworkInterfaces();
assertTrue(command.canDoAction());
assertTrue(command.getReturnValue().getCanDoActionMessages().isEmpty());
@@ -360,13 +337,11 @@
final DiskDao diskDao = mock(DiskDao.class);
when(diskDao.getAllForVm(Guid.Empty, true)).thenReturn(disks);
doReturn(diskDao).when(command).getDiskDao();
- doReturn(diskDao).when(vmRunHandler).getDiskDao();
final StorageDomainDAO storageDomainDAO = mock(StorageDomainDAO.class);
when(storageDomainDAO.getAllForStoragePool(Guid.Empty))
.thenReturn(new ArrayList<StorageDomain>());
doReturn(storageDomainDAO).when(command).getStorageDomainDAO();
- doReturn(storageDomainDAO).when(vmRunHandler).getStorageDomainDAO();
}
private SnapshotsValidator mockSuccessfulSnapshotValidator() {
@@ -393,6 +368,7 @@
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);
+
when(runVmValidator.validateVmStatusUsingMatrix(any(VM.class))).thenReturn(ValidationResult.VALID);
doReturn(runVmValidator).when(command).getRunVmValidator();
return runVmValidator;
}
--
To view, visit http://gerrit.ovirt.org/13407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e2ca6ca3d1410f857591956a6e2b50dde3153dc
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
