Lior Vernia has uploaded a new change for review. Change subject: engine: Extract common interface logic to single method ......................................................................
engine: Extract common interface logic to single method The same code was duplicated across interfaces, bonds and VLANs, whereas it could be re-used. Change-Id: I495f36cea0a63a161ca4ae85fc08e67e718befa2 Signed-off-by: Lior Vernia <[email protected]> --- M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java 1 file changed, 38 insertions(+), 57 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/78/37078/1 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 05268a8..816d8c1 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 @@ -1411,27 +1411,14 @@ if (bonds != null) { for (Entry<String, Object> entry : bonds.entrySet()) { VdsNetworkInterface iface = new Bond(); - VdsNetworkStatistics iStats = new VdsNetworkStatistics(); - iface.setStatistics(iStats); - iStats.setId(Guid.newGuid()); - iStats.setVdsId(vds.getId()); - iface.setId(iStats.getId()); - - iface.setName(entry.getKey()); - iface.setVdsId(vds.getId()); + updateCommonInterfaceData(iface, vds, entry); iface.setBonded(true); Map<String, Object> bond = (Map<String, Object>) entry.getValue(); if (bond != null) { iface.setMacAddress((String) bond.get("hwaddr")); - iface.setAddress((String) bond.get("addr")); - iface.setSubnet((String) bond.get("netmask")); if (bond.get("slaves") != null) { addBondDeviceToHost(vds, iface, (Object[]) bond.get("slaves")); - } - - if (StringUtils.isNotBlank((String) bond.get(VdsProperties.MTU))) { - iface.setMtu(Integer.parseInt((String) bond.get(VdsProperties.MTU))); } Map<String, Object> config = @@ -1440,7 +1427,6 @@ if (config != null && config.get("BONDING_OPTS") != null) { iface.setBondOptions(config.get("BONDING_OPTS").toString()); } - addBootProtocol(config, vds, iface); } } } @@ -1460,17 +1446,10 @@ if (vlans != null) { for (Entry<String, Object> entry : vlans.entrySet()) { VdsNetworkInterface iface = new Vlan(); - VdsNetworkStatistics iStats = new VdsNetworkStatistics(); - iface.setStatistics(iStats); - iStats.setId(Guid.newGuid()); - iface.setId(iStats.getId()); + updateCommonInterfaceData(iface, vds, entry); String vlanDeviceName = entry.getKey(); - iface.setName(vlanDeviceName); - iface.setVdsId(vds.getId()); - Map<String, Object> vlan = (Map<String, Object>) entry.getValue(); - if (vlan.get(VdsProperties.VLAN_ID) != null && vlan.get(VdsProperties.BASE_INTERFACE) != null) { iface.setVlanId((Integer) vlan.get(VdsProperties.VLAN_ID)); iface.setBaseInterface((String) vlan.get(VdsProperties.BASE_INTERFACE)); @@ -1481,14 +1460,6 @@ iface.setBaseInterface(names[0]); } - iface.setAddress((String) vlan.get("addr")); - iface.setSubnet((String) vlan.get("netmask")); - if (StringUtils.isNotBlank((String) vlan.get(VdsProperties.MTU))) { - iface.setMtu(Integer.parseInt((String) vlan.get(VdsProperties.MTU))); - } - - iStats.setVdsId(vds.getId()); - addBootProtocol((Map<String, Object>) vlan.get("cfg"), vds, iface); vds.getInterfaces().add(iface); } } @@ -1507,50 +1478,60 @@ if (nics != null) { for (Entry<String, Object> entry : nics.entrySet()) { VdsNetworkInterface iface = new Nic(); - VdsNetworkStatistics iStats = new VdsNetworkStatistics(); - iface.setStatistics(iStats); - iStats.setId(Guid.newGuid()); - iface.setId(iStats.getId()); - iface.setName(entry.getKey()); - iface.setVdsId(vds.getId()); + updateCommonInterfaceData(iface, vds, entry); - updateNetworkInterfaceDataFromHost(iface, vds, (Map<String, Object>) entry.getValue()); + Map<String, Object> nic = (Map<String, Object>) entry.getValue(); + if (nic != null) { + if (nic.get("speed") != null) { + Object speed = nic.get("speed"); + iface.setSpeed((Integer) speed); + } + iface.setMacAddress((String) nic.get("hwaddr")); + // if we get "permhwaddr", we are a part of a bond and we use that as the mac address + String mac = (String) nic.get("permhwaddr"); + if (mac != null) { + //TODO remove when the minimal supported vdsm version is >=3.6 + // in older VDSM version, slave's Mac is in upper case + iface.setMacAddress(mac.toLowerCase()); + } + } - iStats.setVdsId(vds.getId()); vds.getInterfaces().add(iface); } } } /** - * Updates a given interface by data as collected from the host. + * Updates a given interface (be it physical, bond or VLAN) by data as collected from the host. * * @param iface * The interface to update + * @param vds + * The host to which the interface belongs. * @param nic * A key-value map of the interface properties and their value */ - private static void updateNetworkInterfaceDataFromHost( - VdsNetworkInterface iface, VDS host, Map<String, Object> nic) { + private static void updateCommonInterfaceData(VdsNetworkInterface iface, VDS vds, Entry<String, Object> entry) { + iface.setName(entry.getKey()); + iface.setId(Guid.newGuid()); + iface.setVdsId(vds.getId()); + + VdsNetworkStatistics iStats = new VdsNetworkStatistics(); + iStats.setId(iface.getId()); + iStats.setVdsId(vds.getId()); + iface.setStatistics(iStats); + + Map<String, Object> nic = (Map<String, Object>) entry.getValue(); if (nic != null) { - if (nic.get("speed") != null) { - Object speed = nic.get("speed"); - iface.setSpeed((Integer) speed); - } iface.setAddress((String) nic.get("addr")); iface.setSubnet((String) nic.get("netmask")); - iface.setMacAddress((String) nic.get("hwaddr")); - // if we get "permhwaddr", we are a part of a bond and we use that as the mac address - String mac = (String) nic.get("permhwaddr"); - if (mac != null) { - //TODO remove when the minimal supported vdsm version is >=3.6 - // in older VDSM version, slave's Mac is in upper case - iface.setMacAddress(mac.toLowerCase()); + + String mtu = (String) nic.get(VdsProperties.MTU); + if (StringUtils.isNotBlank(mtu)) { + iface.setMtu(Integer.parseInt(mtu)); } - if (StringUtils.isNotBlank((String) nic.get(VdsProperties.MTU))) { - iface.setMtu(Integer.parseInt((String) nic.get(VdsProperties.MTU))); - } - addBootProtocol((Map<String, Object>) nic.get("cfg"), host, iface); + + addBootProtocol((Map<String, Object>) nic.get("cfg"), vds, iface); } } -- To view, visit http://gerrit.ovirt.org/37078 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I495f36cea0a63a161ca4ae85fc08e67e718befa2 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
