Arik Hadas has uploaded a new change for review.
Change subject: core: move network validations on run vm to RunVmValidator
......................................................................
core: move network validations on run vm to RunVmValidator
Move the network validations from RunVmCommand to RunVmValidator, where
this kind of validations should reside. The methods that were moved were
changed to return ValidationResult instead of boolean to conform the
standard.
Change-Id: I6aff70b45617e90deac826f2a873c1604c9a4241
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/validator/RunVmValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
3 files changed, 106 insertions(+), 88 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/71/17971/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 c71b5ed..f6fb5d2 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
@@ -4,10 +4,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -56,7 +54,6 @@
import org.ovirt.engine.core.common.businessentities.VmPool;
import org.ovirt.engine.core.common.businessentities.VmPoolType;
import org.ovirt.engine.core.common.businessentities.network.Network;
-import
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
import org.ovirt.engine.core.common.businessentities.network.VmNic;
import org.ovirt.engine.core.common.errors.VdcBLLException;
import org.ovirt.engine.core.common.errors.VdcBllErrors;
@@ -78,7 +75,6 @@
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
import org.ovirt.engine.core.dal.job.ExecutionMessageDirector;
import org.ovirt.engine.core.dao.SnapshotDao;
-import org.ovirt.engine.core.utils.NetworkUtils;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
@@ -772,7 +768,7 @@
return false;
}
- if (!validateNetworkInterfaces()) {
+ if (!validate(getRunVmValidator().validateNetworkInterfaces(vm))) {
return false;
}
@@ -906,88 +902,6 @@
cachedVmIsRunningStateless = isStatelessSnapshotExistsForVm();
}
return cachedVmIsRunningStateless;
- }
-
- /**
- * @return true if all VM network interfaces are valid
- */
- protected boolean validateNetworkInterfaces() {
- Map<String, VmNetworkInterface> interfaceNetworkMap =
Entities.vmInterfacesByNetworkName(getVm().getInterfaces());
- Set<String> interfaceNetworkNames = interfaceNetworkMap.keySet();
- List<Network> clusterNetworks =
getNetworkDAO().getAllForCluster(getVm().getVdsGroupId());
- Set<String> clusterNetworksNames =
Entities.objectNames(clusterNetworks);
-
- return isVmInterfacesConfigured() &&
- isVmInterfacesAttachedToClusterNetworks(clusterNetworksNames,
interfaceNetworkNames) &&
- isVmInterfacesAttachedToVmNetworks(clusterNetworks,
interfaceNetworkNames);
- }
-
- /**
- * Checking that the interfaces are all configured, interfaces with no
network are allowed only if network linking
- * is supported.
- *
- * @return true if all VM network interfaces are attached to existing
cluster networks, or to no network (when
- * network linking is supported).
- */
- private boolean isVmInterfacesConfigured() {
- for (VmNetworkInterface nic : getVm().getInterfaces()) {
- if (nic.getVnicProfileId() == null) {
- if
(!FeatureSupported.networkLinking(getVm().getVdsGroupCompatibilityVersion())) {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_INTERFACE_NETWORK_NOT_CONFIGURED);
- return false;
- } else {
- return true;
- }
- }
- }
- return true;
- }
-
- /**
- * @param clusterNetworksNames
- * cluster logical networks names
- * @param interfaceNetworkNames
- * VM interface network names
- * @return true if all VM network interfaces are attached to existing
cluster networks
- */
- private boolean isVmInterfacesAttachedToClusterNetworks(final Set<String>
clusterNetworkNames,
- final Set<String> interfaceNetworkNames) {
-
- Set<String> result = new HashSet<String>(interfaceNetworkNames);
- result.removeAll(clusterNetworkNames);
- if
(FeatureSupported.networkLinking(getVm().getVdsGroupCompatibilityVersion())) {
- result.remove(null);
- }
-
- // If after removing the cluster network names we still have objects,
then we have interface on networks that
- // aren't
- // attached to the cluster
- if (result.size() > 0) {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_NOT_IN_CLUSTER);
- addCanDoActionMessage(String.format("$networks %1$s",
StringUtils.join(result, ",")));
- return false;
- }
- return true;
- }
-
- /**
- * @param clusterNetworks
- * cluster logical networks
- * @param interfaceNetworkNames
- * VM interface network names
- * @return true if all VM network interfaces are attached to VM networks
- */
- private boolean isVmInterfacesAttachedToVmNetworks(final List<Network>
clusterNetworks,
- Set<String> interfaceNetworkNames) {
- List<String> nonVmNetworkNames =
- NetworkUtils.filterNonVmNetworkNames(clusterNetworks,
interfaceNetworkNames);
-
- if (nonVmNetworkNames.size() > 0) {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NOT_A_VM_NETWORK);
- addCanDoActionMessage(String.format("$networks %1$s",
StringUtils.join(nonVmNetworkNames, ",")));
- return false;
- }
- return true;
}
@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 d47f513..589509e 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,6 +2,7 @@
import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -16,11 +17,13 @@
import org.ovirt.engine.core.bll.scheduling.SchedulingManager;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
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.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;
+import org.ovirt.engine.core.common.businessentities.Entities;
import org.ovirt.engine.core.common.businessentities.ImageFileType;
import org.ovirt.engine.core.common.businessentities.RepoImage;
import org.ovirt.engine.core.common.businessentities.StorageDomain;
@@ -30,6 +33,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.network.Network;
+import
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
import org.ovirt.engine.core.common.config.Config;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
@@ -42,7 +47,9 @@
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.NetworkDao;
import org.ovirt.engine.core.dao.network.VmNicDao;
+import org.ovirt.engine.core.utils.NetworkUtils;
import org.ovirt.engine.core.utils.customprop.ValidationError;
import org.ovirt.engine.core.utils.customprop.VmPropertiesUtils;
@@ -286,6 +293,10 @@
return map;
}
+ protected NetworkDao getNetworkDAO() {
+ return DbFacade.getInstance().getNetworkDao();
+ }
+
protected VdsDAO getVdsDao() {
return DbFacade.getInstance().getVdsDao();
}
@@ -378,4 +389,97 @@
return true;
}
+
+ /**
+ * @return true if all VM network interfaces are valid
+ */
+ public ValidationResult validateNetworkInterfaces(VM vm) {
+ Map<String, VmNetworkInterface> interfaceNetworkMap =
Entities.vmInterfacesByNetworkName(vm.getInterfaces());
+ Set<String> interfaceNetworkNames = interfaceNetworkMap.keySet();
+ List<Network> clusterNetworks =
getNetworkDAO().getAllForCluster(vm.getVdsGroupId());
+ Set<String> clusterNetworksNames =
Entities.objectNames(clusterNetworks);
+
+ ValidationResult validationResult = isVmInterfacesConfigured(vm);
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+
+ validationResult = isVmInterfacesAttachedToClusterNetworks(vm,
clusterNetworksNames, interfaceNetworkNames);
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+
+ validationResult = isVmInterfacesAttachedToVmNetworks(clusterNetworks,
interfaceNetworkNames);
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+
+ return ValidationResult.VALID;
+ }
+
+ /**
+ * Checking that the interfaces are all configured, interfaces with no
network are allowed only if network linking
+ * is supported.
+ *
+ * @return true if all VM network interfaces are attached to existing
cluster networks, or to no network (when
+ * network linking is supported).
+ */
+ private ValidationResult isVmInterfacesConfigured(VM vm) {
+ for (VmNetworkInterface nic : vm.getInterfaces()) {
+ if (nic.getVnicProfileId() == null) {
+ return
!FeatureSupported.networkLinking(vm.getVdsGroupCompatibilityVersion()) ?
+ new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_INTERFACE_NETWORK_NOT_CONFIGURED)
:
+ ValidationResult.VALID;
+ }
+ }
+ return ValidationResult.VALID;
+ }
+
+ /**
+ * @param clusterNetworksNames
+ * cluster logical networks names
+ * @param interfaceNetworkNames
+ * VM interface network names
+ * @return true if all VM network interfaces are attached to existing
cluster networks
+ */
+ private ValidationResult isVmInterfacesAttachedToClusterNetworks(VM vm,
+ final Set<String> clusterNetworkNames, final Set<String>
interfaceNetworkNames) {
+
+ Set<String> result = new HashSet<String>(interfaceNetworkNames);
+ result.removeAll(clusterNetworkNames);
+ if
(FeatureSupported.networkLinking(vm.getVdsGroupCompatibilityVersion())) {
+ result.remove(null);
+ }
+
+ // If after removing the cluster network names we still have objects,
then we have interface on networks that
+ // aren't
+ // attached to the cluster
+ if (result.size() > 0) {
+ return new ValidationResult(
+ VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_NOT_IN_CLUSTER,
+ String.format("$networks %1$s", StringUtils.join(result,
",")));
+ }
+ return ValidationResult.VALID;
+ }
+
+ /**
+ * @param clusterNetworks
+ * cluster logical networks
+ * @param interfaceNetworkNames
+ * VM interface network names
+ * @return true if all VM network interfaces are attached to VM networks
+ */
+ private ValidationResult isVmInterfacesAttachedToVmNetworks(final
List<Network> clusterNetworks,
+ Set<String> interfaceNetworkNames) {
+ List<String> nonVmNetworkNames =
+ NetworkUtils.filterNonVmNetworkNames(clusterNetworks,
interfaceNetworkNames);
+
+ if (nonVmNetworkNames.size() > 0) {
+ return new ValidationResult(
+ VdcBllMessages.ACTION_TYPE_FAILED_NOT_A_VM_NETWORK,
+ String.format("$networks %1$s",
StringUtils.join(nonVmNetworkNames, ",")));
+ }
+ return ValidationResult.VALID;
+ }
+
}
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 0c8559e..7867e3d 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
@@ -320,7 +320,6 @@
vm.setStatus(VMStatus.Down);
doReturn(new StoragePool()).when(command).getStoragePool();
doReturn(vm).when(command).getVm();
- doReturn(true).when(command).validateNetworkInterfaces();
doReturn(true).when(command).checkPayload(any(VmPayload.class),
anyString());
doReturn(new VDSGroup()).when(command).getVdsGroup();
assertTrue(command.canDoAction());
@@ -364,6 +363,7 @@
Matchers.anyListOf(Guid.class),
any(Guid.class),
any(VDSGroup.class))).thenReturn(true);
+
when(runVmValidator.validateNetworkInterfaces(any(VM.class))).thenReturn(ValidationResult.VALID);
doReturn(runVmValidator).when(command).getRunVmValidator();
return runVmValidator;
}
--
To view, visit http://gerrit.ovirt.org/17971
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6aff70b45617e90deac826f2a873c1604c9a4241
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
