From 6b1278ccda1bf52fd51c619858dfeaffb12a4029 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Tue, 13 Mar 2018 13:00:59 -0400 Subject: [PATCH] prefs: Add a setting to enable/disable libguestfs inspection Rather than key it on the library being available. Makes it much easier to test both modes of behavior. Fix up a few inspection bugs while I'm in the area, and convert it to be more singleton like. --- .../org.virt-manager.virt-manager.gschema.xml | 6 + ui/preferences.ui | 65 +++++++- virtManager/config.py | 29 ++-- virtManager/details.py | 93 +++++------ virtManager/engine.py | 37 ++--- virtManager/inspection.py | 149 +++++++++++++----- virtManager/preferences.py | 24 +++ 7 files changed, 273 insertions(+), 130 deletions(-) diff --git a/data/org.virt-manager.virt-manager.gschema.xml b/data/org.virt-manager.virt-manager.gschema.xml index d88450f5..f6db721b 100644 --- a/data/org.virt-manager.virt-manager.gschema.xml +++ b/data/org.virt-manager.virt-manager.gschema.xml @@ -47,6 +47,12 @@ Show system tray icon while app is running + + true + Enable libguestfs VM inspection + Enable libguestfs VM inspection for things like OS icons, installed applications, etc. This only works if python libguestfs bindings are installed. + + 0 Default manager window height diff --git a/ui/preferences.ui b/ui/preferences.ui index 81802a28..01b54602 100644 --- a/ui/preferences.ui +++ b/ui/preferences.ui @@ -1,5 +1,5 @@ - + @@ -49,6 +49,7 @@ True True False + start True True @@ -58,6 +59,68 @@ 0 + + + True + False + 3 + + + Enable libgues_tfs VM introspection + True + True + False + start + True + True + + + + 0 + 0 + + + + + False + 3 + + + True + False + gtk-dialog-warning + + + False + True + 0 + + + + + True + False + <small>You must restart the application for this change to take effect</small> + True + + + False + True + 1 + + + + + 0 + 1 + + + + + 0 + 1 + + diff --git a/virtManager/config.py b/virtManager/config.py index de66f80c..c7d3f082 100644 --- a/virtManager/config.py +++ b/virtManager/config.py @@ -25,6 +25,8 @@ from gi.repository import GLib from gi.repository import Gtk from virtinst import CPU + +from .inspection import vmmInspection from .keyring import vmmKeyring, vmmSecret @@ -215,19 +217,6 @@ class vmmConfig(object): self._objects = [] - self.support_inspection = self.check_inspection() - - self._spice_error = None - - - def check_inspection(self): - try: - # Check we can open the Python guestfs module. - from guestfs import GuestFS # pylint: disable=import-error - g = GuestFS(close_on_exit=False) - return bool(getattr(g, "add_libvirt_dom", None)) - except Exception: - return False # General app wide helpers (gsettings agnostic) @@ -242,6 +231,11 @@ class vmmConfig(object): ret = ["vnc", "spice"] return ret + def inspection_supported(self): + if not vmmInspection.libguestfs_installed(): + return False + return self.get_libguestfs_inspect_vms() + def remove_notifier(self, h): self.conf.notify_remove(h) @@ -379,7 +373,6 @@ class vmmConfig(object): def get_confirm_delstorage(self): return self.conf.get("/confirm/delete-storage") - def set_confirm_forcepoweroff(self, val): self.conf.set("/confirm/forcepoweroff", val) def set_confirm_poweroff(self, val): @@ -404,6 +397,14 @@ class vmmConfig(object): def set_view_system_tray(self, val): self.conf.set("/system-tray", val) + # Libguestfs VM inspection + def on_libguestfs_inspect_vms_changed(self, cb): + return self.conf.notify_add("/enable-libguestfs-vm-inspection", cb) + def get_libguestfs_inspect_vms(self): + return self.conf.get("/enable-libguestfs-vm-inspection") + def set_libguestfs_inspect_vms(self, val): + self.conf.set("/enable-libguestfs-vm-inspection", val) + # Stats history and interval length def get_stats_history_length(self): diff --git a/virtManager/details.py b/virtManager/details.py index eeeea4b9..2c7a4525 100644 --- a/virtManager/details.py +++ b/virtManager/details.py @@ -2370,61 +2370,62 @@ class vmmDetails(vmmGObjectUI): self.widget(name).set_value(int(IdMap_proper)) def refresh_inspection_page(self): - inspection_supported = self.config.support_inspection + inspection_supported = self.config.inspection_supported() uiutil.set_grid_row_visible(self.widget("details-overview-error"), self.vm.inspection.error) if self.vm.inspection.error: msg = _("Error while inspecting the guest configuration") self.widget("details-overview-error").set_text(msg) - # Operating System (ie. inspection data) self.widget("details-inspection-os").set_visible(inspection_supported) - if inspection_supported: - hostname = self.vm.inspection.hostname - if not hostname: - hostname = _("unknown") - self.widget("inspection-hostname").set_text(hostname) - os_type = self.vm.inspection.os_type - if not os_type: - os_type = "unknown" - self.widget("inspection-type").set_text(_label_for_os_type(os_type)) - product_name = self.vm.inspection.product_name - if not product_name: - product_name = _("unknown") - self.widget("inspection-product-name").set_text(product_name) + self.widget("details-inspection-apps").set_visible(inspection_supported) + if not inspection_supported: + return + + # Operating System (ie. inspection data) + hostname = self.vm.inspection.hostname + if not hostname: + hostname = _("unknown") + self.widget("inspection-hostname").set_text(hostname) + os_type = self.vm.inspection.os_type + if not os_type: + os_type = "unknown" + self.widget("inspection-type").set_text(_label_for_os_type(os_type)) + product_name = self.vm.inspection.product_name + if not product_name: + product_name = _("unknown") + self.widget("inspection-product-name").set_text(product_name) # Applications (also inspection data) - self.widget("details-inspection-apps").set_visible(inspection_supported) - if inspection_supported: - apps = self.vm.inspection.applications or [] - apps_list = self.widget("inspection-apps") - apps_model = apps_list.get_model() - apps_model.clear() - for app in apps: - name = "" - if app["app_name"]: - name = app["app_name"] - if app["app_display_name"]: - name = app["app_display_name"] - version = "" - if app["app_epoch"] > 0: - version += str(app["app_epoch"]) + ":" - if app["app_version"]: - version += app["app_version"] - if app["app_release"]: - version += "-" + app["app_release"] - summary = "" - if app["app_summary"]: - summary = app["app_summary"] - elif app["app_description"]: - summary = app["app_description"] - pos = summary.find("\n") - if pos > -1: - summary = _("%(summary)s ...") % { - "summary": summary[0:pos] - } + apps = self.vm.inspection.applications or [] + apps_list = self.widget("inspection-apps") + apps_model = apps_list.get_model() + apps_model.clear() + for app in apps: + name = "" + if app["app_name"]: + name = app["app_name"] + if app["app_display_name"]: + name = app["app_display_name"] + version = "" + if app["app_epoch"] > 0: + version += str(app["app_epoch"]) + ":" + if app["app_version"]: + version += app["app_version"] + if app["app_release"]: + version += "-" + app["app_release"] + summary = "" + if app["app_summary"]: + summary = app["app_summary"] + elif app["app_description"]: + summary = app["app_description"] + pos = summary.find("\n") + if pos > -1: + summary = _("%(summary)s ...") % { + "summary": summary[0:pos] + } - apps_model.append([name, version, summary]) + apps_model.append([name, version, summary]) def refresh_stats_page(self): def _multi_color(text1, text2): @@ -3070,7 +3071,7 @@ class vmmDetails(vmmGObjectUI): add_hw_list_option(_("Overview"), HW_LIST_TYPE_GENERAL, "computer") if not self.is_customize_dialog: - if self.config.support_inspection: + if self.config.inspection_supported(): add_hw_list_option(_("OS information"), HW_LIST_TYPE_INSPECTION, "computer") add_hw_list_option(_("Performance"), HW_LIST_TYPE_STATS, diff --git a/virtManager/engine.py b/virtManager/engine.py index d330bac4..47d96ec7 100644 --- a/virtManager/engine.py +++ b/virtManager/engine.py @@ -34,14 +34,15 @@ from .baseclass import vmmGObject from .clone import vmmCloneVM from .connect import vmmConnect from .connection import vmmConnection +from .create import vmmCreate +from .delete import vmmDeleteDialog +from .details import vmmDetails +from .error import vmmErrorDialog +from .host import vmmHost +from .inspection import vmmInspection from .manager import vmmManager from .migrate import vmmMigrateDialog -from .details import vmmDetails -from .create import vmmCreate -from .host import vmmHost -from .error import vmmErrorDialog from .systray import vmmSystray -from .delete import vmmDeleteDialog DETAILS_PERF = 1 DETAILS_CONFIG = 2 @@ -93,8 +94,7 @@ class vmmEngine(vmmGObject): self._tick_thread.daemon = True self._tick_queue = queue.PriorityQueue(100) - self.inspection = None - self._create_inspection_thread() + vmmInspection.get_instance(self) # Counter keeping track of how many manager and details windows # are open. When it is decremented to 0, close the app or @@ -394,10 +394,6 @@ class vmmEngine(vmmGObject): def _cleanup(self): self.err = None - if self.inspection: - self.inspection.cleanup() - self.inspection = None - if self.timer is not None: GLib.source_remove(self.timer) @@ -462,19 +458,6 @@ class vmmEngine(vmmGObject): logging.debug("Exiting app normally.") self._application.quit() - def _create_inspection_thread(self): - logging.debug("libguestfs inspection support: %s", - self.config.support_inspection) - if not self.config.support_inspection: - return - - from .inspection import vmmInspection - self.inspection = vmmInspection() - self.inspection.start() - self.connect("conn-added", self.inspection.conn_added) - self.connect("conn-removed", self.inspection.conn_removed) - return - def _find_error_parent_cb(self): """ Search over the toplevel windows for any that are visible or have @@ -858,12 +841,14 @@ class vmmEngine(vmmGObject): src.err.show_err(_("Error setting clone parameters: %s") % str(e)) def _do_refresh_inspection(self, src_ignore, uri, connkey): - if not self.inspection: + inspection = vmmInspection.get_instance(self) + if not inspection: return conn = self._lookup_conn(uri) vm = conn.get_vm(connkey) - self.inspection.vm_refresh(vm) + inspection.vm_refresh(vm) + ########################################## # Window launchers from virt-manager cli # diff --git a/virtManager/inspection.py b/virtManager/inspection.py index 63ffcf09..02997fa4 100644 --- a/virtManager/inspection.py +++ b/virtManager/inspection.py @@ -17,11 +17,10 @@ # MA 02110-1301 USA. # -from queue import Queue -from threading import Thread +import functools import logging - -from guestfs import GuestFS # pylint: disable=import-error +import queue +import threading from .baseclass import vmmGObject from .domain import vmmInspectionData @@ -30,22 +29,55 @@ from .domain import vmmInspectionData class vmmInspection(vmmGObject): # Can't find a way to make Thread release our reference _leak_check = False + _instance = None + _libguestfs_installed = None - def __init__(self): + @classmethod + def get_instance(cls, engine): + if not cls._instance: + if not cls.libguestfs_installed(): + return None + cls._instance = cls(engine) + return cls._instance + + @classmethod + def libguestfs_installed(cls): + if cls._libguestfs_installed is None: + try: + import guestfs as ignore # pylint: disable=import-error + logging.debug("python guestfs is installed") + cls._libguestfs_installed = True + except ImportError: + logging.debug("python guestfs is not installed") + cls._libguestfs_installed = False + except Exception: + logging.debug("error importing guestfs", + exc_info=True) + cls._libguestfs_installed = False + return cls._libguestfs_installed + + def __init__(self, engine): vmmGObject.__init__(self) - self._thread = Thread(name="inspection thread", target=self._run) - self._thread.daemon = True + self._thread = None self._wait = 5 * 1000 # 5 seconds - self._q = Queue() + self._q = queue.Queue() self._conns = {} self._vmseen = {} self._cached_data = {} + val = self.config.get_libguestfs_inspect_vms() + logging.debug("libguestfs gsetting enabled=%s", str(val)) + if not val: + return + engine.connect("conn-added", self._conn_added) + engine.connect("conn-removed", self._conn_removed) + self._start() + def _cleanup(self): - self._thread = None - self._q = Queue() + self._stop() + self._q = queue.Queue() self._conns = {} self._vmseen = {} self._cached_data = {} @@ -53,16 +85,16 @@ class vmmInspection(vmmGObject): # Called by the main thread whenever a connection is added or # removed. We tell the inspection thread, so it can track # connections. - def conn_added(self, engine_ignore, conn): + def _conn_added(self, engine_ignore, conn): obj = ("conn_added", conn) self._q.put(obj) - def conn_removed(self, engine_ignore, uri): + def _conn_removed(self, engine_ignore, uri): obj = ("conn_removed", uri) self._q.put(obj) # Called by the main thread whenever a VM is added to vmlist. - def vm_added(self, conn, connkey): + def _vm_added(self, conn, connkey): if connkey.startswith("guestfs-"): logging.debug("ignore libvirt/guestfs temporary VM %s", connkey) @@ -75,58 +107,80 @@ class vmmInspection(vmmGObject): obj = ("vm_refresh", vm.conn.get_uri(), vm.get_name(), vm.get_uuid()) self._q.put(obj) - def start(self): - # Wait a few seconds before we do anything. This prevents - # inspection from being a burden for initial virt-manager - # interactivity (although it shouldn't affect interactivity at - # all). + def _start(self): + if self._thread: + return + def cb(): - self._thread.start() + if self._thread: + self._thread.start() return 0 - logging.debug("waiting") + self._thread = threading.Thread( + name="inspection thread", target=self._run) + self._thread.daemon = True + + # Wait a few seconds before we do anything. This prevents + # inspection from being a burden for initial virt-manager + # interactivity (although it shouldn't affect interactivity at all) + logging.debug("waiting before startup wait=%s", self._wait) self.timeout_add(self._wait, cb) + def _stop(self): + if self._thread is None: + return + + self._q.put(None) + self._thread = None + def _run(self): # Process everything on the queue. If the queue is empty when # called, block. while True: obj = self._q.get() + if obj is None: + logging.debug("libguestfs queue obj=None, exiting thread") + return self._process_queue_item(obj) self._q.task_done() def _process_queue_item(self, obj): - if obj[0] == "conn_added": + cmd = obj[0] + if cmd == "conn_added": conn = obj[1] uri = conn.get_uri() - if conn and not (conn.is_remote()) and not (uri in self._conns): - self._conns[uri] = conn - conn.connect("vm-added", self.vm_added) - for vm in conn.list_vms(): - self.vm_added(conn, vm.get_connkey()) - elif obj[0] == "conn_removed": + if conn.is_remote() or uri in self._conns: + return + + self._conns[uri] = conn + conn.connect("vm-added", self._vm_added) + for vm in conn.list_vms(): + self._vm_added(conn, vm.get_connkey()) + + elif cmd == "conn_removed": uri = obj[1] - del self._conns[uri] - elif obj[0] == "vm_added" or obj[0] == "vm_refresh": + self._conns.pop(uri) + + elif cmd == "vm_added" or cmd == "vm_refresh": uri = obj[1] - if not (uri in self._conns): + if uri not in self._conns: # This connection disappeared in the meanwhile. return + conn = self._conns[uri] - if not conn.is_active(): - return - connkey = obj[2] - vm = conn.get_vm(connkey) + vm = conn.get_vm(obj[2]) if not vm: # The VM was removed in the meanwhile. return - if obj[0] == "vm_refresh": + + if cmd == "vm_refresh": vmuuid = obj[3] # When refreshing the inspection data of a VM, # all we need is to remove it from the "seen" cache, # as the data itself will be replaced once the new # results are available. - del self._vmseen[vmuuid] + self._vmseen.pop(vmuuid) + self._process_vm(conn, vm) # Try processing a single VM, keeping into account whether it was @@ -167,12 +221,21 @@ class vmmInspection(vmmGObject): logging.exception("%s: exception while processing", prettyvm) def _inspect_vm(self, conn, vm): - g = GuestFS(close_on_exit=False) + if self._thread is None: + return + + import guestfs # pylint: disable=import-error + + g = guestfs.GuestFS(close_on_exit=False) prettyvm = conn.get_uri() + ":" + vm.get_name() - - g.add_libvirt_dom(vm.get_backend(), readonly=1) - - g.launch() + try: + g.add_libvirt_dom(vm.get_backend(), readonly=1) + g.launch() + except Exception as e: + logging.debug("%s: Error launching libguestfs appliance: %s", + prettyvm, str(e)) + return None + logging.debug("%s: inspection appliance connected", prettyvm) # Inspect the operating system. roots = g.inspect_os() @@ -210,8 +273,8 @@ class vmmInspection(vmmGObject): return 0 else: return -1 - mps.sort(compare) + mps.sort(key=functools.cmp_to_key(compare)) for mp_dev in mps: try: g.mount_ro(mp_dev[1], mp_dev[0]) @@ -261,7 +324,7 @@ class vmmInspection(vmmGObject): data.product_name = str(product_name) data.product_variant = str(product_variant) data.icon = icon - data.applications = list(apps) + data.applications = list(apps or []) data.error = False return data diff --git a/virtManager/preferences.py b/virtManager/preferences.py index a4d57808..11ee6b37 100644 --- a/virtManager/preferences.py +++ b/virtManager/preferences.py @@ -25,6 +25,7 @@ from gi.repository import Gdk from . import uiutil from .baseclass import vmmGObjectUI +from .inspection import vmmInspection class vmmPreferences(vmmGObjectUI): @@ -45,7 +46,10 @@ class vmmPreferences(vmmGObjectUI): self._init_ui() + self._orig_libguestfs_val = None + self.refresh_view_system_tray() + self.refresh_libguestfs() self.refresh_update_interval() self.refresh_console_accels() self.refresh_console_scaling() @@ -73,6 +77,7 @@ class vmmPreferences(vmmGObjectUI): "on_prefs_close_clicked": self.close, "on_prefs_system_tray_toggled": self.change_view_system_tray, + "on_prefs_libguestfs_toggled": self.change_libguestfs, "on_prefs_stats_update_interval_changed": self.change_update_interval, "on_prefs_console_accels_toggled": self.change_console_accels, "on_prefs_console_scaling_changed": self.change_console_scaling, @@ -181,6 +186,11 @@ class vmmPreferences(vmmGObjectUI): combo.set_model(model) uiutil.init_combo_text_column(combo, 1) + if not vmmInspection.libguestfs_installed(): + self.widget("prefs-libguestfs").set_sensitive(False) + self.widget("prefs-libguestfs").set_tooltip_text( + _("python libguestfs support is not installed")) + ######################### # Config Change Options # @@ -190,6 +200,12 @@ class vmmPreferences(vmmGObjectUI): val = self.config.get_view_system_tray() self.widget("prefs-system-tray").set_active(bool(val)) + def refresh_libguestfs(self): + val = self.config.get_libguestfs_inspect_vms() + if self._orig_libguestfs_val is None: + self._orig_libguestfs_val = val + self.widget("prefs-libguestfs").set_active(bool(val)) + def refresh_update_interval(self): self.widget("prefs-stats-update-interval").set_value( self.config.get_stats_update_interval()) @@ -341,6 +357,14 @@ class vmmPreferences(vmmGObjectUI): def change_view_system_tray(self, src): self.config.set_view_system_tray(src.get_active()) + def change_libguestfs(self, src): + val = src.get_active() + self.config.set_libguestfs_inspect_vms(val) + + vis = (val != self._orig_libguestfs_val and + self.widget("prefs-libguestfs").get_sensitive()) + uiutil.set_grid_row_visible( + self.widget("prefs-libguestfs-warn-box"), vis) def change_update_interval(self, src): self.config.set_stats_update_interval(src.get_value_as_int())