[Kimchi-devel] [PATCH] [Kimchi] Fix handling of VLANs in backend
Ramon Medeiros
ramonn at linux.vnet.ibm.com
Wed Feb 10 16:41:02 UTC 2016
On 02/10/2016 02:38 PM, Lucio Correia wrote:
> - Allow VLANs only for bridged virtual networks
> - Code simplification
>
> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
> ---
> i18n.py | 6 ++---
> model/networks.py | 71 ++++++++++++++++++++++++++++++-------------------------
> 2 files changed, 42 insertions(+), 35 deletions(-)
>
> diff --git a/i18n.py b/i18n.py
> index 0b9fa39..bd36efa 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -248,8 +248,8 @@ messages = {
>
> "KCHNET0001E": _("Network %(name)s already exists"),
> "KCHNET0002E": _("Network %(name)s does not exist"),
> - "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."),
> - "KCHNET0004E": _("Specify a network interface to create bridged network %(name)s"),
> + "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."),
> + "KCHNET0004E": _("Specify a network interface to create bridged or macvtap networks."),
> "KCHNET0005E": _("Unable to delete active network %(name)s"),
> "KCHNET0006E": _("Interface %(iface)s specified for network %(network)s is already in use"),
> "KCHNET0007E": _("Interface should be bare NIC, bonding or bridge device."),
> @@ -270,8 +270,8 @@ messages = {
> "KCHNET0022E": _("Failed to start network %(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"),
> - "KCHNET0026E": _("Open VSwitch bridges can only host bridged networks."),
> "KCHNET0027E": _("Unable to create bridge with NetworkManager enabled. Disable it and try again."),
> + "KCHNET0028E": _("Interface should be bare NIC or bonding."),
>
> "KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
>
> diff --git a/model/networks.py b/model/networks.py
> index fba181f..9a49634 100644
> --- a/model/networks.py
> +++ b/model/networks.py
> @@ -86,21 +86,20 @@ class NetworksModel(object):
> if name in self.get_list():
> raise InvalidOperation("KCHNET0001E", {'name': name})
>
> + # handle connection type
> connection = params["connection"]
> - # set forward mode, isolated do not need forward
> if connection == 'macvtap':
> - params['forward'] = {'mode': 'bridge'}
> - elif connection != 'isolated':
> - params['forward'] = {'mode': connection}
> + self._set_network_macvtap(params)
> + elif connection == 'bridge':
> + self._set_network_bridge(params)
> + elif connection in ['nat', 'isolated']:
> + if connection == 'nat':
> + params['forward'] = {'mode': 'nat'}
Just saw that you use a lot of strings like 'nat', 'bridge'. What about
put them as constant?
>
> - # set subnet, bridge network do not need subnet
> - if connection in ["nat", 'isolated']:
> + # set subnet; bridge/macvtap networks do not need subnet
> self._set_network_subnet(params)
>
> - # only bridge network need bridge(linux bridge) or interface(macvtap)
> - if connection in ['bridge', 'macvtap']:
> - self._set_network_bridge(params)
> -
> + # create network XML
> params['name'] = escape(params['name'])
> xml = to_network_xml(**params)
>
> @@ -165,7 +164,7 @@ class NetworksModel(object):
> else:
> raise OperationFailed("KCHNET0021E", {'iface': iface})
>
> - def _set_network_bridge(self, params):
> + def _check_network_interface(self, params):
> try:
> # fails if host interface is already in use by a libvirt network
> iface = params['interface']
> @@ -175,9 +174,24 @@ 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"):
> + def _set_network_macvtap(self, params):
> + self._check_network_interface(params)
> +
> + iface = params['interface']
> + if 'vlan_id' in params or not (netinfo.is_bare_nic(iface) or
> + netinfo.is_bonding(iface)):
> + raise InvalidParameter('KCHNET0028E', {'name': iface})
> +
> + # set macvtap network
> + params['forward'] = {'mode': 'bridge', 'dev': iface}
> +
> + def _set_network_bridge(self, params):
> + self._check_network_interface(params)
> + params['forward'] = {'mode': 'bridge'}
> +
> + # Bridges cannot be the trunk device of a VLAN
> + iface = params['interface']
> + if 'vlan_id' in params and netinfo.is_bridge(iface):
> raise InvalidParameter('KCHNET0019E', {'name': iface})
>
> # User specified bridge interface, simply use it
> @@ -189,29 +203,22 @@ class NetworksModel(object):
> if netinfo.is_ovs_bridge(iface):
> params['ovs'] = True
>
> - # OVS bridges don't work with macvtap
> - if params['connection'] != "bridge":
> - raise InvalidParameter('KCHNET0026E')
> -
> - # User wants Linux bridge network, but didn't specify bridge interface
> - elif params['connection'] == "bridge":
> -
> - # libvirt will fail to create bridge if NetworkManager is enabled
> - if self.caps.nm_running:
> - raise InvalidParameter('KCHNET0027E')
> -
> - # 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
> - else:
> + if 'vlan_id' in params:
> params['bridge'] = \
> self._create_vlan_tagged_bridge(str(iface),
> str(params['vlan_id']))
> + else:
> + # libvirt bridge creation will fail with NetworkManager enabled
> + if self.caps.nm_running:
> + raise InvalidParameter('KCHNET0027E')
> +
> + # create Linux bridge interface and use it as actual iface
> + iface = self._create_linux_bridge(iface)
> + params['bridge'] = iface
> +
> + # unrecognized interface type: fail
> else:
> raise InvalidParameter("KCHNET0007E")
>
--
Ramon Nunes Medeiros
Kimchi Developer
Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ramonn at br.ibm.com
More information about the Kimchi-devel
mailing list