From c92c454fde106cedf8753b731a3ad002005d4c28 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sat, 25 Jan 2020 19:47:35 -0500 Subject: [PATCH] virtManager: Drop interface list for bridge and macvtap Some related bits were discussed here: https://www.redhat.com/archives/virt-tools-list/2019-June/msg00117.html macvtap is problematic for inexperienced users so we shouldn't be broadly advertising it, plus our device listing was incomplete anyways. Both bridge and macvtap device listing are largely dependent on the libvirt virInterface APIs, which have varying degrees of completeness across distros and are not particularly reliable to begin with. Drop both of these in favor of the available support for manually specifying a device name Signed-off-by: Cole Robinson --- tests/uitests/addhardware.py | 19 ++---- tests/uitests/details.py | 18 ++---- tests/uitests/newvm.py | 7 ++- virtManager/device/netlist.py | 114 ++-------------------------------- 4 files changed, 19 insertions(+), 139 deletions(-) diff --git a/tests/uitests/addhardware.py b/tests/uitests/addhardware.py index 8f0798724..afaf50920 100644 --- a/tests/uitests/addhardware.py +++ b/tests/uitests/addhardware.py @@ -272,15 +272,16 @@ class AddHardware(uiutils.UITestCase): finish.click() uiutils.check_in_loop(lambda: details.active) - # macvtap selection + # Manual macvtap self._open_addhw_window(details) tab = self._select_hw(addhw, "Network", "network-tab") src.click() - tab.find_fuzzy("macvtap", "menu item").click() + tab.find_fuzzy("Macvtap device...", "menu item").click() + tab.find("Device name:", "text").text = "macvtapfoo7" finish.click() uiutils.check_in_loop(lambda: details.active) - # Manual bridge + # Manual bridge. Also trigger MAC collision self._open_addhw_window(details) tab = self._select_hw(addhw, "Network", "network-tab") tab.find("mac-address-enable", "check box").click() @@ -301,18 +302,6 @@ class AddHardware(uiutils.UITestCase): finish.click() uiutils.check_in_loop(lambda: details.active) - # Manual macvtap - self._open_addhw_window(details) - tab = self._select_hw(addhw, "Network", "network-tab") - tab.find("MAC Address Field", "text").text = "00:11:0B:11:00:11" - src.click() - self.sleep(1) - self.pressKey("Home") - tab.find_fuzzy("Macvtap device...", "menu item").click() - tab.find("Device name:", "text").text = "macvtapfoo7" - finish.click() - uiutils.check_in_loop(lambda: details.active) - def testAddGraphics(self): """ diff --git a/tests/uitests/details.py b/tests/uitests/details.py index ecff59a81..3dfc70dc9 100644 --- a/tests/uitests/details.py +++ b/tests/uitests/details.py @@ -232,11 +232,14 @@ class Details(uiutils.UITestCase): uiutils.check_in_loop(lambda: not appl.sensitive) - # Network values + # Network values w/ macvtap manual tab = self._select_hw(win, "NIC :54:32:10", "network-tab") src = tab.find("Network source:", "combo box") src.click() - tab.find_fuzzy("macvtap", "menu item").bring_on_screen().click() + self.pressKey("Home") + tab.find_fuzzy("Macvtap device...", + "menu item").bring_on_screen().click() + tab.find("Device name:", "text").text = "fakedev12" tab.find("Device model:", "combo box").click_combo_entry() tab.find("rtl8139", "menu item").click() appl.click() @@ -246,6 +249,7 @@ class Details(uiutils.UITestCase): src.click() tab.find_fuzzy("Bridge device...", "menu item").bring_on_screen().click() + tab.find("Device name:", "text").text = "" appl.click() # Check validation error alert = self.app.root.find("vmm dialog", "alert") @@ -255,16 +259,6 @@ class Details(uiutils.UITestCase): appl.click() uiutils.check_in_loop(lambda: not appl.sensitive) - # Manual macvtap - src.click() - self.pressKey("Home") - tab.find_fuzzy("Macvtap device...", - "menu item").bring_on_screen().click() - appl.click() - tab.find("Device name:", "text").text = "fakedev12" - appl.click() - uiutils.check_in_loop(lambda: not appl.sensitive) - def testDetailsEditDevices(self): """ diff --git a/tests/uitests/newvm.py b/tests/uitests/newvm.py index 18192c8a7..499d9188a 100644 --- a/tests/uitests/newvm.py +++ b/tests/uitests/newvm.py @@ -493,9 +493,10 @@ class NewVM(uiutils.UITestCase): win.find_fuzzy("NIC", "table cell").click() tab = win.find("network-tab") win.find("XML", "page tab").click() - oldbrname = "brplain" newbrname = "BRFAKE" - xmleditor.text = xmleditor.text.replace(oldbrname, newbrname) + newx = xmleditor.text.replace("network", "bridge") + newx = newx.replace('bridge="default"', "bridge='%s'" % newbrname) + xmleditor.text = newx finish.click() # Finish install. @@ -514,7 +515,7 @@ class NewVM(uiutils.UITestCase): win.find_fuzzy("NIC", "table cell").click() tab = win.find("network-tab") self.assertEqual( - tab.find("Bridge name:", "text").text, newbrname) + tab.find("Device name:", "text").text, newbrname) # Verify install media is handled correctly after XML customize win.find_fuzzy("IDE CDROM 1", "table cell").click() diff --git a/virtManager/device/netlist.py b/virtManager/device/netlist.py index 8c5ea5b79..55f44f0fe 100644 --- a/virtManager/device/netlist.py +++ b/virtManager/device/netlist.py @@ -3,8 +3,6 @@ # This work is licensed under the GNU GPLv2 or later. # See the COPYING file in the top-level directory. -import collections - from gi.repository import Gtk import virtinst @@ -14,8 +12,6 @@ from ..lib import uiutil from ..baseclass import vmmGObjectUI -NetDev = collections.namedtuple('Netdev', ['name', 'is_bridge', 'slave_names']) - NET_ROW_TYPE = 0 NET_ROW_SOURCE = 1 NET_ROW_LABEL = 2 @@ -117,12 +113,9 @@ class vmmNetworkList(vmmGObjectUI): self.conn.connect("net-added", self._repopulate_network_list) self.conn.connect("net-removed", self._repopulate_network_list) - self.conn.connect("interface-added", self._repopulate_network_list) - self.conn.connect("interface-removed", self._repopulate_network_list) def _find_virtual_networks(self): rows = [] - vnet_bridges = [] for net in self.conn.list_nets(): nettype = virtinst.DeviceInterface.TYPE_VIRTUAL @@ -138,77 +131,6 @@ class vmmNetworkList(vmmGObjectUI): nettype, net.get_name(), label, True, connkey=net.get_connkey())) - # Build a list of vnet bridges, so we know not to list them - # in the physical interface list - vnet_bridge = net.get_bridge_device() - if vnet_bridge: - vnet_bridges.append(vnet_bridge) - - return rows, vnet_bridges - - def _find_physical_devices(self, vnet_bridges): - rows = [] - skip_ifaces = ["lo"] - - vnet_taps = [] - for vm in self.conn.list_vms(): - for nic in vm.get_interface_devices_norefresh(): - if nic.target_dev and nic.target_dev not in vnet_taps: - vnet_taps.append(nic.target_dev) - - netdevs = {} - for iface in self.conn.list_interfaces(): - name = iface.get_name() - netdevs[name] = NetDev(name, iface.is_bridge(), - iface.get_interface_names()) - for nodedev in self.conn.filter_nodedevs("net"): - if nodedev.xmlobj.interface not in netdevs: - netdev = NetDev(nodedev.xmlobj.interface, False, []) - netdevs[nodedev.xmlobj.interface] = netdev - - # For every bridge used by a virtual network, and any slaves of - # those devices, don't list them. - for vnet_bridge in vnet_bridges: - slave_names = netdevs.pop(vnet_bridge, - NetDev(None, None, [])).slave_names - for slave in slave_names: - netdevs.pop(slave, None) - - for name, is_bridge, slave_names in list(netdevs.values()): - if ((name in vnet_taps) or - (name in [v + "-nic" for v in vnet_bridges]) or - (name in skip_ifaces)): - # Don't list this, as it is basically duplicating - # virtual net info - continue - - sensitive = True - source_name = name - - label = _("Host device %s") % (name) - if is_bridge: - nettype = virtinst.DeviceInterface.TYPE_BRIDGE - if slave_names: - extra = (_("Host device %s") % slave_names[0]) - can_default = True - else: - extra = _("Empty bridge") - label = _("Bridge %s: %s") % (name, extra) - - elif self.conn.is_qemu() or self.conn.is_test(): - nettype = virtinst.DeviceInterface.TYPE_DIRECT - label += (": %s" % _("macvtap")) - - else: - nettype = None - sensitive = False - source_name = None - label += (": %s" % _("Not bridged")) - - rows.append(_build_row( - nettype, source_name, label, sensitive, - connkey=name)) - return rows def _populate_network_model(self, model): @@ -231,27 +153,9 @@ class vmmNetworkList(vmmGObjectUI): _add_manual_bridge_row() return - vnets, vnet_bridges = self._find_virtual_networks() - iface_rows = self._find_physical_devices(vnet_bridges) - - # Sorting is: - # 1) Bridges - # 2) Virtual networks - # 3) direct/macvtap - # 4) Disabled list entries - # Each category sorted alphabetically - bridges = [row for row in iface_rows if row[0] == "bridge"] - direct = [row for row in iface_rows if row[0] == "direct"] - disabled = [row for row in iface_rows if row[0] is None] - - for rows in [bridges, vnets, direct, disabled]: - rows.sort(key=lambda r: r[2]) - for row in rows: - model.append(row) - - default_bridge = virtinst.DeviceInterface.default_bridge( - self.conn.get_backend()) - + vnets = self._find_virtual_networks() + for row in sorted(vnets, key=lambda r: r[NET_ROW_LABEL]): + model.append(row) if not len(model): row = _build_label_row(_("No networking"), True) model.insert(0, row) @@ -399,16 +303,8 @@ class vmmNetworkList(vmmGObjectUI): def reset_state(self): self._repopulate_network_list() - net_err = None - if (not self.conn.support.conn_nodedev() or - not self.conn.support.conn_interface()): - net_err = _("Libvirt version does not support " - "physical interface listing.") - - net_warn = self.widget("net-source-warn") - net_warn.set_visible(bool(net_err)) - net_warn.set_tooltip_text(net_err or "") - + self.widget("net-source-warn").set_visible(False) + self.widget("net-source-warn").set_tooltip_text("") self.widget("net-manual-source").set_text("") def set_dev(self, net):