[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [PATCH 1/4] Add inspection thread.



On 06/30/2011 04:02 AM, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones redhat com>
> 
> This commit adds an inspection daemon thread which performs libguestfs
> inspection on domains when they first appear in the manager UI.
> 
> python-guestfs is not required.
> ---
>  src/virtManager/config.py     |   25 ++++++
>  src/virtManager/engine.py     |   15 ++++
>  src/virtManager/inspection.py |  182 +++++++++++++++++++++++++++++++++++++++++
>  src/virtManager/manager.py    |    3 +
>  4 files changed, 225 insertions(+), 0 deletions(-)
>  create mode 100644 src/virtManager/inspection.py
> 
> diff --git a/src/virtManager/config.py b/src/virtManager/config.py
> index 791ef4d..d3c3155 100644
> --- a/src/virtManager/config.py
> +++ b/src/virtManager/config.py
> @@ -113,6 +113,31 @@ class vmmConfig(object):
>          self._objects = []
>  
>          self.support_threading = virtinst.support.support_threading()
> +
> +        # Check that we have the guestfs module with the non-broken
> +        # support for Python threads.  Also libvirt must be
> +        # thread-safe too.
> +        self.support_inspection = False
> +        if self.support_threading:
> +            try:
> +                from guestfs import GuestFS
> +                g = GuestFS()
> +                version = g.version()
> +                if version["major"] == 1: # major must be 1
> +                    if version["minor"] == 8:
> +                        if version["release"] >= 6: # >= 1.8.6
> +                            self.support_inspection = True
> +                    elif version["minor"] == 10:
> +                        if version["release"] >= 1: # >= 1.10.1
> +                            self.support_inspection = True
> +                    elif version["minor"] == 11:
> +                        if version["release"] >= 2: # >= 1.11.2
> +                            self.support_inspection = True
> +                    elif version["minor"] >= 12:    # >= 1.12.x
> +                        self.support_inspection = True
> +            except:
> +                pass
> +
>          self._spice_error = None
>  

Minor, but could you break this logic out into a separate function

self.support_inspection = support_inspection(self.support_threading)

>          self.status_icons = {
> diff --git a/src/virtManager/engine.py b/src/virtManager/engine.py
> index 3ae1f7a..3d0cbff 100644
> --- a/src/virtManager/engine.py
> +++ b/src/virtManager/engine.py
> @@ -236,6 +236,9 @@ class vmmEngine(vmmGObject):
>          if not self.config.support_threading:
>              logging.debug("Libvirt doesn't support threading, skipping.")
>  
> +        self.inspection_thread = None
> +        self._create_inspection_thread()
> +
>          # Counter keeping track of how many manager and details windows
>          # are open. When it is decremented to 0, close the app or
>          # keep running in system tray if enabled
> @@ -529,6 +532,18 @@ class vmmEngine(vmmGObject):
>          logging.debug("Exiting app normally.")
>          gtk.main_quit()
>  
> +    def _create_inspection_thread(self):
> +        if not self.config.support_inspection:
> +            logging.debug("No inspection thread because "
> +                          "libguestfs is too old, not available, "
> +                          "or libvirt is not thread safe.")
> +            return
> +        from virtManager.inspection import vmmInspection
> +        self.inspection_thread = vmmInspection()
> +        self.inspection_thread.daemon = True
> +        self.inspection_thread.start()
> +        return
> +
>      def add_connection(self, uri, readOnly=None, autoconnect=False):
>          conn = self._check_connection(uri)
>          if conn:
> diff --git a/src/virtManager/inspection.py b/src/virtManager/inspection.py
> new file mode 100644
> index 0000000..146e021
> --- /dev/null
> +++ b/src/virtManager/inspection.py
> @@ -0,0 +1,182 @@
> +#
> +# Copyright (C) 2011 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301 USA.
> +#
> +
> +import sys
> +import time
> +import traceback
> +from Queue import Queue
> +from threading import Thread
> +
> +from guestfs import GuestFS
> +
> +import logging
> +import util
> +
> +class vmmInspection(Thread):
> +    _name = "inspection thread"
> +    _wait = 15 # seconds
> +
> +    def __init__(self):
> +        Thread.__init__(self, name=self._name)
> +        self._q = Queue()
> +        self._vmseen = dict()
> +
> +    # Called by the main thread whenever a VM is added to vmlist.
> +    def vm_added(self, connection, vmuuid):
> +        obj = (connection, vmuuid)
> +        self._q.put(obj)
> +
> +    def run(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).
> +        logging.debug("%s: waiting" % self._name)
> +        time.sleep(self._wait)
> +
> +        logging.debug("%s: ready" % self._name)
> +
> +        while True:
> +            obj = self._q.get(True)
> +            (connection, vmuuid) = obj
> +
> +            logging.debug("%s: processing started on '%s'" %
> +                          (self._name, vmuuid))
> +            try:
> +                self._process(connection, vmuuid)
> +            except:
> +                logging.debug("%s: exception while processing '%s':\n%s" %
> +                              (self._name, vmuuid, traceback.format_exc()))

You can just use logging.exception here, which will autolog the
traceback. It logs at a different level but in virt-manager we really
don't distinguish.

> +
> +            self._q.task_done()
> +            logging.debug("%s: processing done on '%s'" %
> +                          (self._name, vmuuid))
> +
> +    def _process(self, connection, vmuuid):
> +        if not connection or connection.is_remote():
> +            logging.debug("%s: %s: no connection object or "
> +                          "connection is remote" % (self._name, vmuuid))
> +            return
> +        vm = connection.get_vm(vmuuid)
> +
> +        # If we've already inspected this VM, don't do it again.  Note
> +        # that we must filter here (not before adding to the queue in
> +        # the main thread) because of the case where the VM has
> +        # already been enqueued but the results are not yet in the
> +        # cache.
> +        if vmuuid in self._vmseen: return
> +
> +        xml = vm.get_xml()
> +
> +        # Add the disks.  Note they *must* be added with readonly flag set.
> +        g = GuestFS()
> +        for disk in vm.get_disk_devices():
> +            path = disk.path
> +            driver_type = disk.driver_type
> +            if (disk.type == "block" or disk.type == "file") and path != None:
> +                g.add_drive_opts(path, readonly=1, format=driver_type)
> +
> +        g.launch()
> +
> +        # Inspect the operating system.
> +        roots = g.inspect_os()
> +        if len(roots) == 0:
> +            logging.debug("%s: %s: no operating systems found" %
> +                          (self._name, vmuuid))
> +            return
> +
> +        # Arbitrarily pick the first root device.
> +        root = roots[0]
> +
> +        # Inspection results.
> +        typ = g.inspect_get_type(root) # eg. "linux"
> +        distro = g.inspect_get_distro(root) # eg. "fedora"
> +        major_version = g.inspect_get_major_version(root) # eg. 14
> +        minor_version = g.inspect_get_minor_version(root) # eg. 0
> +        hostname = g.inspect_get_hostname(root) # string
> +        product_name = g.inspect_get_product_name(root) # string
> +
> +        # Added in libguestfs 1.9.13:
> +        try:
> +            product_variant = g.inspect_get_product_variant(root) # string
> +        except AttributeError:
> +            pass
> +
> +        # For inspect_list_applications and inspect_get_icon we
> +        # require that the guest filesystems are mounted.  However
> +        # don't fail if this is not possible (I'm looking at you,
> +        # FreeBSD).
> +        filesystems_mounted = False
> +        try:
> +            # Mount up the disks, like guestfish --ro -i.
> +
> +            # Sort keys by length, shortest first, so that we end up
> +            # mounting the filesystems in the correct order.
> +            mps = g.inspect_get_mountpoints (root)

The space before parenths should trip up make check-pylint, please run
that script before submitting patches.

> +            def compare (a, b):
> +                if len(a[0]) > len(b[0]):
> +                    return 1
> +                elif len(a[0]) == len(b[0]):
> +                    return 0
> +                else:
> +                    return -1
> +            mps.sort (compare)
> +
> +            for mp_dev in mps:
> +                try:
> +                    g.mount_ro (mp_dev[1], mp_dev[0])
> +                except:
> +                    logging.debug ("%s: exception mounting %s on %s "
> +                                   "(ignored):\n%s" % (
> +                            self._name,
> +                            mp_dev[1], mp_dev[0],
> +                            traceback.format_exc()))
> +
> +            filesystems_mounted = True
> +        except:
> +            logging.debug("%s: exception while mounting disks (ignored):\n%s" %
> +                          (self._name, vmuuid, traceback.format_exc()))
> +
> +        if filesystems_mounted:
> +            # Added in libguestfs 1.11.12:
> +            try:
> +                # string containing PNG data
> +                icon = g.inspect_get_icon(root, favicon=0, highquality=1)
> +                if icon == "":
> +                    icon = None
> +            except AttributeError:
> +                pass
> +
> +            # Inspection applications.
> +            apps = g.inspect_list_applications(root)
> +
> +        # Force the libguestfs handle to close right now.
> +        del g
> +
> +        self._vmseen[vmuuid] = True
> +
> +        # Log what we found.
> +        logging.debug("%s: detected operating system: %s %s %d.%d (%s)",
> +                      self._name, typ, distro, major_version, minor_version,
> +                      product_name)
> +        logging.debug("%s: hostname: %s", self._name, hostname)
> +        if icon:
> +            logging.debug("%s: icon: %d bytes", self._name, len(icon))
> +        if apps:
> +            logging.debug("%s: # apps: %d", self._name, len(apps))
> diff --git a/src/virtManager/manager.py b/src/virtManager/manager.py
> index b1837ee..c7cca67 100644
> --- a/src/virtManager/manager.py
> +++ b/src/virtManager/manager.py
> @@ -685,6 +685,9 @@ class vmmManager(vmmGObjectUI):
>  
>          self._append_vm(model, vm, connection)
>  
> +        if self.engine.inspection_thread:
> +            self.engine.inspection_thread.vm_added(connection, vmuuid)
> +

This method isn't really sufficient, since it won't track VM removal. I
think the easiest thing to do is just track connections, and for each
run of the inspection thread, iterate over all VMs of each open connection.

In engine.py when you create the inspection thread, do something like

self.connect("connection-added", self.inspection_thread.conn_added)
self.connect("connection-removed", self.inspection_thread.conn_removed)

Then use those callbacks to just build up a list of currently known
URIs. That will also allow you to do the conn.is_remote() check once for
each connection.

Then when the inspection thread runs, get the VM list with:

for uuid in conn.list_vm_uuids():
    self._process(conn.get_vm(uuid))

Looks good otherwise!

Thanks,
Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]