Lior Vernia has uploaded a new change for review.

Change subject: engine: Stop using network as DTO
......................................................................

engine: Stop using network as DTO

A network entity was being used as a data transfer object, where in
fact it had no real meaning, which was quite confusing.

Change-Id: Id32ee23132d505fc80a4e60b3a70d3453f5d0b35
Signed-off-by: Lior Vernia <[email protected]>
---
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
3 files changed, 38 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/80/37080/1

diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
index c6d0b45..f8ed5ba 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
@@ -9,7 +9,6 @@
 import java.util.Map;
 
 import java.util.Set;
-import org.ovirt.engine.core.common.businessentities.network.Network;
 import 
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
 import org.ovirt.engine.core.common.utils.ObjectUtils;
 import org.ovirt.engine.core.compat.Guid;
@@ -22,7 +21,7 @@
     private VdsDynamic vdsDynamic;
     private VdsStatistics vdsStatistics;
     private ArrayList<VdsNetworkInterface> interfaces;
-    private ArrayList<Network> networks;
+    private Set<String> networks;
     private String activeNic;
     private boolean balloonEnabled;
     private boolean countThreadsAsCores;
@@ -56,7 +55,7 @@
         storagePoolId = Guid.Empty;
         spmStatus = VdsSpmStatus.None;
         interfaces = new ArrayList<>();
-        networks = new ArrayList<>();
+        networks = new HashSet<>();
         fenceAgents = new LinkedList<>();
     }
 
@@ -1024,7 +1023,7 @@
         vdsStatistics = value;
     }
 
