Re: [Kimchi-devel] [PATCH V3] Support Linux Bridge creation

From: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com&gt; This commit adds a new option to bridged network creation API: "macvtap". When it is chosen, the usual behavior for option "bridge" will be applied. This commit also changes the behavior for the already existing "bridge" option. Now, when that option is chosen, a Linux bridge will be created, not a macvtap one. Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com&gt; Signed-off-by: Lucio Correia <luciojhc(a)linux.vnet.ibm.com&gt; V3: - applied code review (including API change) Test case results in ubuntu (no additional error added): MASTER: Ran 108 tests in 340.323s FAILED (failures=2, errors=2, skipped=7) This branch: Ran 110 tests in 344.527s FAILED (failures=2, errors=2, skipped=7) IMPORTANT: This patch only changes backend. UI needs to be adapted in a further patch. --- src/wok/plugins/kimchi/API.json | 2 +- src/wok/plugins/kimchi/docs/API.md | 11 +- src/wok/plugins/kimchi/i18n.py | 5 +- src/wok/plugins/kimchi/model/networks.py | 114 +++++++++++++++------ src/wok/plugins/kimchi/tests/test_mock_network.py | 16 ++- src/wok/plugins/kimchi/tests/test_model_network.py | 2 + src/wok/plugins/kimchi/tests/test_networkxml.py | 24 +++++ src/wok/plugins/kimchi/xmlutils/network.py | 25 +++++ 8 files changed, 161 insertions(+), 38 deletions(-) diff --git a/src/wok/plugins/kimchi/API.json b/src/wok/plugins/kimchi/API.json index 961f35f..5b86ee1 100644 --- a/src/wok/plugins/kimchi/API.json +++ b/src/wok/plugins/kimchi/API.json @@ -342,7 +342,7 @@ "connection": { "description": "Specifies how this network should be connected to the other networks", "type": "string", - "pattern": "^isolated|nat|bridge$", + "pattern": "^isolated|nat|bridge|macvtap$", "required": true, "error": "KCHNET0012E" }, diff --git a/src/wok/plugins/kimchi/docs/API.md b/src/wok/plugins/kimchi/docs/API.md index 52368b7..6cbe913 100644 --- a/src/wok/plugins/kimchi/docs/API.md +++ b/src/wok/plugins/kimchi/docs/API.md @@ -623,13 +623,16 @@ A interface represents available interface on host. networks visible to this host. * isolated: Create a private, isolated virtual network. * nat: Outgoing traffic will be routed through the host. - * bridge: All traffic on this network will be bridged through the indicated - interface. + * macvtap: All traffic on this network will be bridged through the + specified interface. + * bridge: All traffic on this network will be bridged through a Linux + bridged, which is created upon specified interface in case + it is not already a Linux bridge. * subnet *(optional)*: Network segment in slash-separated format with ip address and prefix or netmask used to create nat network. * interface *(optional)*: The name of a network interface on the host. - For bridge network, the interface can be a bridge or nic/bonding - device. + For "macvtap" and "bridge" connections, the + interface can be a nic, bridge or bonding device. * vlan_id *(optional)*: VLAN tagging ID for the bridge network. ### Resource: Network diff --git a/src/wok/plugins/kimchi/i18n.py b/src/wok/plugins/kimchi/i18n.py index 59b61de..8de3a19 100644 --- a/src/wok/plugins/kimchi/i18n.py +++ b/src/wok/plugins/kimchi/i18n.py @@ -242,7 +242,7 @@ messages = { "KCHNET0009E": _("Unable to find a free IP address for network '%(name)s'"), "KCHNET0010E": _("The interface %(iface)s already exists."), "KCHNET0011E": _("Network name must be a string without slashes (/) or quotes (\")"), - "KCHNET0012E": _("Supported network types are isolated, NAT and bridge"), + "KCHNET0012E": _("Supported network types are isolated, NAT, macvtap and bridge"), "KCHNET0013E": _("Network subnet must be a string with IP address and prefix or netmask"), "KCHNET0014E": _("Network interface must be a string"), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), @@ -253,6 +253,9 @@ messages = { "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."), "KCHNET0021E": _("Failed to activate interface %(iface)s. Please check the physical link status."), "KCHNET0022E": _("Failed to start network %(name)s. Details: %(err)s"), + "KCHNET0023E": _("Unable to get XML definition for interface %(name)s. Details: %(err)s"), + "KCHNET0024E": _("Unable to redefine interface %(name)s. Details: %(err)s"), + "KCHNET0025E": _("Unable to create bridge %(name)s. Details: %(err)s"), "KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"), diff --git a/src/wok/plugins/kimchi/model/networks.py b/src/wok/plugins/kimchi/model/networks.py index 71ea595..54cb0ea 100644 --- a/src/wok/plugins/kimchi/model/networks.py +++ b/src/wok/plugins/kimchi/model/networks.py @@ -21,19 +21,21 @@ import ipaddr import libvirt import sys import time +from libvirt import VIR_INTERFACE_XML_INACTIVE from xml.sax.saxutils import escape from wok.config import PluginPaths from wok.exception import InvalidOperation, InvalidParameter from wok.exception import MissingParameter, NotFoundError, OperationFailed -from wok.rollbackcontext import RollbackContext from wok.utils import run_command, wok_log from wok.xmlutils.utils import xpath_get_text from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults +from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml from wok.plugins.kimchi.xmlutils.network import create_vlan_tagged_bridge_xml +from wok.plugins.kimchi.xmlutils.network import get_no_network_config_xml from wok.plugins.kimchi.xmlutils.network import to_network_xml @@ -90,7 +92,7 @@ class NetworksModel(object): self._set_network_subnet(params) # only bridge network need bridge(linux bridge) or interface(macvtap) - if connection == 'bridge': + if connection in ['bridge', 'macvtap']: self._set_network_bridge(params) params['name'] = escape(params['name']) @@ -159,6 +161,7 @@ class NetworksModel(object): def _set_network_bridge(self, params): try: + # fails if host interface is already in use by a libvirt network iface = params['interface'] if iface in self.get_all_networks_interfaces(): msg_args = {'iface': iface, 'network': params['name']} @@ -166,11 +169,23 @@ class NetworksModel(object): except KeyError: raise MissingParameter("KCHNET0004E", {'name': params['name']}) + # Linux bridges cannot be the trunk device of a VLAN + if 'vlan_id' in params and \ + (netinfo.is_bridge(iface) or params['connection'] == "bridge"): + raise InvalidParameter('KCHNET0019E', {'name': iface}) + + # User specified bridge interface, simply use it self._ensure_iface_up(iface) if netinfo.is_bridge(iface): - if 'vlan_id' in params: - raise InvalidParameter('KCHNET0019E', {'name': iface}) params['bridge'] = iface + + # User wants Linux bridge network, but didn't specify bridge interface + elif params['connection'] == "bridge": + # create Linux bridge interface first and use it as actual iface + iface = self._create_linux_bridge(iface) + params['bridge'] = iface + + # connection == macvtap and iface is not bridge elif netinfo.is_bare_nic(iface) or netinfo.is_bonding(iface): if params.get('vlan_id') is None: params['forward']['dev'] = iface @@ -190,42 +205,79 @@ class NetworksModel(object): xml = network.XMLDesc(0) net_dict = NetworkModel.get_network_from_xml(xml) forward = net_dict['forward'] - (forward['mode'] == 'bridge' and forward['interface'] and + (forward['mode'] == 'macvtap' and forward['interface'] and interfaces.append(forward['interface'][0]) is None or interfaces.extend(forward['interface'] + forward['pf'])) net_dict['bridge'] and interfaces.append(net_dict['bridge']) return interfaces + def _create_bridge(self, name, xml): + conn = self.conn.get() + + # check if name exists + if name in netinfo.all_interfaces(): + raise InvalidOperation("KCHNET0010E", {'iface': name}) + + # create bridge through libvirt + try: + bridge = conn.interfaceDefineXML(xml) + bridge.create() + except libvirt.libvirtError as e: + raise OperationFailed("KCHNET0025E", {'name': name, + 'err': e.get_error_message()}) + + def _create_linux_bridge(self, interface): + # get xml definition of interface + iface_xml = self._get_interface_desc_xml(interface) + + # Truncate the interface name if it exceeds 13 characters to make sure + # the length of bridge name is less than 15 (its maximum value). + br_name = KIMCHI_BRIDGE_PREFIX + interface[-13:] + br_xml = create_linux_bridge_xml(br_name, interface, iface_xml) + + # drop network config from interface + self._redefine_iface_no_network(interface) + + # create and start bridge + self._create_bridge(br_name, br_xml) + + return br_name + def _create_vlan_tagged_bridge(self, interface, vlan_id): # Truncate the interface name if it exceeds 8 characters to make sure # the length of bridge name is less than 15 (its maximum value). br_name = KIMCHI_BRIDGE_PREFIX + interface[-8:] + '-' + vlan_id br_xml = create_vlan_tagged_bridge_xml(br_name, interface, vlan_id) + + self._create_bridge(br_name, br_xml) + + return br_name + + def _get_interface_desc_xml(self, name): conn = self.conn.get() - bridges = [] - for net in conn.listAllNetworks(): - # Bridged networks do not have a bridge name - # So in those cases, libvirt raises an error when trying to get - # the bridge name - try: - bridges.append(net.bridgeName()) - except libvirt.libvirtError, e: - wok_log.error(e.message) + try: + iface = conn.interfaceLookupByName(name) + xml = iface.XMLDesc(flags=VIR_INTERFACE_XML_INACTIVE) + except libvirt.libvirtError, e: + raise OperationFailed("KCHNET0023E", + {'name': name, 'err': e.get_error_message()}) - if br_name in bridges: - raise InvalidOperation("KCHNET0010E", {'iface': br_name}) + return xml - with RollbackContext() as rollback: - try: - vlan_tagged_br = conn.interfaceDefineXML(br_xml, 0) - rollback.prependDefer(vlan_tagged_br.destroy) - vlan_tagged_br.create(0) - except libvirt.libvirtError as e: - raise OperationFailed(e.message) - else: - return br_name + def _redefine_iface_no_network(self, name): + conn = self.conn.get() + # drop network config from definition of interface + iface_xml = self._get_interface_desc_xml(name) + xml = get_no_network_config_xml(iface_xml.encode("utf-8")) + + try: + # redefine interface + iface = conn.interfaceDefineXML(xml.encode("utf-8")) + except libvirt.libvirtError as e: + raise OperationFailed("KCHNET0024E", {'name': name, + 'err': e.get_error_message()}) class NetworkModel(object): def __init__(self, **kargs): @@ -243,10 +295,12 @@ class NetworkModel(object): connection = forward['mode'] or "isolated" # FIXME, if we want to support other forward mode well. - if connection == 'bridge': - # macvtap bridge + # macvtap + if connection == 'macvtap': interface = interface or forward['interface'][0] - # exposing the network on linux bridge or macvtap interface + + # exposing the network on Linux bridge or macvtap interface + if connection in ['bridge', 'macvtap']: interface_subnet = knetwork.get_dev_netaddr(interface) subnet = subnet if subnet else interface_subnet @@ -333,7 +387,7 @@ class NetworkModel(object): if network.isActive(): raise InvalidOperation("KCHNET0005E", {'name': name}) - self._remove_vlan_tagged_bridge(network) + self._remove_bridge(network) network.undefine() @staticmethod @@ -369,7 +423,7 @@ class NetworkModel(object): 'interface': forward_if, 'pf': forward_pf}} - def _remove_vlan_tagged_bridge(self, network): + def _remove_bridge(self, network): try: bridge = network.bridgeName() except libvirt.libvirtError: diff --git a/src/wok/plugins/kimchi/tests/test_mock_network.py b/src/wok/plugins/kimchi/tests/test_mock_network.py index 4e2a939..e47f8ec 100644 --- a/src/wok/plugins/kimchi/tests/test_mock_network.py +++ b/src/wok/plugins/kimchi/tests/test_mock_network.py @@ -68,6 +68,18 @@ class MockNetworkTests(unittest.TestCase): ) if len(interfaces) > 0: iface = interfaces[0]['name'] - _do_network_test(self, model, {'name': u'bridge-network', - 'connection': 'bridge', + _do_network_test(self, model, {'name': u'macvtap-network', + 'connection': 'macvtap', 'interface': iface, 'vlan_id': 987}) + + def test_linux_bridge(self): + # Verify the current system has at least one interface to create a + # bridged network + interfaces = json.loads( + self.request('/plugins/kimchi/interfaces?type=nic').read() + ) + if len(interfaces) > 0: + iface = interfaces[0]['name'] + _do_network_test(self, model, {'name': u'bridged-network', + 'connection': 'bridge', + 'interface': iface}) diff --git a/src/wok/plugins/kimchi/tests/test_model_network.py b/src/wok/plugins/kimchi/tests/test_model_network.py index e4cf5ef..aad8a95 100644 --- a/src/wok/plugins/kimchi/tests/test_model_network.py +++ b/src/wok/plugins/kimchi/tests/test_model_network.py @@ -142,6 +142,8 @@ class NetworkTests(unittest.TestCase): ) if len(interfaces) > 0: iface = interfaces[0]['name'] + networks.append({'name': u'macvtap-network', + 'connection': 'macvtap', 'interface': iface}) networks.append({'name': u'bridge-network', 'connection': 'bridge', 'interface': iface}) diff --git a/src/wok/plugins/kimchi/tests/test_networkxml.py b/src/wok/plugins/kimchi/tests/test_networkxml.py index a64b6c2..e9485a8 100644 --- a/src/wok/plugins/kimchi/tests/test_networkxml.py +++ b/src/wok/plugins/kimchi/tests/test_networkxml.py @@ -170,3 +170,27 @@ class InterfaceXmlTests(unittest.TestCase): """ actual_xml = nxml.create_vlan_tagged_bridge_xml('br10', 'em1', '10') self.assertEquals(actual_xml, utils.normalize_xml(expected_xml)) + + def test_linux_bridge_no_ip(self): + em1_xml = """ + <interface type='ethernet' name='em1'> + <start mode='onboot'/> + <protocol family='ipv4'> + <dhcp/> + </protocol> + </interface> + """ + expected_xml = """ + <interface type='bridge' name='br10'> + <start mode='onboot'/> + <bridge> + <interface type='ethernet' name='em1' /> + </bridge> + <protocol family='ipv4'> + <dhcp/> + </protocol> + </interface> + """ + actual_xml = nxml.create_linux_bridge_xml('br10', 'em1', + utils.normalize_xml(em1_xml)) + self.assertEquals(actual_xml, utils.normalize_xml(expected_xml)) diff --git a/src/wok/plugins/kimchi/xmlutils/network.py b/src/wok/plugins/kimchi/xmlutils/network.py index c73aad9..14c7e13 100644 --- a/src/wok/plugins/kimchi/xmlutils/network.py +++ b/src/wok/plugins/kimchi/xmlutils/network.py @@ -120,3 +120,28 @@ def create_vlan_tagged_bridge_xml(bridge, interface, vlan_id): type='bridge', name=bridge) return ET.tostring(m) + +def create_linux_bridge_xml(bridge, interface, iface_xml): + m = E.interface( + E.start(mode='onboot'), + E.bridge( + E.interface( + type='ethernet', + name=interface)), + type='bridge', + name=bridge) + + # use same network configuration of lower interface + iface = ET.fromstring(iface_xml) + for element in iface.iter("protocol"): + m.append(element) + + return ET.tostring(m) + +def get_no_network_config_xml(iface_xml): + # remove all protocol elements from interface xml + xml = ET.fromstring(iface_xml) + for element in xml.iter("protocol"): + element.getparent().remove(element) + + return ET.tostring(xml)