[PATCH] Fix issue 766 - Define network interface in libvirt
When a network interface was only visible to the system and not to libvirt, libvirt throws an error when user asks a bridged network to be created using that interface, since it is not visible to libvirt. This patch adds network interface redefinition code to make it visible to libvirt, allowing the bridged network to be created. Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/plugins/kimchi/model/networks.py | 40 +++++++++++++++------------- src/wok/plugins/kimchi/network.py | 5 ++++ src/wok/plugins/kimchi/xmlutils/interface.py | 20 +++++++++++--- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/wok/plugins/kimchi/model/networks.py b/src/wok/plugins/kimchi/model/networks.py index 7f50003..25640a4 100644 --- a/src/wok/plugins/kimchi/model/networks.py +++ b/src/wok/plugins/kimchi/model/networks.py @@ -33,6 +33,7 @@ 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.interface import get_iface_xml 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 @@ -237,8 +238,23 @@ class NetworksModel(object): 'err': e.get_error_message()}) def _create_linux_bridge(self, interface): - # get xml definition of interface - iface_xml = self._get_interface_desc_xml(interface) + iface_defined = False + try: + # get xml definition of configured interface + conn = self.conn.get() + iface = conn.interfaceLookupByName(interface) + iface_xml = iface.XMLDesc(flags=VIR_INTERFACE_XML_INACTIVE) + except libvirt.libvirtError, e: + try: + # try to define that interface in libvirt + mac = knetwork.get_dev_macaddr(str(interface)) + iface_xml = get_iface_xml({'name': interface, 'mac': mac, + 'startmode': "onboot"}) + conn.interfaceDefineXML(iface_xml.encode("utf-8")) + iface_defined = True + except libvirt.libvirtError, e: + raise OperationFailed("KCHNET0023E", {'name': interface, + 'err': e.get_error_message()}) # Truncate the interface name if it exceeds 13 characters to make sure # the length of bridge name is less than 15 (its maximum value). @@ -246,7 +262,7 @@ class NetworksModel(object): br_xml = create_linux_bridge_xml(br_name, interface, iface_xml) # drop network config from interface - self._redefine_iface_no_network(interface) + iface_defined or self._redefine_iface_no_network(interface, iface_xml) # create and start bridge self._create_bridge(br_name, br_xml) @@ -263,27 +279,13 @@ class NetworksModel(object): return br_name - def _get_interface_desc_xml(self, name): - conn = self.conn.get() - - 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()}) - - return xml - - def _redefine_iface_no_network(self, name): - conn = self.conn.get() - + def _redefine_iface_no_network(self, name, iface_xml): # 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 + conn = self.conn.get() conn.interfaceDefineXML(xml.encode("utf-8")) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0024E", {'name': name, diff --git a/src/wok/plugins/kimchi/network.py b/src/wok/plugins/kimchi/network.py index 1433b8a..eee0e8c 100644 --- a/src/wok/plugins/kimchi/network.py +++ b/src/wok/plugins/kimchi/network.py @@ -31,6 +31,11 @@ DefaultNetsPool = [ipaddr.IPNetwork('192.168.122.0/23'), ipaddr.IPNetwork('192.168.128.0/17')] +def get_dev_macaddr(dev): + info = ethtool.get_interfaces_info(dev)[0] + return info.mac_address + + def get_dev_netaddr(dev): info = ethtool.get_interfaces_info(dev)[0] return (info.ipv4_address and diff --git a/src/wok/plugins/kimchi/xmlutils/interface.py b/src/wok/plugins/kimchi/xmlutils/interface.py index 64df8cd..7730407 100644 --- a/src/wok/plugins/kimchi/xmlutils/interface.py +++ b/src/wok/plugins/kimchi/xmlutils/interface.py @@ -26,15 +26,27 @@ from wok.plugins.kimchi import osinfo def get_iface_xml(params, arch=None, os_distro=None, os_version=None): """ - <interface type='network'> + <interface type='network' name='ethX'> + <start mode='onboot'/> <source network='default'/> <model type='virtio'/> </interface> """ - interface = E.interface(type=params['type']) - interface.append(E.source(network=params['network'])) + name = params.get('name', None) + if name: + interface = E.interface(type=params.get('type', 'ethernet'), name=name) + else: + interface = E.interface(type=params.get('type', 'ethernet')) - model = params.get('model') + stmode = params.get('startmode', None) + if stmode: + interface.append(E.start(mode=stmode)) + + nw = params.get('network', None) + if nw: + interface.append(E.source(network=nw)) + + model = params.get('model', None) # no model specified; let's try querying osinfo if model is None: -- 1.9.1
Review: +1, just a few inline comments On 11/26/2015 01:48 PM, Lucio Correia wrote:
When a network interface was only visible to the system and not to libvirt, libvirt throws an error when user asks a bridged network to be created using that interface, since it is not visible to libvirt.
This patch adds network interface redefinition code to make it visible to libvirt, allowing the bridged network to be created.
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/plugins/kimchi/model/networks.py | 40 +++++++++++++++------------- src/wok/plugins/kimchi/network.py | 5 ++++ src/wok/plugins/kimchi/xmlutils/interface.py | 20 +++++++++++--- 3 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/src/wok/plugins/kimchi/model/networks.py b/src/wok/plugins/kimchi/model/networks.py index 7f50003..25640a4 100644 --- a/src/wok/plugins/kimchi/model/networks.py +++ b/src/wok/plugins/kimchi/model/networks.py @@ -33,6 +33,7 @@ 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.interface import get_iface_xml 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 @@ -237,8 +238,23 @@ class NetworksModel(object): 'err': e.get_error_message()})
def _create_linux_bridge(self, interface): - # get xml definition of interface - iface_xml = self._get_interface_desc_xml(interface) + iface_defined = False + try: + # get xml definition of configured interface + conn = self.conn.get() + iface = conn.interfaceLookupByName(interface) + iface_xml = iface.XMLDesc(flags=VIR_INTERFACE_XML_INACTIVE) + except libvirt.libvirtError, e: + try: + # try to define that interface in libvirt + mac = knetwork.get_dev_macaddr(str(interface)) + iface_xml = get_iface_xml({'name': interface, 'mac': mac, + 'startmode': "onboot"}) + conn.interfaceDefineXML(iface_xml.encode("utf-8")) + iface_defined = True + except libvirt.libvirtError, e: + raise OperationFailed("KCHNET0023E", {'name': interface, + 'err': e.get_error_message()})
You could have avoided this nested exception by creating nested methods, like: def _create_linux_bridge(...) def get_configured_interface_xml_def(...) (return iface_xml or None if exception thrown) def define_libvirt_interface() (do stuff) iface_xml = get_configured_interface_xml_def() if iface_xml is None: iface_xml = define_libvirt_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). @@ -246,7 +262,7 @@ class NetworksModel(object): br_xml = create_linux_bridge_xml(br_name, interface, iface_xml)
# drop network config from interface - self._redefine_iface_no_network(interface) + iface_defined or self._redefine_iface_no_network(interface, iface_xml)
# create and start bridge self._create_bridge(br_name, br_xml) @@ -263,27 +279,13 @@ class NetworksModel(object):
return br_name
- def _get_interface_desc_xml(self, name): - conn = self.conn.get() - - 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()}) - - return xml - - def _redefine_iface_no_network(self, name): - conn = self.conn.get() - + def _redefine_iface_no_network(self, name, iface_xml): # 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 + conn = self.conn.get() conn.interfaceDefineXML(xml.encode("utf-8")) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0024E", {'name': name, diff --git a/src/wok/plugins/kimchi/network.py b/src/wok/plugins/kimchi/network.py index 1433b8a..eee0e8c 100644 --- a/src/wok/plugins/kimchi/network.py +++ b/src/wok/plugins/kimchi/network.py @@ -31,6 +31,11 @@ DefaultNetsPool = [ipaddr.IPNetwork('192.168.122.0/23'), ipaddr.IPNetwork('192.168.128.0/17')]
+def get_dev_macaddr(dev): + info = ethtool.get_interfaces_info(dev)[0] + return info.mac_address + + def get_dev_netaddr(dev): info = ethtool.get_interfaces_info(dev)[0] return (info.ipv4_address and diff --git a/src/wok/plugins/kimchi/xmlutils/interface.py b/src/wok/plugins/kimchi/xmlutils/interface.py index 64df8cd..7730407 100644 --- a/src/wok/plugins/kimchi/xmlutils/interface.py +++ b/src/wok/plugins/kimchi/xmlutils/interface.py @@ -26,15 +26,27 @@ from wok.plugins.kimchi import osinfo
def get_iface_xml(params, arch=None, os_distro=None, os_version=None): """ - <interface type='network'> + <interface type='network' name='ethX'> + <start mode='onboot'/> <source network='default'/> <model type='virtio'/> </interface> """ - interface = E.interface(type=params['type']) - interface.append(E.source(network=params['network'])) + name = params.get('name', None)
You don't need to specify 'None' as default return value of dict.get(). 'None' is the default return value when you don't define it. On the other hand, we can appreciate the clearness of showing that the default value is 'None'. Just a general comment.
+ if name: + interface = E.interface(type=params.get('type', 'ethernet'), name=name) + else: + interface = E.interface(type=params.get('type', 'ethernet'))
- model = params.get('model') + stmode = params.get('startmode', None) + if stmode: + interface.append(E.start(mode=stmode)) + + nw = params.get('network', None) + if nw: + interface.append(E.source(network=nw)) + + model = params.get('model', None)
# no model specified; let's try querying osinfo if model is None:
Thanks for the review, see my inline answers. On 26-11-2015 14:31, Daniel Henrique Barboza wrote:
Review: +1, just a few inline comments I've sent a V2.
On 11/26/2015 01:48 PM, Lucio Correia wrote:
When a network interface was only visible to the system and not to libvirt, libvirt throws an error when user asks a bridged network to be created using that interface, since it is not visible to libvirt.
This patch adds network interface redefinition code to make it visible to libvirt, allowing the bridged network to be created.
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/plugins/kimchi/model/networks.py | 40 +++++++++++++++------------- src/wok/plugins/kimchi/network.py | 5 ++++ src/wok/plugins/kimchi/xmlutils/interface.py | 20 +++++++++++--- 3 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/src/wok/plugins/kimchi/model/networks.py b/src/wok/plugins/kimchi/model/networks.py index 7f50003..25640a4 100644 --- a/src/wok/plugins/kimchi/model/networks.py +++ b/src/wok/plugins/kimchi/model/networks.py @@ -33,6 +33,7 @@ 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.interface import get_iface_xml 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 @@ -237,8 +238,23 @@ class NetworksModel(object): 'err': e.get_error_message()}) def _create_linux_bridge(self, interface): - # get xml definition of interface - iface_xml = self._get_interface_desc_xml(interface) + iface_defined = False + try: + # get xml definition of configured interface + conn = self.conn.get() + iface = conn.interfaceLookupByName(interface) + iface_xml = iface.XMLDesc(flags=VIR_INTERFACE_XML_INACTIVE) + except libvirt.libvirtError, e: + try: + # try to define that interface in libvirt + mac = knetwork.get_dev_macaddr(str(interface)) + iface_xml = get_iface_xml({'name': interface, 'mac': mac, + 'startmode': "onboot"}) + conn.interfaceDefineXML(iface_xml.encode("utf-8")) + iface_defined = True + except libvirt.libvirtError, e: + raise OperationFailed("KCHNET0023E", {'name': interface, + 'err': e.get_error_message()})
You could have avoided this nested exception by creating nested methods, like:
def _create_linux_bridge(...) def get_configured_interface_xml_def(...) (return iface_xml or None if exception thrown)
def define_libvirt_interface() (do stuff)
iface_xml = get_configured_interface_xml_def()
if iface_xml is None: iface_xml = define_libvirt_interface()
Agreed, included in V2.
# Truncate the interface name if it exceeds 13 characters to make sure # the length of bridge name is less than 15 (its maximum value). @@ -246,7 +262,7 @@ class NetworksModel(object): br_xml = create_linux_bridge_xml(br_name, interface, iface_xml) # drop network config from interface - self._redefine_iface_no_network(interface) + iface_defined or self._redefine_iface_no_network(interface, iface_xml) # create and start bridge self._create_bridge(br_name, br_xml) @@ -263,27 +279,13 @@ class NetworksModel(object): return br_name - def _get_interface_desc_xml(self, name): - conn = self.conn.get() - - 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()}) - - return xml - - def _redefine_iface_no_network(self, name): - conn = self.conn.get() - + def _redefine_iface_no_network(self, name, iface_xml): # 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 + conn = self.conn.get() conn.interfaceDefineXML(xml.encode("utf-8")) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0024E", {'name': name, diff --git a/src/wok/plugins/kimchi/network.py b/src/wok/plugins/kimchi/network.py index 1433b8a..eee0e8c 100644 --- a/src/wok/plugins/kimchi/network.py +++ b/src/wok/plugins/kimchi/network.py @@ -31,6 +31,11 @@ DefaultNetsPool = [ipaddr.IPNetwork('192.168.122.0/23'), ipaddr.IPNetwork('192.168.128.0/17')] +def get_dev_macaddr(dev): + info = ethtool.get_interfaces_info(dev)[0] + return info.mac_address + + def get_dev_netaddr(dev): info = ethtool.get_interfaces_info(dev)[0] return (info.ipv4_address and diff --git a/src/wok/plugins/kimchi/xmlutils/interface.py b/src/wok/plugins/kimchi/xmlutils/interface.py index 64df8cd..7730407 100644 --- a/src/wok/plugins/kimchi/xmlutils/interface.py +++ b/src/wok/plugins/kimchi/xmlutils/interface.py @@ -26,15 +26,27 @@ from wok.plugins.kimchi import osinfo def get_iface_xml(params, arch=None, os_distro=None, os_version=None): """ - <interface type='network'> + <interface type='network' name='ethX'> + <start mode='onboot'/> <source network='default'/> <model type='virtio'/> </interface> """ - interface = E.interface(type=params['type']) - interface.append(E.source(network=params['network'])) + name = params.get('name', None)
You don't need to specify 'None' as default return value of dict.get(). 'None' is the default return value when you don't define it. On the other hand, we can appreciate the clearness of showing that the default value is 'None'.
Yes, went for clearness here, and also to keep same standard of code I added.
Just a general comment.
+ if name: + interface = E.interface(type=params.get('type', 'ethernet'), name=name) + else: + interface = E.interface(type=params.get('type', 'ethernet')) - model = params.get('model') + stmode = params.get('startmode', None) + if stmode: + interface.append(E.start(mode=stmode)) + + nw = params.get('network', None) + if nw: + interface.append(E.source(network=nw)) + + model = params.get('model', None) # no model specified; let's try querying osinfo if model is None:
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Lucio Correia Software Engineer IBM LTC Brazil
participants (2)
-
Daniel Henrique Barboza -
Lucio Correia