From eee1caa94648cc5385a2153c7c3589eb9af29a34 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Wed, 21 Mar 2018 14:42:50 -0400 Subject: [PATCH] domain: Drop most device list wrappers There's lots of hacks stuffed into the domain device lists. Formalize some of it, move some of the specific stuff to details.py, and drop a lot of the needless API wrappers --- tests/xmlparse.py | 6 ++ virtManager/addhardware.py | 18 ++-- virtManager/clone.py | 4 +- virtManager/console.py | 15 +-- virtManager/delete.py | 2 +- virtManager/details.py | 70 ++++++++++---- virtManager/domain.py | 183 ++++++++++++------------------------- virtManager/netlist.py | 2 +- virtManager/serialcon.py | 4 +- virtManager/snapshots.py | 2 +- virtinst/xmlbuilder.py | 11 +++ 11 files changed, 148 insertions(+), 169 deletions(-) diff --git a/tests/xmlparse.py b/tests/xmlparse.py index 14bfa38c..f1327042 100644 --- a/tests/xmlparse.py +++ b/tests/xmlparse.py @@ -267,6 +267,10 @@ class XMLParseTest(unittest.TestCase): guest.cpu.add_feature("x2apic", "forbid") guest.cpu.set_topology_defaults(guest.vcpus) self.assertTrue(guest.cpu.get_xml_config().startswith(" 0: diff --git a/virtManager/clone.py b/virtManager/clone.py index e404572b..23a95e61 100644 --- a/virtManager/clone.py +++ b/virtManager/clone.py @@ -311,7 +311,7 @@ class vmmCloneVM(vmmGObjectUI): self.net_list[origmac] = net_row self.mac_list.append(origmac) - for net in self.vm.get_network_devices(): + for net in self.vm.xmlobj.devices.interface: mac = net.macaddr net_dev = net.source net_type = net.type @@ -356,7 +356,7 @@ class vmmCloneVM(vmmGObjectUI): """ Determine which storage is cloneable, and which isn't """ - diskinfos = self.vm.get_disk_devices() + diskinfos = self.vm.xmlobj.devices.disk cd = self.clone_design storage_list = {} diff --git a/virtManager/console.py b/virtManager/console.py index 1212f83f..47687ff7 100644 --- a/virtManager/console.py +++ b/virtManager/console.py @@ -681,7 +681,7 @@ class vmmConsolePages(vmmGObjectUI): ginfo = None try: - gdevs = self.vm.get_graphics_devices() + gdevs = self.vm.xmlobj.devices.graphics gdev = gdevs and gdevs[0] or None if gdev: ginfo = ConnectionInfo(self.vm.conn, gdev) @@ -864,7 +864,8 @@ class vmmConsolePages(vmmGObjectUI): """ Find the default graphical or serial console for the VM """ - if self.vm.get_graphics_devices() or not self.vm.get_serial_devs(): + if (self.vm.xmlobj.devices.graphics or + not self.vm.get_serialcon_devices()): return # We iterate through the 'console' menu and activate the first @@ -886,7 +887,7 @@ class vmmConsolePages(vmmGObjectUI): self.widget("console-pages").set_current_page(_CONSOLE_PAGE_VIEWER) return - target_port = dev.vmmindex + target_port = dev.get_xml_idx() serial = None name = src.get_label() for s in self._serial_consoles: @@ -911,7 +912,7 @@ class vmmConsolePages(vmmGObjectUI): self.widget("serial-pages").set_current_page(page_idx) def _build_serial_menu_items(self, menu_item_cb): - devs = self.vm.get_serial_devs() + devs = self.vm.get_serialcon_devices() if len(devs) == 0: menu_item_cb(_("No text console available"), radio=False, sensitive=False) @@ -926,9 +927,9 @@ class vmmConsolePages(vmmGObjectUI): for dev in devs: if dev.DEVICE_TYPE == "console": - label = _("Text Console %d") % (dev.vmmindex + 1) + label = _("Text Console %d") % (dev.get_xml_idx() + 1) else: - label = _("Serial %d") % (dev.vmmindex + 1) + label = _("Serial %d") % (dev.get_xml_idx() + 1) tooltip = vmmSerialConsole.can_connect(self.vm, dev) sensitive = not bool(tooltip) @@ -938,7 +939,7 @@ class vmmConsolePages(vmmGObjectUI): tooltip=tooltip, cb=self._console_menu_toggled, cbdata=dev) def _build_graphical_menu_items(self, menu_item_cb): - devs = self.vm.get_graphics_devices() + devs = self.vm.xmlobj.devices.graphics if len(devs) == 0: menu_item_cb(_("No graphical console available"), radio=False, sensitive=False) diff --git a/virtManager/delete.py b/virtManager/delete.py index 00e1162e..e49985f2 100644 --- a/virtManager/delete.py +++ b/virtManager/delete.py @@ -239,7 +239,7 @@ def populate_storage_list(storage_list, vm, conn): diskdata = [(d.target, d.path, d.read_only, d.shareable, d.device in ["cdrom", "floppy"]) for - d in vm.get_disk_devices()] + d in vm.xmlobj.devices.disk] diskdata.append(("kernel", vm.get_xmlobj().os.kernel, True, False, True)) diskdata.append(("initrd", vm.get_xmlobj().os.initrd, True, False, True)) diff --git a/virtManager/details.py b/virtManager/details.py index f23e9b1d..fd669fc5 100644 --- a/virtManager/details.py +++ b/virtManager/details.py @@ -149,6 +149,24 @@ remove_pages = [HW_LIST_TYPE_NIC, HW_LIST_TYPE_INPUT, _remove_tooltip = _("Remove this device from the virtual machine") +def _calculate_disk_bus_index(disklist): + # Iterate through all disks and calculate what number they are + # This sets disk.disk_bus_index which is not a standard property + idx_mapping = {} + for dev in disklist: + devtype = dev.device + bus = dev.bus + key = devtype + (bus or "") + + if key not in idx_mapping: + idx_mapping[key] = 1 + + dev.disk_bus_index = idx_mapping[key] + idx_mapping[key] += 1 + + return disklist + + def _label_for_device(dev): devtype = dev.DEVICE_TYPE @@ -210,7 +228,8 @@ def _label_for_device(dev): if devtype == "graphics": return _("Display %s") % dev.pretty_type_simple(dev.type) if devtype == "redirdev": - return _("%s Redirector %s") % (dev.bus.upper(), dev.vmmindex + 1) + return _("%s Redirector %s") % (dev.bus.upper(), + dev.get_xml_idx() + 1) if devtype == "hostdev": return dev.pretty_name() if devtype == "sound": @@ -2856,7 +2875,7 @@ class vmmDetails(vmmGObjectUI): char_type = chardev.DEVICE_TYPE.capitalize() target_port = chardev.target_port dev_type = chardev.type or "pty" - primary = hasattr(chardev, "virtmanager_console_dup") + primary = self.vm.serial_is_console_dup(chardev) typelabel = "" if char_type == "serial": @@ -2974,7 +2993,7 @@ class vmmDetails(vmmGObjectUI): if controller.type == "scsi": model = self.widget("controller-device-list").get_model() model.clear() - for disk in self.vm.get_disk_devices(): + for disk in _calculate_disk_bus_index(self.vm.xmlobj.devices.disk): if disk.address.compare_controller(controller, disk.bus): can_remove = False name = _label_for_device(disk) @@ -3146,28 +3165,39 @@ class vmmDetails(vmmGObjectUI): add_hw_list_option(insertAt, label, hwtype, dev, icon) - for dev in self.vm.get_disk_devices(): + consoles = self.vm.xmlobj.devices.console + serials = self.vm.xmlobj.devices.serial + if serials and consoles and self.vm.serial_is_console_dup(serials[0]): + consoles.pop(0) + + for dev in _calculate_disk_bus_index(self.vm.xmlobj.devices.disk): update_hwlist(HW_LIST_TYPE_DISK, dev) - for dev in self.vm.get_network_devices(): + for dev in self.vm.xmlobj.devices.interface: update_hwlist(HW_LIST_TYPE_NIC, dev) - for dev in self.vm.get_input_devices(): + for dev in self.vm.xmlobj.devices.input: update_hwlist(HW_LIST_TYPE_INPUT, dev) - for dev in self.vm.get_graphics_devices(): + for dev in self.vm.xmlobj.devices.graphics: update_hwlist(HW_LIST_TYPE_GRAPHICS, dev) - for dev in self.vm.get_sound_devices(): + for dev in self.vm.xmlobj.devices.sound: update_hwlist(HW_LIST_TYPE_SOUND, dev) - for dev in self.vm.get_char_devices(): + for dev in serials: update_hwlist(HW_LIST_TYPE_CHAR, dev) - for dev in self.vm.get_hostdev_devices(): + for dev in self.vm.xmlobj.devices.parallel: + update_hwlist(HW_LIST_TYPE_CHAR, dev) + for dev in consoles: + update_hwlist(HW_LIST_TYPE_CHAR, dev) + for dev in self.vm.xmlobj.devices.channel: + update_hwlist(HW_LIST_TYPE_CHAR, dev) + for dev in self.vm.xmlobj.devices.hostdev: update_hwlist(HW_LIST_TYPE_HOSTDEV, dev) - for dev in self.vm.get_redirdev_devices(): + for dev in self.vm.xmlobj.devices.redirdev: update_hwlist(HW_LIST_TYPE_REDIRDEV, dev) - for dev in self.vm.get_video_devices(): + for dev in self.vm.xmlobj.devices.video: update_hwlist(HW_LIST_TYPE_VIDEO, dev) - for dev in self.vm.get_watchdog_devices(): + for dev in self.vm.xmlobj.devices.watchdog: update_hwlist(HW_LIST_TYPE_WATCHDOG, dev) - for dev in self.vm.get_controller_devices(): + for dev in self.vm.xmlobj.devices.controller: # skip USB2 ICH9 companion controllers if dev.model in ["ich9-uhci1", "ich9-uhci2", "ich9-uhci3"]: continue @@ -3180,15 +3210,15 @@ class vmmDetails(vmmGObjectUI): update_hwlist(HW_LIST_TYPE_CONTROLLER, dev) - for dev in self.vm.get_filesystem_devices(): + for dev in self.vm.xmlobj.devices.filesystem: update_hwlist(HW_LIST_TYPE_FILESYSTEM, dev) - for dev in self.vm.get_smartcard_devices(): + for dev in self.vm.xmlobj.devices.smartcard: update_hwlist(HW_LIST_TYPE_SMARTCARD, dev) - for dev in self.vm.get_tpm_devices(): + for dev in self.vm.xmlobj.devices.tpm: update_hwlist(HW_LIST_TYPE_TPM, dev) - for dev in self.vm.get_rng_devices(): + for dev in self.vm.xmlobj.devices.rng: update_hwlist(HW_LIST_TYPE_RNG, dev) - for dev in self.vm.get_panic_devices(): + for dev in self.vm.xmlobj.devices.panic: update_hwlist(HW_LIST_TYPE_PANIC, dev) devs = list(range(len(hw_list_model))) @@ -3217,7 +3247,7 @@ class vmmDetails(vmmGObjectUI): icon = _icon_for_device(dev) label = _label_for_device(dev) - ret.append([dev.vmmidstr, label, icon, False, True]) + ret.append([dev.get_xml_id(), label, icon, False, True]) if not ret: ret.append([None, _("No bootable devices"), None, False, False]) diff --git a/virtManager/domain.py b/virtManager/domain.py index c5208e7f..0b88e1c5 100644 --- a/virtManager/domain.py +++ b/virtManager/domain.py @@ -29,26 +29,26 @@ class _SENTINEL(object): def compare_device(origdev, newdev, idx): devprops = { "disk": ["target", "bus"], - "interface": ["macaddr", "vmmindex"], - "input": ["bus", "type", "vmmindex"], - "sound": ["model", "vmmindex"], - "video": ["model", "vmmindex"], - "watchdog": ["vmmindex"], - "hostdev": ["type", "managed", "vmmindex", + "interface": ["macaddr", "xmlindex"], + "input": ["bus", "type", "xmlindex"], + "sound": ["model", "xmlindex"], + "video": ["model", "xmlindex"], + "watchdog": ["xmlindex"], + "hostdev": ["type", "managed", "xmlindex", "product", "vendor", "function", "domain", "slot"], "serial": ["type", "target_port"], "parallel": ["type", "target_port"], "console": ["type", "target_type", "target_port"], - "graphics": ["type", "vmmindex"], + "graphics": ["type", "xmlindex"], "controller": ["type", "index"], "channel": ["type", "target_name"], - "filesystem": ["target", "vmmindex"], - "smartcard": ["mode", "vmmindex"], - "redirdev": ["bus", "type", "vmmindex"], - "tpm": ["type", "vmmindex"], - "rng": ["type", "vmmindex"], - "panic": ["type", "vmmindex"], + "filesystem": ["target", "xmlindex"], + "smartcard": ["mode", "xmlindex"], + "redirdev": ["bus", "type", "xmlindex"], + "tpm": ["type", "xmlindex"], + "rng": ["type", "xmlindex"], + "panic": ["type", "xmlindex"], } if id(origdev) == id(newdev): @@ -58,10 +58,11 @@ def compare_device(origdev, newdev, idx): return False for devprop in devprops[origdev.DEVICE_TYPE]: - origval = getattr(origdev, devprop) - if devprop == "vmmindex": + if devprop == "xmlindex": + origval = origdev.get_xml_idx() newval = idx else: + origval = getattr(origdev, devprop) newval = getattr(newdev, devprop) if origval != newval: @@ -315,7 +316,7 @@ class vmmDomain(vmmLibvirtObject): "To fix this, remove and reattach the USB device " "to your guest using the 'Add Hardware' wizard.") - for hostdev in self.get_hostdev_devices(): + for hostdev in self.xmlobj.devices.hostdev: devtype = hostdev.type if devtype != "usb": @@ -373,7 +374,7 @@ class vmmDomain(vmmLibvirtObject): return self.get_xmlobj().stable_defaults() def has_spicevmc_type_redirdev(self): - devs = self.get_redirdev_devices() + devs = self.xmlobj.devices.redirdev for dev in devs: if dev.type == "spicevmc": return True @@ -416,7 +417,7 @@ class vmmDomain(vmmLibvirtObject): # Check if our disks are all qcow2 seen_qcow2 = False - for disk in self.get_disk_devices(refresh_if_nec=False): + for disk in self.get_disk_devices_norefresh(): if disk.read_only: continue if not disk.path: @@ -532,11 +533,11 @@ class vmmDomain(vmmLibvirtObject): """ Remove passed device from the inactive guest XML """ - # HACK: If serial and console are both present, they both need + # If serial and duplicate console are both present, they both need # to be removed at the same time con = None - if hasattr(devobj, "virtmanager_console_dup"): - con = getattr(devobj, "virtmanager_console_dup") + if self.serial_is_console_dup(devobj): + con = self.xmlobj.devices.consoles[0] xmlobj = self._make_xmlobj_to_define() editdev = self._lookup_device_to_define(xmlobj, devobj, False) @@ -634,7 +635,7 @@ class vmmDomain(vmmLibvirtObject): guest = self._make_xmlobj_to_define() def _change_boot_order(): boot_dev_order = [] - devmap = dict((dev.vmmidstr, dev) for dev in + devmap = dict((dev.get_xml_id(), dev) for dev in self.get_bootable_devices()) for b in boot_order: if b in devmap: @@ -708,8 +709,8 @@ class vmmDomain(vmmLibvirtObject): return used = [] - disks = (self.get_disk_devices() + - self.get_disk_devices(inactive=True)) + disks = (self.xmlobj.devices.disk + + self.get_xmlobj(inactive=True).devices.disk) for d in disks: used.append(d.target) @@ -1104,7 +1105,7 @@ class vmmDomain(vmmLibvirtObject): # Ugly workaround for VNC bug where the display cannot be opened # if the listen type is "none". This bug was fixed in QEMU-2.9.0. - graphics = self.get_graphics_devices()[0] + graphics = self.xmlobj.devices.graphics[0] if (graphics.type == "vnc" and graphics.get_first_listen_type() == "none" and not self.conn.SUPPORT_CONN_VNC_NONE_AUTH): @@ -1208,7 +1209,7 @@ class vmmDomain(vmmLibvirtObject): floppy = None net = None - for d in self.get_disk_devices(): + for d in self.xmlobj.devices.disk: if not cdrom and d.device == "cdrom": cdrom = d if not floppy and d.device == "floppy": @@ -1218,19 +1219,19 @@ class vmmDomain(vmmLibvirtObject): if cdrom and disk and floppy: break - for n in self.get_network_devices(): + for n in self.xmlobj.devices.interface: net = n break for b in boot_order: if b == "network" and net: - ret.append(net.vmmidstr) + ret.append(net.get_xml_id()) if b == "hd" and disk: - ret.append(disk.vmmidstr) + ret.append(disk.get_xml_id()) if b == "cdrom" and cdrom: - ret.append(cdrom.vmmidstr) + ret.append(cdrom.get_xml_id()) if b == "fd" and floppy: - ret.append(floppy.vmmidstr) + ret.append(floppy.get_xml_id()) return ret def _get_device_boot_order(self): @@ -1239,7 +1240,7 @@ class vmmDomain(vmmLibvirtObject): for dev in devs: if not dev.boot.order: continue - order.append((dev.vmmidstr, dev.boot.order)) + order.append((dev.get_xml_id(), dev.boot.order)) if not order: # No devices individually marked bootable, convert traditional @@ -1263,97 +1264,26 @@ class vmmDomain(vmmLibvirtObject): return (guest.os.kernel, guest.os.initrd, guest.os.dtb, guest.os.kernel_args) - # XML Device listing + def get_interface_devices_norefresh(self): + xmlobj = self.get_xmlobj(refresh_if_nec=False) + return xmlobj.devices.interface + def get_disk_devices_norefresh(self): + xmlobj = self.get_xmlobj(refresh_if_nec=False) + return xmlobj.devices.disk - def get_serial_devs(self): - devs = self.get_char_devices() - devlist = [] + def serial_is_console_dup(self, serial): + if serial.DEVICE_TYPE != "serial": + return False - devlist += [x for x in devs if x.DEVICE_TYPE == "serial"] - devlist += [x for x in devs if x.DEVICE_TYPE == "console"] - return devlist + consoles = self.xmlobj.devices.console + if not consoles: + return False - def _build_device_list(self, device_type, - refresh_if_nec=True, inactive=False): - guest = self.get_xmlobj(refresh_if_nec=refresh_if_nec, - inactive=inactive) - devs = getattr(guest.devices, device_type) - - for idx, dev in enumerate(devs): - dev.vmmindex = idx - dev.vmmidstr = dev.DEVICE_TYPE + ("%.3d" % idx) - - return devs - - def get_network_devices(self, refresh_if_nec=True): - return self._build_device_list("interface", refresh_if_nec) - def get_video_devices(self): - return self._build_device_list("video") - def get_hostdev_devices(self): - return self._build_device_list("hostdev") - def get_watchdog_devices(self): - return self._build_device_list("watchdog") - def get_input_devices(self): - return self._build_device_list("input") - def get_graphics_devices(self): - return self._build_device_list("graphics") - def get_sound_devices(self): - return self._build_device_list("sound") - def get_controller_devices(self): - return self._build_device_list("controller") - def get_filesystem_devices(self): - return self._build_device_list("filesystem") - def get_smartcard_devices(self): - return self._build_device_list("smartcard") - def get_redirdev_devices(self): - return self._build_device_list("redirdev") - def get_tpm_devices(self): - return self._build_device_list("tpm") - def get_rng_devices(self): - return self._build_device_list("rng") - def get_panic_devices(self): - return self._build_device_list("panic") - - def get_disk_devices(self, refresh_if_nec=True, inactive=False): - devs = self._build_device_list("disk", refresh_if_nec, inactive) - - # Iterate through all disks and calculate what number they are - # HACK: We are making a variable in DeviceDisk to store the index - idx_mapping = {} - for dev in devs: - devtype = dev.device - bus = dev.bus - key = devtype + (bus or "") - - if key not in idx_mapping: - idx_mapping[key] = 1 - - dev.disk_bus_index = idx_mapping[key] - idx_mapping[key] += 1 - - return devs - - def get_char_devices(self): - devs = [] - serials = self._build_device_list("serial") - parallels = self._build_device_list("parallel") - consoles = self._build_device_list("console") - channels = self._build_device_list("channel") - - for devicelist in [serials, parallels, consoles, channels]: - devs.extend(devicelist) - - # Don't display if it's just a duplicate of - if (len(consoles) > 0 and len(serials) > 0): - con = consoles[0] - ser = serials[0] - - if (con.type == ser.type and - con.target_type is None or con.target_type == "serial"): - ser.virtmanager_console_dup = con - devs.remove(con) - - return devs + console = consoles[0] + if (console.type == serial.type and + (console.target_type is None or console.target_type == "serial")): + return True + return False def can_use_device_boot_order(self): # Return 'True' if guest can use new style boot device ordering @@ -1361,14 +1291,15 @@ class vmmDomain(vmmLibvirtObject): self.conn.SUPPORT_CONN_DEVICE_BOOTORDER) def get_bootable_devices(self): - devs = self.get_disk_devices() - devs += self.get_network_devices() - devs += self.get_hostdev_devices() - # redirdev can also be marked bootable, but it should be rarely # used and clutters the UI + devs = (self.xmlobj.devices.disk + + self.xmlobj.devices.interface + + self.xmlobj.devices.hostdev) return devs + def get_serialcon_devices(self): + return self.xmlobj.devices.serial + self.xmlobj.devices.console ############################ # Domain lifecycle methods # @@ -1770,7 +1701,7 @@ class vmmDomain(vmmLibvirtObject): self._stats_net_skip = [] return rx, tx - for netdev in self.get_network_devices(refresh_if_nec=False): + for netdev in self.get_interface_devices_norefresh(): dev = netdev.target_dev if not dev: continue @@ -1820,7 +1751,7 @@ class vmmDomain(vmmLibvirtObject): self._summary_disk_stats_skip = True # did not work, iterate over all disks - for disk in self.get_disk_devices(refresh_if_nec=False): + for disk in self.get_disk_devices_norefresh(): dev = disk.target if not dev: continue diff --git a/virtManager/netlist.py b/virtManager/netlist.py index a424a317..12c09787 100644 --- a/virtManager/netlist.py +++ b/virtManager/netlist.py @@ -159,7 +159,7 @@ class vmmNetworkList(vmmGObjectUI): vnet_taps = [] for vm in self.conn.list_vms(): - for nic in vm.get_network_devices(refresh_if_nec=False): + 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) diff --git a/virtManager/serialcon.py b/virtManager/serialcon.py index d7ccce76..05ec83ae 100644 --- a/virtManager/serialcon.py +++ b/virtManager/serialcon.py @@ -415,9 +415,9 @@ class vmmSerialConsole(vmmGObject): self.console.close() def lookup_dev(self): - devs = self.vm.get_serial_devs() + devs = self.vm.get_serialcon_devices() for dev in devs: - port = dev.vmmindex + port = dev.get_xml_idx() path = dev.source_path if port == self.target_port: diff --git a/virtManager/snapshots.py b/virtManager/snapshots.py index 9cdcc0a2..c714c78e 100644 --- a/virtManager/snapshots.py +++ b/virtManager/snapshots.py @@ -400,7 +400,7 @@ class vmmSnapshotPage(vmmGObjectUI): if not self.vm.is_active(): logging.debug("Skipping screenshot since VM is not active") return - if not self.vm.get_graphics_devices(): + if not self.vm.xmlobj.devices.graphics: logging.debug("Skipping screenshot since VM has no graphics") return diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py index c7751d31..506d9b76 100644 --- a/virtinst/xmlbuilder.py +++ b/virtinst/xmlbuilder.py @@ -632,6 +632,17 @@ class XMLBuilder(object): """ return self._xmlstate.abs_xpath() + def get_xml_idx(self): + """ + This is basically the offset parsed out of the object's xpath, + minus 1. So if this is the fifth in a , ret=4. + If this is the only in a domain, ret=0. + """ + xpath = self._xmlstate.abs_xpath() + if "[" not in xpath: + return 0 + return int(xpath.rsplit("[", 1)[1].strip("]")) - 1 + ################ # Internal API #