[Kimchi-devel] [PATCH V3] Support Linux Bridge creation
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Nov 16 21:23:16 UTC 2015
Reviewed-by: Aline Manera <alinefm at linux.vnet.ibm.com>
On 13/11/2015 18:38, Lucio Correia wrote:
> From: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
>
> 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 at linux.vnet.ibm.com>
> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>
>
> 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)
More information about the Kimchi-devel
mailing list