From e2cc2c1f6ec4bdcc9addf0c6893e29385fcdea61 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 19 Mar 2012 16:59:21 +0000 Subject: [PATCH] Fixing remaining issues with per-VIF flow script and removing version-specific scripts. Now generating XSnetwork names using gre keys Plus other minor corrections --- .../network/ovs/OvsDestroyBridgeCommand.java | 12 ++- .../network/ovs/OvsDestroyTunnelCommand.java | 13 ++- .../xen/resource/CitrixResourceBase.java | 32 ++++---- .../{xenserver56fp1 => }/ovs-vif-flows.py | 8 +- scripts/vm/hypervisor/xenserver/ovstunnel | 6 +- .../hypervisor/xenserver/xenserver56fp1/patch | 2 +- .../xenserver/xenserver60/ovs-vif-flows.py | 79 ------------------- .../vm/hypervisor/xenserver/xenserver60/patch | 2 +- .../network/ovs/OvsTunnelManagerImpl.java | 49 +++++++----- .../cloud/resource/ResourceManagerImpl.java | 6 +- 10 files changed, 80 insertions(+), 129 deletions(-) rename scripts/vm/hypervisor/xenserver/{xenserver56fp1 => }/ovs-vif-flows.py (94%) delete mode 100644 scripts/vm/hypervisor/xenserver/xenserver60/ovs-vif-flows.py diff --git a/api/src/com/cloud/network/ovs/OvsDestroyBridgeCommand.java b/api/src/com/cloud/network/ovs/OvsDestroyBridgeCommand.java index 11f762bafa8..689e03eca5b 100644 --- a/api/src/com/cloud/network/ovs/OvsDestroyBridgeCommand.java +++ b/api/src/com/cloud/network/ovs/OvsDestroyBridgeCommand.java @@ -21,16 +21,22 @@ import com.cloud.agent.api.Command; public class OvsDestroyBridgeCommand extends Command { - long networkId; + Long networkId; + Integer key; - public OvsDestroyBridgeCommand(long networkId) { + public OvsDestroyBridgeCommand(Long networkId, Integer key) { this.networkId = networkId; + this.key = key; } - public long getNetworkId() { + public Long getNetworkId() { return networkId; } + public Integer getKey() { + return key; + } + @Override public boolean executeInSequence() { return true; diff --git a/api/src/com/cloud/network/ovs/OvsDestroyTunnelCommand.java b/api/src/com/cloud/network/ovs/OvsDestroyTunnelCommand.java index 53fc4ceeb44..e5a80953fac 100644 --- a/api/src/com/cloud/network/ovs/OvsDestroyTunnelCommand.java +++ b/api/src/com/cloud/network/ovs/OvsDestroyTunnelCommand.java @@ -15,15 +15,18 @@ package com.cloud.network.ovs; import com.cloud.agent.api.Command; public class OvsDestroyTunnelCommand extends Command { - long networkId; + + Long networkId; + Integer key; String inPortName; - public OvsDestroyTunnelCommand(long networkId, String inPortName) { + public OvsDestroyTunnelCommand(Long networkId, Integer key, String inPortName) { this.networkId = networkId; this.inPortName = inPortName; + this.key = key; } - public long getNetworkId() { + public Long getNetworkId() { return networkId; } @@ -31,6 +34,10 @@ public class OvsDestroyTunnelCommand extends Command { return inPortName; } + public Integer getKey() { + return key; + } + @Override public boolean executeInSequence() { return true; diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index ec8ded7e40d..e5af2c4a076 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -620,23 +620,21 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe /** * This method just creates a XenServer network following the tunnel network naming convention */ - private synchronized Network findOrCreateTunnelNetwork(Connection conn, long networkId) { + private synchronized Network findOrCreateTunnelNetwork(Connection conn, long vnetId) { try { - String nwName = "OVSTunnel" + networkId; + String nwName = "OVSTunnel" + vnetId; Network nw = null; Network.Record rec = new Network.Record(); Set networks = Network.getByNameLabel(conn, nwName); if (networks.size() == 0) { - rec.nameDescription = "tunnel network id# " + networkId; + rec.nameDescription = "tunnel network id# " + vnetId; rec.nameLabel = nwName; //Initialize the ovs-host-setup to avoid error when doing get-param in plugin Map otherConfig = new HashMap(); otherConfig.put("ovs-host-setup", ""); rec.otherConfig = otherConfig; nw = Network.create(conn, rec); - // Plug dom0 vif only when creating network - enableXenServerNetwork(conn, nw, nwName, "tunnel network for account " + networkId); s_logger.debug("### Xen Server network for tunnels created:" + nwName); } else { nw = networks.iterator().next(); @@ -654,7 +652,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe */ private synchronized Network configureTunnelNetwork(Connection conn, long networkId, long hostId, int key) { try { - Network nw = findOrCreateTunnelNetwork(conn, networkId); + // Note: the vnet (or gre key) is used to identify the XS network + Network nw = findOrCreateTunnelNetwork(conn, key); + String nwName = "OVSTunnel" + key; //Invoke plugin to setup the bridge which will be used by this network String bridge = nw.getBridge(conn); Map nwOtherConfig = nw.getOtherConfig(conn); @@ -670,6 +670,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } if (!configured) { + // Plug dom0 vif only if not done before for network and host + enableXenServerNetwork(conn, nw, nwName, "tunnel network for account " + key); String result = callHostPlugin(conn, "ovstunnel", "setup_ovs_bridge", "bridge", bridge, "key", String.valueOf(key), "xs_nw_uuid", nw.getUuid(conn), @@ -689,9 +691,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } - private synchronized void destroyTunnelNetwork(Connection conn, long networkId) { + private synchronized void destroyTunnelNetwork(Connection conn, int key) { try { - Network nw = findOrCreateTunnelNetwork(conn, networkId); + Network nw = findOrCreateTunnelNetwork(conn, key); String bridge = nw.getBridge(conn); String result = callHostPlugin(conn, "ovstunnel", "destroy_ovs_bridge", "bridge", bridge); String[] res = result.split(":"); @@ -731,8 +733,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe _isOvs = true; return setupvSwitchNetwork(conn); } else { - long networkId = Long.parseLong(nic.getBroadcastUri().getHost()); - return findOrCreateTunnelNetwork(conn, networkId); + long vnetId = Long.parseLong(nic.getBroadcastUri().getHost()); + return findOrCreateTunnelNetwork(conn, vnetId); } } else if (nic.getBroadcastType() == BroadcastDomainType.Storage) { URI broadcastUri = nic.getBroadcastUri(); @@ -1196,7 +1198,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe startVM(conn, host, vm, vmName); if (_isOvs) { - // TODO(Salvatore-orlando): First option is to do per-NIC rules here + // TODO(Salvatore-orlando): This code should go for (NicTO nic : vmSpec.getNics()) { if (nic.getBroadcastType() == Networks.BroadcastDomainType.Vswitch) { HashMap args = parseDefaultOvsRuleComamnd(nic.getBroadcastUri().toString().substring(Networks.BroadcastDomainType.Vswitch.scheme().length() + "://".length())); @@ -4791,7 +4793,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe private Answer execute(OvsSetupBridgeCommand cmd) { Connection conn = getConnection(); s_logger.debug("### About to configure OVS bridge"); - Network nw=findOrCreateTunnelNetwork(conn, cmd.getNetworkId()); + Network nw=findOrCreateTunnelNetwork(conn, cmd.getKey()); this.configureTunnelNetwork(conn, cmd.getNetworkId(), cmd.getHostId(), cmd.getKey()); s_logger.debug("### Bridge configured"); return new Answer(cmd, true, null); @@ -4800,7 +4802,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe private Answer execute(OvsDestroyBridgeCommand cmd) { Connection conn = getConnection(); s_logger.debug("### About to destroy OVS bridge"); - destroyTunnelNetwork(conn, cmd.getNetworkId()); + destroyTunnelNetwork(conn, cmd.getKey()); s_logger.debug("### Bridge destroyed"); return new Answer(cmd, true, null); } @@ -4809,7 +4811,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe Connection conn = getConnection(); s_logger.debug("### About to destroy tunnel network"); try { - Network nw = findOrCreateTunnelNetwork(conn, cmd.getNetworkId()); + Network nw = findOrCreateTunnelNetwork(conn, cmd.getKey()); if (nw == null) { s_logger.warn("### Unable to find tunnel network"); return new Answer(cmd, false, "No network found"); @@ -4841,7 +4843,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe String bridge = "unknown"; try { s_logger.debug("### About to create tunnel network"); - Network nw = findOrCreateTunnelNetwork(conn, cmd.getNetworkId()); + Network nw = findOrCreateTunnelNetwork(conn, cmd.getKey()); if (nw == null) { s_logger.debug("### SOMETHING WENT WRONG DURING NETWORK SETUP"); return new OvsCreateTunnelAnswer(cmd, false, "Cannot create network", bridge); diff --git a/scripts/vm/hypervisor/xenserver/xenserver56fp1/ovs-vif-flows.py b/scripts/vm/hypervisor/xenserver/ovs-vif-flows.py similarity index 94% rename from scripts/vm/hypervisor/xenserver/xenserver56fp1/ovs-vif-flows.py rename to scripts/vm/hypervisor/xenserver/ovs-vif-flows.py index e2d7b423f4e..0cfe4422228 100644 --- a/scripts/vm/hypervisor/xenserver/xenserver56fp1/ovs-vif-flows.py +++ b/scripts/vm/hypervisor/xenserver/ovs-vif-flows.py @@ -65,8 +65,7 @@ def main(command, vif_raw): # We need the REAL bridge name bridge = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'br-to-parent', bridge]) - # For the OVS version shipped with XS56FP1 we need to retrieve - # the ofport number for all interfaces + vsctl_output = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'list-ports', bridge]) vifs = vsctl_output.split('\n') @@ -75,8 +74,9 @@ def main(command, vif_raw): vif_ofport = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'get', 'Interface', vif, 'ofport']) if this_vif == vif: - this_vif_ofport = vif_ofport - vif_ofports.append(vif_ofport) + this_vif_ofport = vif_ofport + if vif.startswith('vif'): + vif_ofports.append(vif_ofport) if command == 'offline': clear_flows(bridge, this_vif_ofport, vif_ofports) diff --git a/scripts/vm/hypervisor/xenserver/ovstunnel b/scripts/vm/hypervisor/xenserver/ovstunnel index d900477d928..0790e1f68c3 100755 --- a/scripts/vm/hypervisor/xenserver/ovstunnel +++ b/scripts/vm/hypervisor/xenserver/ovstunnel @@ -144,7 +144,7 @@ def setup_ovs_bridge(session, args): "uuid=%s" % xs_nw_uuid, "param-name=other-config", "param-key=ovs-host-setup", "--minimal"]) - conf_hosts = conf_hosts + ",%s" %cs_host_id + conf_hosts = cs_host_id + (conf_hosts and ',%s' % conf_hosts or '') lib.do_cmd([lib.XE_PATH,"network-param-set", "uuid=%s" % xs_nw_uuid, "other-config:ovs-host-setup=%s" %conf_hosts]) @@ -176,8 +176,8 @@ def destroy_ovs_bridge(session, args): # Note that the bridge has been removed on xapi network object xs_nw_uuid = lib.do_cmd([xePath, "network-list", "bridge=%s" % bridge, "--minimal"]) - lib.do_cmd([xePath,"network-param-set", "uuid=%s" % xs_nw_uuid, - "other-config:ovs-setup=False"]) + #lib.do_cmd([xePath,"network-param-set", "uuid=%s" % xs_nw_uuid, + # "other-config:ovs-setup=False"]) result = "SUCCESS:%s" %bridge logging.debug("Destroy_ovs_bridge completed with result:%s" %result) diff --git a/scripts/vm/hypervisor/xenserver/xenserver56fp1/patch b/scripts/vm/hypervisor/xenserver/xenserver56fp1/patch index bca65cbdfc5..88d33c8b2df 100644 --- a/scripts/vm/hypervisor/xenserver/xenserver56fp1/patch +++ b/scripts/vm/hypervisor/xenserver/xenserver56fp1/patch @@ -12,7 +12,7 @@ NFSSR.py=/opt/xensource/sm vmops=..,0755,/etc/xapi.d/plugins xen-ovs-vif-flows.rules=..,0644,/etc/udev/rules.d -ovs-vif-flows.py=.,0755,/etc/xapi.d/plugins +ovs-vif-flows.py=..,0755,/etc/xapi.d/plugins cloudstack_plugins.conf=..,0644,/etc/xensource cloudstack_pluginlib.py=..,0755,/etc/xapi.d/plugins ovsgre=..,0755,/etc/xapi.d/plugins diff --git a/scripts/vm/hypervisor/xenserver/xenserver60/ovs-vif-flows.py b/scripts/vm/hypervisor/xenserver/xenserver60/ovs-vif-flows.py deleted file mode 100644 index 1e2ab230f17..00000000000 --- a/scripts/vm/hypervisor/xenserver/xenserver60/ovs-vif-flows.py +++ /dev/null @@ -1,79 +0,0 @@ -# A simple script for enabling and disabling per-vif rules for explicitly -# allowing broadcast/multicast traffic on the port where the VIF is attached -# -# Copyright (C) 2012 Citrix Systems - -import os -import sys - -import cloudstack_pluginlib as pluginlib - - -def clear_flows(bridge, vif_ofport): - # Remove flow entries originating from given ofport - pluginlib.del_flows(bridge, in_port=vif_ofport) - # Leverage out_port option for removing flows based on actions - pluginlib.del_flows(bridge, out_port=vif_ofport) - - -def apply_flows(bridge, vif_ofport): - action = "output:%s," % vif_ofport - # Ensure {b|m}casts sent from VIF ports are always allowed - pluginlib.add_flow(bridge, priority=1200, - in_port=vif_ofport, - dl_dst='ff:ff:ff:ff:ff:ff', - actions='NORMAL') - pluginlib.add_flow(bridge, priority=1200, - in_port=vif_ofport, - nw_dst='224.0.0.0/24', - actions='NORMAL') - # Ensure {b|m}casts are always propagated to VIF ports - pluginlib.add_flow(bridge, priority=1100, - dl_dst='ff:ff:ff:ff:ff:ff', actions=action) - pluginlib.add_flow(bridge, priority=1100, - nw_dst='224.0.0.0/24', actions=action) - - -def main(command, vif_raw): - if command not in ('online', 'offline'): - return - # TODO (very important) - # Quit immediately if networking is NOT being managed by the OVS tunnel manager - vif_name, dom_id, vif_index = vif_raw.split('-') - # validate vif and dom-id - vif = "%s%s.%s" % (vif_name, dom_id, vif_index) - - bridge = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'iface-to-br', vif]) - # find xs network for this bridge, verify is used for ovs tunnel network - xs_nw_uuid = pluginlib.do_cmd([pluginlib.XE_PATH, "network-list", - "bridge=%s" % bridge, "--minimal"]) - result = pluginlib.do_cmd([pluginlib.XE_PATH,"network-param-get", - "uuid=%s" % xs_nw_uuid, - "param-name=other-config", - "param-key=is-ovs-tun-network", "--minimal"]) - - if result != 'True': - return - - vlan = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'br-to-vlan', bridge]) - if vlan != '0': - # We need the REAL bridge name - bridge = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'br-to-parent', bridge]) - vif_ofport = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'get', 'Interface', - vif, 'ofport']) - - if command == 'offline': - clear_flows(bridge, vif_ofport) - - if command == 'online': - apply_flows(bridge, vif_ofport) - - -if __name__ == "__main__": - if len(sys.argv) != 3: - print "usage: %s [online|offline] vif-domid-idx" % \ - os.path.basename(sys.argv[0]) - sys.exit(1) - else: - command, vif_raw = sys.argv[1:3] - main(command, vif_raw) \ No newline at end of file diff --git a/scripts/vm/hypervisor/xenserver/xenserver60/patch b/scripts/vm/hypervisor/xenserver/xenserver60/patch index b75596b3adb..bb25ee23b16 100644 --- a/scripts/vm/hypervisor/xenserver/xenserver60/patch +++ b/scripts/vm/hypervisor/xenserver/xenserver60/patch @@ -12,7 +12,7 @@ NFSSR.py=/opt/xensource/sm vmops=..,0755,/etc/xapi.d/plugins xen-ovs-vif-flows.rules=..,0644,/etc/udev/rules.d -ovs-vif-flows.py=.,0755,/etc/xapi.d/plugins +ovs-vif-flows.py=..,0755,/etc/xapi.d/plugins cloudstack_plugins.conf=..,0644,/etc/xensource cloudstack_pluginlib.py=..,0755,/etc/xapi.d/plugins ovsgre=..,0755,/etc/xapi.d/plugins diff --git a/server/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java b/server/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java index f01184f67cf..c3e8c4573d1 100644 --- a/server/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java +++ b/server/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java @@ -129,6 +129,24 @@ public class OvsTunnelManagerImpl implements OvsTunnelManager { _tunnelNetworkDao.update(tunnel.getId(), tunnel); } + private int getGreKey(Network network) { + int key = 0; + try { + //The GRE key is actually in the host part of the URI + String keyStr = network.getBroadcastUri().getHost(); + // The key is most certainly and int. + // So we feel quite safe in converting it into a string + key = Integer.valueOf(keyStr); + return key; + } catch (NumberFormatException e) { + s_logger.debug("Well well, how did '" + key + + "' end up in the broadcast URI for the network?"); + throw new CloudRuntimeException( + String.format("Invalid GRE key parsed from network broadcast URI (%s)", + network.getBroadcastUri().toString())); + } + } + @DB protected void CheckAndCreateTunnel(VirtualMachine instance, Network nw, DeployDestination dest) { if (!_isEnabled) { @@ -143,18 +161,7 @@ public class OvsTunnelManagerImpl implements OvsTunnelManager { } long hostId = dest.getHost().getId(); - int key = 0; - try { - //The GRE key is actually in the host part of the URI - String keyStr = nw.getBroadcastUri().getHost(); - // The key is most certainly and int. - // So we feel quite safe in converting it into a string - key = Integer.valueOf(keyStr); - } catch (NumberFormatException e) { - s_logger.debug("Well well, how did '" + key + - "' end up in the broadcast URI for the network?"); - s_logger.warn("Unable to create GRE tunnels on host:" + hostId); - } + int key = getGreKey(nw); // Find active (i.e.: not shut off) VMs with a NIC on the target network List vms = _userVmDao.listByNetworkIdAndStates(nw.getId(), State.Running, State.Starting, State.Stopping, State.Unknown, State.Migrating); @@ -198,7 +205,7 @@ public class OvsTunnelManagerImpl implements OvsTunnelManager { } } } - + //FIXME: Why are we cancelling the exception here? try { String myIp = dest.getHost().getPrivateIpAddress(); boolean noHost = true; @@ -337,7 +344,8 @@ public class OvsTunnelManagerImpl implements OvsTunnelManager { try { /* Now we are last one on host, destroy the bridge with all * the tunnels for this network */ - Command cmd = new OvsDestroyBridgeCommand(nw.getId()); + int key = getGreKey(nw); + Command cmd = new OvsDestroyBridgeCommand(nw.getId(), key); s_logger.debug("### Destroying bridge for network " + nw.getId() + " on host:" + vm.getHostId()); Answer ans = _agentMgr.send(vm.getHostId(), cmd); handleDestroyBridgeAnswer(ans, vm.getHostId(), nw.getId()); @@ -345,11 +353,14 @@ public class OvsTunnelManagerImpl implements OvsTunnelManager { /* Then ask hosts have peer tunnel with me to destroy them */ List peers = _tunnelNetworkDao.listByToNetwork(vm.getHostId(), nw.getId()); for (OvsTunnelNetworkVO p : peers) { - cmd = new OvsDestroyTunnelCommand(p.getNetworkId(), p.getPortName()); - s_logger.debug("### Destroying tunnel to " + vm.getHostId() + - " from " + p.getFrom()); - ans = _agentMgr.send(p.getFrom(), cmd); - handleDestroyTunnelAnswer(ans, p.getFrom(), p.getTo(), p.getNetworkId()); + // If the tunnel was not successfully created don't bother to remove it + if (p.getState().equals("SUCCESS")) { + cmd = new OvsDestroyTunnelCommand(p.getNetworkId(), key, p.getPortName()); + s_logger.debug("### Destroying tunnel to " + vm.getHostId() + + " from " + p.getFrom()); + ans = _agentMgr.send(p.getFrom(), cmd); + handleDestroyTunnelAnswer(ans, p.getFrom(), p.getTo(), p.getNetworkId()); + } } } catch (Exception e) { s_logger.warn(String.format("Destroy tunnel(account:%1$s, hostId:%2$s) failed", vm.getAccountId(), vm.getHostId()), e); diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 99f95578158..651e37053bc 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -573,6 +573,10 @@ public class ResourceManagerImpl implements ResourceManager, ResourceService, Ma if (pod == null) { throw new InvalidParameterValueException("Can't find pod by id " + podId); } + HostPodVO pod = _podDao.findById(podId); + if (pod == null) { + throw new InvalidParameterValueException("Can't find pod with specified podId " + podId); + } // check if pod belongs to the zone if (!Long.valueOf(pod.getDataCenterId()).equals(dcId)) { InvalidParameterValueException ex = new InvalidParameterValueException("Pod with specified podId" + podId + " doesn't belong to the zone with specified zoneId" + dcId); @@ -625,7 +629,7 @@ public class ResourceManagerImpl implements ResourceManager, ResourceService, Ma if (pod == null) { throw new InvalidParameterValueException("Can't find pod by id " + podId); } - ClusterVO cluster = new ClusterVO(dcId, podId, clusterName); + ClusterVO cluster = new ClusterVO(dcId, podId, clusterName); cluster.setHypervisorType(hypervisorType); try { cluster = _clusterDao.persist(cluster);