[Kimchi-devel] [PATCH] [Kimchi] Fix handling of VLANs in backend
Lucio Correia
luciojhc at linux.vnet.ibm.com
Wed Feb 10 18:54:55 UTC 2016
On 10-02-2016 14:41, Ramon Medeiros wrote:
>
>
> 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?
'nat' is only used here, but 'bridge' is spread out through all the code
and has two different, independent meanings.
Considering that, I prefer to leave them hardcoded for now, since the
changes would require lots of more tests than the focus of this patch.
>>
>> - # 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")
>>
>
--
Lucio Correia
Software Engineer
IBM LTC Brazil
More information about the Kimchi-devel
mailing list