diff --git a/interfaces/forms.py b/interfaces/forms.py index ab680f43..2fb54c25 100644 --- a/interfaces/forms.py +++ b/interfaces/forms.py @@ -32,11 +32,13 @@ class AddInterface(forms.Form): stp = forms.ChoiceField(required=False, choices=(("on", "on"), ("off", "off"))) delay = forms.IntegerField(required=False, min_value=0) - def __init__(self, *args, netdevs=(), **kwargs): + def __init__(self, *args, netdevs=(), defined=(), **kwargs): # The host's network devices: a bridge member, or the device an - # ethernet interface configures, must be one of them. + # ethernet interface configures, must be one of them. The interfaces + # the host defines: defining one again would replace its configuration. super().__init__(*args, **kwargs) self.netdevs = set(netdevs) + self.defined = set(defined) def clean_ipv4_addr(self): ipv4_addr = self.cleaned_data["ipv4_addr"] @@ -90,6 +92,8 @@ class AddInterface(forms.Form): raise forms.ValidationError( _("The interface name may have up to 15 letters, digits and . _ -") ) + if name in self.defined: + raise forms.ValidationError(_("The host already defines interface %(name)s") % {"name": name}) return name def clean_netdev(self): diff --git a/interfaces/tests/test_forms.py b/interfaces/tests/test_forms.py index bfdc7341..b201f9a8 100644 --- a/interfaces/tests/test_forms.py +++ b/interfaces/tests/test_forms.py @@ -28,6 +28,16 @@ def form(**values): return AddInterface(data={**BASE, **values}, netdevs=NETDEVS) +class DefinedNameTestCase(SimpleTestCase): + def test_an_interface_the_host_defines_is_not_replaced(self): + defined = ["br0", "bond-lan"] # as libvirt lists them + for values in ({"name": "br0"}, {"name": "bond-lan", "itype": "ethernet", "netdev": ""}): + with self.subTest(**values): + f = AddInterface(data={**BASE, **values}, netdevs=NETDEVS, defined=defined) + self.assertEqual(f.errors["name"], [f"The host already defines interface {values['name']}"]) + self.assertTrue(AddInterface(data={**BASE, "ipv6_type": "none"}, netdevs=NETDEVS, defined=defined).is_valid()) + + class Ipv6FormTestCase(SimpleTestCase): def test_valid_addresses_are_normalized(self): f = form(ipv6_addr="2001:DB8::10/64", ipv6_gw="2001:DB8::1") @@ -196,11 +206,8 @@ class BridgeMemberXmlTestCase(SimpleTestCase): def member(self, host_xml): conn = wvmInterfaces.__new__(wvmInterfaces) conn.define_iface = MagicMock() - created = MagicMock() def get_iface(name): - if name == "br1": - return created if host_xml is None: raise no_interface(name) iface = MagicMock() @@ -209,7 +216,7 @@ class BridgeMemberXmlTestCase(SimpleTestCase): conn.get_iface = MagicMock(side_effect=get_iface) conn.create_iface("br1", "bridge", "onboot", "bond0", "none", "", "", "none", "", "", "on", 0) - self.assertTrue(created.create.called) + self.assertTrue(conn.define_iface.return_value.create.called) members = etree.fromstring(conn.define_iface.call_args[0][0]).findall("bridge/interface") self.assertEqual(len(members), 1) return members[0] @@ -254,6 +261,15 @@ class BridgeMemberXmlTestCase(SimpleTestCase): conn.create_iface("br1", "bridge", "onboot", "bond0", "none", "", "", "none", "", "", "on", 0) self.assertFalse(conn.define_iface.called) + def test_an_interface_that_does_not_start_is_undefined(self): + conn = wvmInterfaces.__new__(wvmInterfaces) + conn.define_iface = MagicMock() + iface = conn.define_iface.return_value + iface.create.side_effect = libvirt.libvirtError("failed to bring up br1") + with self.assertRaises(libvirt.libvirtError): + conn.create_iface("br1", "ethernet", "onboot", "", "none", "", "", "none", "", "", "on", 0) + iface.undefine.assert_called_once_with() + class BridgeSettingsTestCase(SimpleTestCase): def test_a_bridge_needs_stp_and_a_delay(self): diff --git a/interfaces/views.py b/interfaces/views.py index 78d74e64..07fb30ca 100644 --- a/interfaces/views.py +++ b/interfaces/views.py @@ -55,7 +55,7 @@ def interfaces(request, compute_id): # the page says why nothing is created on a host that cannot change # interfaces; without the host's devices nothing can be checked if "create" in request.POST and changeable and netdev_error is None: - form = AddInterface(request.POST, netdevs=netdevs) + form = AddInterface(request.POST, netdevs=netdevs, defined=ifaces) if form.is_valid(): data = form.cleaned_data with audit(request, compute, f"Interface {data['name']}", "add"): diff --git a/vrtManager/interface.py b/vrtManager/interface.py index 97b59f95..29382763 100644 --- a/vrtManager/interface.py +++ b/vrtManager/interface.py @@ -8,7 +8,7 @@ from vrtManager.connection import wvmConnect class wvmInterfaces(wvmConnect): def define_iface(self, xml, flag=0): - self.wvm.interfaceDefineXML(xml, flag) + return self.wvm.interfaceDefineXML(xml, flag) def bridge_member_xml(self, netdev): """netdev as a bridge member: as the host defines it (a bond keeps its @@ -69,9 +69,13 @@ class wvmInterfaces(wvmConnect): {self.bridge_member_xml(netdev)} """ xml += """""" - self.define_iface(xml) - iface = self.get_iface(name) - iface.create() + # the form refuses a name the host defines, so nothing is replaced + iface = self.define_iface(xml) + try: + iface.create() + except libvirtError: # the name stays free for another try + iface.undefine() + raise class wvmInterface(wvmConnect):