On 01/03/2014 02:04 AM, Aline Manera wrote:
On 01/02/2014 07:50 AM, Mark Wu wrote:
It creates a vlan interface on top of the given nic or bond interface, and bridge the vlan interface to a new created bridge, which is used to forward VM's traffic. So all packets transmitted from VM will be tagged before it departs from the physical network interface.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/API.json | 6 ++++++ src/kimchi/model.py | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 7b90826..07bf41e 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -56,6 +56,12 @@ "interface": { "description": "The name of a network interface on the host", "type": "string" + }, + "vlan_id": { + "description": "Network's VLAN ID", + "type": "integer", + "maximum": 4094, + "minimum": 1 } } }, diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a6790b8..b3589e9 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -58,6 +58,7 @@ from kimchi import config from kimchi import isoinfo from kimchi import netinfo from kimchi import network as knetwork +from kimchi import networkxml from kimchi import vnc from kimchi import xmlutils from kimchi.asynctask import AsyncTask @@ -65,7 +66,6 @@ from kimchi.distroloader import DistroLoader from kimchi.exception import InvalidOperation, InvalidParameter, MissingParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.featuretests import FeatureTests -from kimchi.networkxml import to_network_xml from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner from kimchi.screenshot import VMScreenshot @@ -806,7 +806,12 @@ class Model(object): if netinfo.is_bridge(iface): params['bridge'] = iface elif netinfo.is_bare_nic(iface) or netinfo.is_bonding(iface): - params['forward']['dev'] = iface + if params.get('vlan_id') is None: + params['forward']['dev'] = iface + else: + params['bridge'] = \ + self._create_vlanned_bridge(str(iface), + str(params['vlan_id'])) else: raise InvalidParameter("the interface should be bare nic, " "bonding or bridge device.") @@ -830,7 +835,7 @@ class Model(object): if connection == 'bridge': self._set_network_bridge(params)
- xml = to_network_xml(**params) + xml = networkxml.to_network_xml(**params)
try: network = conn.networkDefineXML(xml) @@ -888,8 +893,44 @@ class Model(object): if network.isActive(): raise InvalidOperation( "Unable to delete the active network %s" % name) + self._remove_vlanned_bridge(network) network.undefine()
+ def _get_vlanned_bridge_name(self, interface, vlan_id): + return '-'.join(('kimchi', interface, vlan_id)) + + def _is_vlanned_bridge(self, bridge): + return bridge.startswith('kimchi-') + + def _create_vlanned_bridge(self, interface, vlan_id): + bridge = self._get_vlanned_bridge_name(interface, vlan_id) + bridge_xml = networkxml.create_vlanned_bridge_xml(bridge, interface, + vlan_id) + conn = self.conn.get() + conn.changeBegin() + try: + vlanned_bridge = conn.interfaceDefineXML(bridge_xml) + vlanned_bridge.create() + except: + conn.changeRollback() + raise OperationFailed('Failed to create vlanned bridge') + else: + conn.changeCommit() + return bridge +
You can avoid multiple indents by removing 'else' statement I prefer to use 'try...except...else' to because it matches 'do...rollback...commit' clearly. If the 'else' statement is removed, we need to check the code in 'except' to see if unindent code only runs on success. Moreover, the statement under 'else' is very short. We don't need care about the indents. Does it make sense?
+ def _remove_vlanned_bridge(self, network): + try: + bridge = network.bridgeName() + except libvirt.libvirtError: + pass
Log the information.
kimchi_log.info() or kimchi_log.debug() It doesn't mean any actual error. It only happen when the network doesn't have a bridge, like the networks forwarded by macvtap. So it's just like a normal 'if' statement to check the forward mode of the network. I don't think we need a special logging here.
+ else: + if self._is_vlanned_bridge(bridge): + conn = self.conn.get() + iface = conn.interfaceLookupByName(bridge) + if iface.isActive(): + iface.destroy() + iface.undefine()
You can avoid multiple indents by removing 'else' statement Nope, we can't remove 'else' here. It change the semantic. Because the 'pass' statement in 'except' block doesn't stop execution, it will run into the code no matter it succeeds or not. We need only call it on success.
+ def add_task(self, target_uri, fn, opaque=None): id = self.next_taskid self.next_taskid = self.next_taskid + 1