Lior Vernia has uploaded a new change for review. Change subject: webadmin: Enabled more dragging in Setup Networks ......................................................................
webadmin: Enabled more dragging in Setup Networks Changed the validation in NetworkOperationsFactory to allow any logically allowed operation, including bonding together NICs with networks attached to them and dragging bonds over other NICs/bonds. En route, simplified many of the conditions and refactored some common code into functions. Change-Id: Iea5793221d295fa0bd73fc1ed988c90df8fee7c8 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=825801 Signed-off-by: Lior Vernia <[email protected]> --- M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java 1 file changed, 64 insertions(+), 78 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/75/13775/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java index 37952eb..d2a97ac 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java @@ -2,7 +2,9 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; @@ -41,64 +43,22 @@ public static NetworkOperation operationFor(NetworkItemModel<?> op1, NetworkItemModel<?> op2, boolean isDrag) { // !! always check bond before nic because of inheritance !! - // op1: bond - if (op1 instanceof BondNetworkInterfaceModel) { - BondNetworkInterfaceModel bond = (BondNetworkInterfaceModel) op1; - // op2: null - if (op2 == null) { - return NetworkOperation.BREAK_BOND; - } - return NetworkOperation.NULL_OPERATION; - } - // op1: nic - else if (op1 instanceof NetworkInterfaceModel) { + // op1: nic/bond + if (op1 instanceof NetworkInterfaceModel) { NetworkInterfaceModel nic1 = (NetworkInterfaceModel) op1; // op2: null - if (nic1.isBonded() && op2 == null) { - return NetworkOperation.REMOVE_FROM_BOND; + if (op2 == null) { + if (nic1 instanceof BondNetworkInterfaceModel) { + return NetworkOperation.BREAK_BOND; + } else if (nic1.isBonded()) { + return NetworkOperation.REMOVE_FROM_BOND; + } } - // op2: bond - else if (op2 instanceof BondNetworkInterfaceModel) { - BondNetworkInterfaceModel bond = (BondNetworkInterfaceModel) op2; - if (!nic1.isBonded()) { - - boolean containsUnmanaged = containsUnmanaged(nic1); - if (containsUnmanaged){ - return NetworkOperation.NULL_OPERATION_ADD_TO_BOND_UNMANAGED; - } - - boolean containsUnsync = containsUnsync(nic1); - if (containsUnsync){ - return NetworkOperation.NULL_OPERATION_ADD_TO_BOND_UNSYNC; - } - - if (canBond(nic1, bond)) { - return NetworkOperation.ADD_TO_BOND; - } else { - return NetworkOperation.NULL_OPERATION_BOND; - } - } - } - // op2: nic + // op2: nic/bond else if (op2 instanceof NetworkInterfaceModel) { NetworkInterfaceModel nic2 = (NetworkInterfaceModel) op2; if (!nic1.isBonded() && !nic1.equals(nic2)) { - - boolean containsUnmanaged = containsUnmanaged(nic1) || containsUnmanaged(nic2); - if (containsUnmanaged){ - return NetworkOperation.NULL_OPERATION_BOND_WITH_UNMANAGED; - } - - boolean containsUnsync = containsUnsync(nic1) || containsUnsync(nic2); - if (containsUnsync){ - return NetworkOperation.NULL_OPERATION_BOND_WITH_UNSYNC; - } - - if (canBond(nic1, nic2)) { - return NetworkOperation.BOND_WITH; - } else { - return NetworkOperation.NULL_OPERATION_BOND; - } + return nic1.getClass() == nic2.getClass() ? operationForBondWith(nic1, nic2) : operationForAddToBond(nic1, nic2); } } return NetworkOperation.NULL_OPERATION; @@ -135,30 +95,10 @@ return NetworkOperation.NULL_OPERATION_UNSYNC; } - List<LogicalNetworkModel> nicNetworks = nic.getItems(); - if (!nicNetworks.contains(network)) { - if (!network.hasVlan()) { - - // non-vlan, bridge - can't be added to a nic that already has networks - if ((nicNetworks.size() > 0) && (network.getEntity().isVmNetwork())) { - return NetworkOperation.NULL_OPERATION; - } - - // non-vlan, non-bridge - can't be added to a nic that already has a non-vlan network - for (LogicalNetworkModel nicNetwork : nicNetworks) { - if (!nicNetwork.hasVlan()) { - return NetworkOperation.NULL_OPERATION; - } - } - } else { - // vlan- can't be added to a nic that already has non-vlan bridge network - for (LogicalNetworkModel nicNetwork : nicNetworks) { - if (!nicNetwork.hasVlan() && nicNetwork.getEntity().isVmNetwork()) { - return NetworkOperation.NULL_OPERATION; - } - } - } - return NetworkOperation.ATTACH_NETWORK; + Set<LogicalNetworkModel> networks = new HashSet<LogicalNetworkModel>(); + networks.addAll(nic.getItems()); + if (networks.add(network)) { + return setupValid(networks) ? NetworkOperation.ATTACH_NETWORK : NetworkOperation.NULL_OPERATION; } } return NetworkOperation.NULL_OPERATION; @@ -169,8 +109,54 @@ public static NetworkOperation operationFor(NetworkItemModel<?> op1, NetworkItemModel<?> op2) { return operationFor(op1, op2, false); } - private static boolean canBond(NetworkInterfaceModel nic1, NetworkInterfaceModel nic2) { - return (nic1.getItems().size() == 0 || nic2.getItems().size() == 0); + + private static NetworkOperation operationForBondWith(NetworkInterfaceModel nic1, NetworkInterfaceModel nic2) { + if (containsUnmanaged(nic1) || containsUnmanaged(nic2)) { + return NetworkOperation.NULL_OPERATION_BOND_WITH_UNMANAGED; + } else if (containsUnsync(nic1) || containsUnsync(nic2)) { + return NetworkOperation.NULL_OPERATION_BOND_WITH_UNSYNC; + } else { + return setupValid(nic1, nic2) ? NetworkOperation.BOND_WITH : NetworkOperation.NULL_OPERATION_BOND; + } + } + + private static NetworkOperation operationForAddToBond(NetworkInterfaceModel nic1, NetworkInterfaceModel nic2) { + if (containsUnmanaged(nic1) || containsUnmanaged(nic2)) { + return NetworkOperation.NULL_OPERATION_ADD_TO_BOND_UNMANAGED; + } else if (containsUnsync(nic1) || containsUnsync(nic2)) { + return NetworkOperation.NULL_OPERATION_ADD_TO_BOND_UNSYNC; + } else if (setupValid(nic1, nic2)) { + return nic1 instanceof BondNetworkInterfaceModel ? NetworkOperation.EXTEND_BOND : NetworkOperation.ADD_TO_BOND; + } else { + return NetworkOperation.NULL_OPERATION_BOND; + } + } + + private static boolean setupValid(NetworkInterfaceModel nic1, NetworkInterfaceModel nic2) { + Set<LogicalNetworkModel> networks = new HashSet<LogicalNetworkModel>(); + networks.addAll(nic1.getItems()); + networks.addAll(nic2.getItems()); + return setupValid(networks); + } + + private static boolean setupValid(Set<LogicalNetworkModel> networks) { + boolean vlanFound = false; + boolean vmFound = false; + int nonVmCounter = 0; + for (LogicalNetworkModel network : networks) { + if (network.hasVlan()) { + vlanFound = true; + } else if (network.getEntity().isVmNetwork()) { + vmFound = true; + } else { + ++nonVmCounter; + } + + if (nonVmCounter > 1 || (vmFound && (nonVmCounter > 0 || vlanFound))) { + return false; + } + } + return true; } private static boolean containsUnmanaged(NetworkInterfaceModel nic) { -- To view, visit http://gerrit.ovirt.org/13775 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iea5793221d295fa0bd73fc1ed988c90df8fee7c8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
