Open Stack

Hi all,

You already know about the path forward for no-db-compute from Russell's
email earlier today. On that path, we're going to have a pile of
situations where the upper layers need to know about the properties of a
virt driver in order to perform certain tasks on its behalf
(i.e. hitting the database).

The first such example is moving libvirt's manage_image_cache() method
up a layer into manager.py, but not doing the pre-work for it unless
we're on top of a driver that actually wants it. We discussed the
following approaches to handling this specific case:

1. Always doing it and saying "to hell with performance"
2. Creating a class to house the manage_image_cache() method (instead of
   ComputeDriver) and only doing it in the case that the driver inherits
   from said class.
3. Checking to see if the manage_image_cache() method has been
   implemented in the driver subclass.
4. Trying the method once, and if it throws NotImplementedError, never
   trying again
5. Checking some arbitrary property of the driver class that we expect
   to be set if it wants us to call manage_image_cache()

Most of the above solutions have obvious or subtle warts, which led the
discussion to the implementation of a capabilities object and interface,
which we can use to determine if the driver wants its
manage_image_cache() method to be called periodically. In order to
facilitate sane discussion of this approach, I'm including that patch
here.

If you have nothing to say other than +1 (or +2) it's up on Gerrit as
well:

  https://review.openstack.org/#/c/14706/

If you want to see the previous approach (using an extra class), which
would be rebased on top of this, it's here:

  https://review.openstack.org/#/c/14610/

Thoughts? Thanks!

-- 
Dan Smith
IBM Linux Technology Center

commit ea725957621c88de049c476c35c8f01cf192e5a4
Author: Dan Smith <danms at us.ibm.com>
Date:   Tue Oct 23 12:44:54 2012 -0700

    Add virt driver capabilities definition
    
    Going forward with the early stages of no-db-compute, we will
    consistently hit cases where we need to eject some database usage
    from one virt driver up a few layers. Since those are not commonly
    used by all, some way of determining (and the higher layer) the
    properties of the virt driver being used will be useful.
    
    We have discussed using something like a mixin interface class
    and multiple inheritance. We have discussed tricks like detecting
    if the manage_image_cache() method has been overridden. We have
    discussed trying it once and catching NotImplementedError and then
    never trying again. Aside from the first, none of these are
    unsneaky enough to bite us later.
    
    This approach will provide us a clean way to declare such properties
    succinctly in the compute driver to help the higher layers know
    what we want them to do on our behalf. The implementation may look a
    little heavyweight at the moment, but I think it should help us
    ensure that properties in this object get documented and typed
    appropriately going forward.
    
    Change-Id: I74dea9322a5b4688319ebf5d9afe416e93401c58

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index f598ccb..a93253f 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -234,6 +234,8 @@ class ComputeManager(manager.SchedulerDependentManager):
             LOG.error(_("Unable to load the virtualization driver: %s") % (e))
             sys.exit(1)
 
+        self.driver_capabilities = self.driver.get_capabilities()
+
         self.network_api = network.API()
         self.volume_api = volume.API()
         self.network_manager = importutils.import_object(
@@ -2970,6 +2972,8 @@ class ComputeManager(manager.SchedulerDependentManager):
     def _run_image_cache_manager_pass(self, context):
         """Run a single pass of the image cache manager."""
 
+        if not self.driver_capabilities.has_imagecache:
+            return
         if FLAGS.image_cache_manager_interval == 0:
             return
 
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 59fac11..d696a3e 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -57,6 +57,33 @@ def block_device_info_get_mapping(block_device_info):
     return block_device_mapping
 
 
+def BooleanProperty(self, name, doc, default=False):
+    """Constructs a boolean property named @name on @self, with documentation
+    @doc and a default value of @default"""
+    def fget(self):
+        return self._properties[name]
+
+    def fset(self, value):
+        self._properties[name] = bool(value)
+
+    self._properties[name] = default
+    setattr(self, name, property(fget=fget, fset=fset, doc=doc))
+
+
+class ComputeDriverCapabilities(object):
+    """Capabilities definition for a driver
+
+    Attributes are:
+      has_imagecache: If True, the driver needs to have its manage_imagecache()
+                      method called periodically. Default is False.
+    """
+    def __init__(self, **kwargs):
+        self._properties = {}
+        BooleanProperty(self, "has_imagecache",
+            "Driver requires manage_imagecache() to be called periodically")
+        map(lambda x: setattr(self, *x), kwargs.items())
+
+
 class ComputeDriver(object):
     """Base class for compute drivers.
 
@@ -88,6 +115,17 @@ class ComputeDriver(object):
 
     """
 
+    def get_capabilities(self):
+        """Return a ComputeDriverCapabilities object describing attributes
+        about this driver
+
+        Upper layers can use this to decide whether or not do to
+        special things that one driver may depend on, and optimize out
+        that behavior for others. Drivers may not implement this
+        method if they agree to the defaults.
+        """
+        return ComputeDriverCapabilities()
+
     def init_host(self, host):
         """Initialize anything that is necessary for the driver to function,
         including catching up with currently running VM's on the given host."""
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 8ef469b..4a6ca71 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -294,6 +294,9 @@ class LibvirtDriver(driver.ComputeDriver):
         self.image_cache_manager = imagecache.ImageCacheManager()
         self.image_backend = imagebackend.Backend(FLAGS.use_cow_images)
 
+    def get_capabilities(self):
+        return driver.ComputeDriverCapabilities(has_imagecache=True)
+
     @property
     def disk_cachemode(self):
         if self._disk_cachemode is None: