Martin Mucha has uploaded a new change for review.
Change subject: core: refactored HostSetupNetworksValidator to use
HostInterfaceValidator
......................................................................
core: refactored HostSetupNetworksValidator to use HostInterfaceValidator
Change-Id: I070bb5c75d635d28911f7f367051c71dbd0127d2
Signed-off-by: Martin Mucha <mmu...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
2 files changed, 65 insertions(+), 27 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/78/35978/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
index b7af35d..9b24576 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java
@@ -16,6 +16,7 @@
import org.ovirt.engine.core.bll.Backend;
import org.ovirt.engine.core.bll.ValidationResult;
import org.ovirt.engine.core.bll.network.VmInterfaceManager;
+import org.ovirt.engine.core.bll.validator.HostInterfaceValidator;
import org.ovirt.engine.core.bll.validator.NetworkAttachmentValidator;
import org.ovirt.engine.core.bll.validator.NetworkAttachmentsValidator;
import org.ovirt.engine.core.common.FeatureSupported;
@@ -42,7 +43,7 @@
private static final Logger log =
LoggerFactory.getLogger(HostSetupNetworksValidator.class);
private HostSetupNetworksParameters params;
private VDS host;
- private Map<VdcBllMessages, List<String>> violations = new
HashMap<VdcBllMessages, List<String>>();
+ private Map<VdcBllMessages, List<String>> violations = new HashMap<>();
private Map<String, VdsNetworkInterface> existingIfaces;
private List<NetworkAttachment> existingAttachments;
private boolean networkCustomPropertiesSupported;
@@ -72,7 +73,9 @@
}
public List<String> validate() {
+ //TODO MM: this should be instance parameter
Map<Guid, NetworkAttachment> attachmentsById =
Entities.businessEntitiesById(existingAttachments);
+ //TODO MM: this should be instance parameter
BusinessEntityMap<Bond> removedBonds = new
BusinessEntityMap<>(params.getRemovedBonds());
if (validModifiedNetworkAttachments(attachmentsById)
@@ -141,21 +144,20 @@
boolean passed = true;
for (Bond removedBond : params.getRemovedBonds()) {
- if (removedBond.getName() == null) {
+ String bondName = removedBond.getName();
+ if (bondName == null) {
addViolation(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST,
null);
passed = false;
continue;
}
- VdsNetworkInterface existingBond =
existingIfaces.get(removedBond.getName());
- if (existingBond != null && !isBond(existingBond)) {
- addViolation(VdcBllMessages.NETWORK_INTERFACE_IS_NOT_BOND,
removedBond.getName());
+ if (validateAndAddViolation(new
HostInterfaceValidator(existingIfaces.get(bondName)).notBond(), bondName)) {
passed = false;
continue;
}
- if (requiredNicsNames.contains(removedBond.getName())) {
- addViolation(VdcBllMessages.BOND_USED_BY_NETWORK_ATTACHMENTS,
removedBond.getName());
+ if (requiredNicsNames.contains(bondName)) {
+ addViolation(VdcBllMessages.BOND_USED_BY_NETWORK_ATTACHMENTS,
bondName);
passed = false;
continue;
}
@@ -206,51 +208,57 @@
private boolean validModifiedBonds(BusinessEntityMap<Bond> removedBonds) {
boolean passed = true;
- for (Bond modifiedBond : params.getBonds()) {
- if (modifiedBond.getName() == null) {
- addViolation(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST,
null);
+ for (Bond modifiedOrNewBond : params.getBonds()) {
+ String bondName = modifiedOrNewBond.getName();
+
+ if (!validateAndAddViolation(new
HostInterfaceValidator(modifiedOrNewBond).interfaceByNameExists(), null)) {
passed = false;
continue;
}
- VdsNetworkInterface existingBond =
existingIfaces.get(modifiedBond.getName());
- if (existingBond != null && !isBond(existingBond)) {
- addViolation(VdcBllMessages.NETWORK_INTERFACE_IS_NOT_BOND,
modifiedBond.getName());
+ //either it's newly create bond, thus non existing, or given name
must reference existing bond.
+ if (validateAndAddViolation(new
HostInterfaceValidator(existingIfaces.get(bondName)).notBond(), bondName)) {
passed = false;
continue;
}
- // verify bond-slave count legal
- if (modifiedBond.getSlaves().size() < 2) {
- addViolation(VdcBllMessages.NETWORK_BONDS_INVALID_SLAVE_COUNT,
modifiedBond.getName());
+ //count of bond slaves must be at least two.
+ if (modifiedOrNewBond.getSlaves().size() < 2) {
+ addViolation(VdcBllMessages.NETWORK_BONDS_INVALID_SLAVE_COUNT,
bondName);
passed = false;
continue;
}
- for (String slaveName : modifiedBond.getSlaves()) {
+ for (String slaveName : modifiedOrNewBond.getSlaves()) {
VdsNetworkInterface slave = getExistingIfaces().get(slaveName);
- if (slave == null) {
-
addViolation(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST, slaveName);
+ HostInterfaceValidator slaveHostInterfaceValidator = new
HostInterfaceValidator(slave);
+
+ if
(!validateAndAddViolation(slaveHostInterfaceValidator.interfaceExists(),
slaveName)) {
passed = false;
continue;
}
- if (NetworkUtils.isVlan(slave) || isBond(slave)) {
-
addViolation(VdcBllMessages.NETWORK_INTERFACE_BOND_OR_VLAN_CANNOT_BE_SLAVE,
slave.getName());
+ if
(!validateAndAddViolation(slaveHostInterfaceValidator.validSlave(), slaveName))
{
passed = false;
continue;
}
+ //definition of currently processed bond references this
slave, but this slave already 'slaves' for another bond. This is ok only when
this bond will be removed as a part of this request.
String slaveBondName = slave.getBondName();
- if (slave.isBondSlave() &&
(!slaveBondName.equals(modifiedBond.getName())
+ if (slave.isBondSlave() &&
+ //we're creating new bond, and it's definition
contains reference to slave already assigned to a different bond.
+ (!slaveBondName.equals(bondName)
+ //…but this bond is also removed in this request, so
it's ok.
|| !slaveUsedByRemovedBond(slaveBondName))) {
addViolation(VdcBllMessages.NETWORK_INTERFACE_ALREADY_IN_BOND, slaveName);
passed = false;
continue;
}
+ //slave has network assigned and there isn't request for
unassigning it;
+ //so this probably check, that nic is part of newly crated
bond, and any previously attached network has to be unattached.
if (slave.getNetworkName() != null &&
!nicUsedByRemovedNetwork(slave)) {
-
addViolation(VdcBllMessages.NETWORK_INTERFACE_ATTACHED_TO_NETWORK_CANNOT_BE_SLAVE,
slave.getName());
+
addViolation(VdcBllMessages.NETWORK_INTERFACE_ATTACHED_TO_NETWORK_CANNOT_BE_SLAVE,
slave.getName());//TODO MM: missing translation
passed = false;
continue;
}
@@ -439,16 +447,27 @@
return passed;
}
+ private boolean validateAndAddViolation(ValidationResult validationResult,
String violatingEntity) {
+ if (validationResult.isValid()) {
+ return true;
+ } else {
+ addViolation(validationResult.getMessage(), violatingEntity);
+ return false;
+ }
+ }
+
private void addViolation(VdcBllMessages violation, String
violatingEntity) {
+ if (violatingEntity == null) {
+ return;
+ }
+
List<String> violatingEntities = violations.get(violation);
if (violatingEntities == null) {
- violatingEntities = new ArrayList<String>();
+ violatingEntities = new ArrayList<>();
violations.put(violation, violatingEntities);
}
- if (violatingEntity != null) {
- violatingEntities.add(violatingEntity);
- }
+ violatingEntities.add(violatingEntity);
}
private boolean validate(ValidationResult validationResult, String
violatingEntity) {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
index d584729..422aca0 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostInterfaceValidator.java
@@ -27,6 +27,11 @@
return
ValidationResult.failWith(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST).when(iface
== null);
}
+ public ValidationResult interfaceByNameExists() {
+ return
ValidationResult.failWith(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST)
+ .when(iface == null || iface.getName() == null);
+ }
+
public ValidationResult interfaceAlreadyLabeledWith(String label) {
return
ValidationResult.failWith(VdcBllMessages.INTERFACE_ALREADY_LABELED)
.when(NetworkUtils.isLabeled(iface) &&
iface.getLabels().contains(label));
@@ -34,6 +39,11 @@
public ValidationResult interfaceInHost(Guid hostId) {
return
ValidationResult.failWith(VdcBllMessages.NIC_NOT_EXISTS_ON_HOST).when(!iface.getVdsId().equals(hostId));
+ }
+
+ public ValidationResult notBond() {
+ return
ValidationResult.failWith(VdcBllMessages.NETWORK_INTERFACE_IS_NOT_BOND)
+ .when(iface != null && !isBond());
}
public ValidationResult labeledValidBond(List<VdsNetworkInterface> nics) {
@@ -70,6 +80,15 @@
"$NETWORK_BONDS_INVALID_SLAVE_COUNT_LIST " +
slavesCount).when(slavesCount < 2);
}
+ public ValidationResult validSlave() {
+ return
ValidationResult.failWith(VdcBllMessages.NETWORK_INTERFACE_BOND_OR_VLAN_CANNOT_BE_SLAVE)//TODO
MM: missing translation.
+ .when(NetworkUtils.isVlan(iface) || isBond());
+ }
+
+ private boolean isBond() {
+ return Boolean.TRUE.equals(iface.getBonded());
+ }
+
public ValidationResult anotherInterfaceAlreadyLabeledWithThisLabel(String
label,
List<VdsNetworkInterface> interfacesToCheck) {
--
To view, visit http://gerrit.ovirt.org/35978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I070bb5c75d635d28911f7f367051c71dbd0127d2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