-    public ArrayList<Network> getNetworks() {
+    public Set<String> getNetworks() {
         return networks;
     }
 
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java
index 40cc8d1..5aeadcc 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java
@@ -257,13 +257,12 @@
     }
 
     private String getMissingOperationalClusterNetworks(VDS host, 
List<Network> clusterNetworks) {
-        Map<String, Network> hostNetworksByName = 
Entities.entitiesByName(host.getNetworks());
         List<String> networkNames = new ArrayList<>();
 
         for (Network net : clusterNetworks) {
             if (net.getCluster().getStatus() == OPERATIONAL &&
                 net.getCluster().isRequired() &&
-                !hostNetworksByName.containsKey(net.getName())) {
+                !host.getNetworks().contains(net.getName())) {
                 networkNames.add(net.getName());
             }
         }
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
index c3de166..f21e3b8 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
@@ -54,7 +54,6 @@
 import org.ovirt.engine.core.common.businessentities.network.Bond;
 import org.ovirt.engine.core.common.businessentities.network.HostNetworkQos;
 import org.ovirt.engine.core.common.businessentities.network.InterfaceStatus;
-import org.ovirt.engine.core.common.businessentities.network.Network;
 import 
org.ovirt.engine.core.common.businessentities.network.NetworkBootProtocol;
 import org.ovirt.engine.core.common.businessentities.network.Nic;
 import org.ovirt.engine.core.common.businessentities.network.VdsInterfaceType;
@@ -1309,6 +1308,7 @@
         if (networks != null) {
             vds.getNetworks().clear();
             for (Entry<String, Object> entry : networks.entrySet()) {
+                String networkName = entry.getKey();
                 Map<String, Object> network = (Map<String, Object>) 
entry.getValue();
                 if (network != null) {
                     String interfaceName = (String) 
network.get(VdsProperties.INTERFACE);
@@ -1318,22 +1318,34 @@
 
                     boolean bridgedNetwork = isBridgedNetwork(network);
                     HostNetworkQos qos = new 
HostNetworkQosMapper(network).deserialize();
-                    Network net = createNetworkData(entry.getKey(), network);
+                    String addr = extractAddress(network);
+                    String subnet = extractSubnet(network);
+                    String gateway = (String) 
network.get(VdsProperties.GLOBAL_GATEWAY);
 
                     List<VdsNetworkInterface> interfaces =
                             findNetworkInterfaces(vdsInterfaces, 
bridgesReported, networkIface,
                                     bridgesReported ? bridge : network);
                     for (VdsNetworkInterface iface : interfaces) {
-                        updateNetworkDetailsInInterface(iface,
-                                network,
-                                bridgedNetwork,
-                                qos,
-                                vds,
-                                net);
+                        iface.setNetworkName(networkName);
+                        iface.setAddress(addr);
+                        iface.setSubnet(subnet);
+                        iface.setBridged(bridgedNetwork);
+                        iface.setQos(qos);
+
+                        // set the management ip
+                        if (StringUtils.equals(iface.getNetworkName(), 
NetworkUtils.getEngineNetwork())) {
+                            iface.setType(iface.getType() | 
VdsInterfaceType.MANAGEMENT.getValue());
+                        }
+
+                        setGatewayIfNecessary(iface, vds, gateway);
+
+                        if (bridgedNetwork) {
+                            addBootProtocol(network, vds, iface);
+                        }
                     }
 
-                    vds.getNetworks().add(net);
-                    reportInvalidInterfacesForNetwork(interfaces, net, vds);
+                    vds.getNetworks().add(networkName);
+                    reportInvalidInterfacesForNetwork(interfaces, networkName, 
vds);
                 }
             }
         }
@@ -1350,19 +1362,19 @@
      * @param vds
      *            The host in which the network is defined
      */
-    private static void 
reportInvalidInterfacesForNetwork(List<VdsNetworkInterface> interfaces, Network 
network, VDS vds) {
+    private static void 
reportInvalidInterfacesForNetwork(List<VdsNetworkInterface> interfaces, String 
networkName, VDS vds) {
         if (interfaces.isEmpty()) {
-            AuditLogDirector.log(createHostNetworkAuditLog(network, vds), 
AuditLogType.NETWORK_WITHOUT_INTERFACES);
+            AuditLogDirector.log(createHostNetworkAuditLog(networkName, vds), 
AuditLogType.NETWORK_WITHOUT_INTERFACES);
         } else if (interfaces.size() > 1) {
-            AuditLogableBase logable = createHostNetworkAuditLog(network, vds);
+            AuditLogableBase logable = createHostNetworkAuditLog(networkName, 
vds);
             logable.addCustomValue("Interfaces", 
StringUtils.join(Entities.objectNames(interfaces), ","));
             AuditLogDirector.log(logable, 
AuditLogType.BRIDGED_NETWORK_OVER_MULTIPLE_INTERFACES);
         }
     }
 
-    protected static AuditLogableBase createHostNetworkAuditLog(Network 
network, VDS vds) {
+    protected static AuditLogableBase createHostNetworkAuditLog(String 
networkName, VDS vds) {
         AuditLogableBase logable = new AuditLogableBase(vds.getId());
-        logable.addCustomValue("NetworkName", network.getName());
+        logable.addCustomValue("NetworkName", networkName);
         return logable;
     }
 
@@ -1384,18 +1396,6 @@
             interfaces.addAll(findBridgedNetworkInterfaces(entry, 
vdsInterfaces));
         }
         return interfaces;
-    }
-
-    private static Network createNetworkData(String networkName, Map<String, 
Object> network) {
-        Network net = new Network();
-        net.setName(networkName);
-        net.setAddr((String) network.get("addr"));
-        net.setSubnet((String) network.get("netmask"));
-        net.setGateway((String) network.get(VdsProperties.GLOBAL_GATEWAY));
-        if (StringUtils.isNotBlank((String) network.get(VdsProperties.MTU))) {
-            net.setMtu(Integer.parseInt((String) 
network.get(VdsProperties.MTU)));
-        }
-        return net;
     }
 
     private static List<VdsNetworkInterface> 
findBridgedNetworkInterfaces(Map<String, Object> bridge,
@@ -1529,8 +1529,8 @@
 
         Map<String, Object> nic = (Map<String, Object>) entry.getValue();
         if (nic != null) {
-            iface.setAddress((String) nic.get("addr"));
-            iface.setSubnet((String) nic.get("netmask"));
+            iface.setAddress(extractAddress(nic));
+            iface.setSubnet(extractSubnet(nic));
 
             String mtu = (String) nic.get(VdsProperties.MTU);
             if (StringUtils.isNotBlank(mtu)) {
@@ -1541,49 +1541,12 @@
         }
     }
 
-    /**
-     * Update the network details on a given interface.
-     *
-     * @param iface
-     *            The interface to update.
-     * @param network
-     *            Network struct to get details from.
-     * @param bridgedNetwork
-     *            Whether the network is bridged.
-     * @param qos
-     *            The reported network QoS.
-     * @param host
-     *            The host to which the interface belongs.
-     * @param net
-     *            Network to get details from.
-     */
-    private static void updateNetworkDetailsInInterface(VdsNetworkInterface 
iface,
-            Map<String, Object> network,
-            boolean bridgedNetwork,
-            HostNetworkQos qos,
-            VDS host,
-            Network net) {
+    private static String extractAddress(Map<String, Object> nic) {
+        return (String) nic.get("addr");
+    }
 
-        if (iface != null) {
-            iface.setNetworkName(net.getName());
-
-            // set the management ip
-            if (StringUtils.equals(iface.getNetworkName(), 
NetworkUtils.getEngineNetwork())) {
-                iface.setType(iface.getType() | 
VdsInterfaceType.MANAGEMENT.getValue());
-            }
-
-            iface.setAddress(net.getAddr());
-            iface.setSubnet(net.getSubnet());
-            iface.setBridged(bridgedNetwork);
-            setGatewayIfNecessary(iface, host, net.getGateway());
-
-            if (bridgedNetwork) {
-                Map<String, Object> networkConfig = (Map<String, Object>) 
network.get("cfg");
-                addBootProtocol(networkConfig, host, iface);
-            }
-
-            iface.setQos(qos);
-        }
+    private static String extractSubnet(Map<String, Object> nic) {
+        return (String) nic.get("netmask");
     }
 
     /**


-- 
To view, visit http://gerrit.ovirt.org/37080
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id32ee23132d505fc80a4e60b3a70d3453f5d0b35
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

Reply via email to