From faf8dceb639c1c70ef2b3277cfb0a2e370706f45 Mon Sep 17 00:00:00 2001
From: David McNally <dave.mcnally@hp.com>
Date: Thu, 13 Dec 2012 15:03:13 +0000
Subject: Adding ability to specify the libvirt cache mode for disk devices
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In past versions of Nova it was possible to explicitly configure
the cache mode of disks via the libvirt XML template. The current approach
makes this a derived setting of either “none” or “writethrough” based
on the support of O_DIRECT. Whilst this provides a good set of default
settings it removes the ability of the cloud provider to use other
modes such as “writeback” and “unsafe” which are valuable in certain
configurations.

This change allows the cache mode to be specified on a per-disk type
basis. If a disk type does not have a cache mode specified then the
default behaviour remains unchanged.

DocImpact

bug 1086386

Change-Id: I3d296fe0b4b9b976db02db90ad69fd299cd2096a
---
 nova/tests/test_libvirt.py  | 38 ++++++++++++++++++++++++++++++++++++++
 nova/virt/libvirt/driver.py | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 6882bb082..333922eea 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -269,6 +269,17 @@ class FakeVolumeDriver(object):
         return ""
 
 
+class FakeConfigGuestDisk(object):
+    def __init__(self, *args, **kwargs):
+        self.source_type = None
+        self.driver_cache = None
+
+
+class FakeConfigGuest(object):
+    def __init__(self, *args, **kwargs):
+        self.driver_cache = None
+
+
 class LibvirtConnTestCase(test.TestCase):
 
     def setUp(self):
@@ -3431,6 +3442,33 @@ class LibvirtConnTestCase(test.TestCase):
         self.assertEqual(got_events[0].transition,
                          virtevent.EVENT_LIFECYCLE_STOPPED)
 
+    def test_set_cache_mode(self):
+        self.flags(disk_cachemodes=['file=directsync'])
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        fake_conf = FakeConfigGuestDisk()
+
+        fake_conf.source_type = 'file'
+        conn.set_cache_mode(fake_conf)
+        self.assertEqual(fake_conf.driver_cache, 'directsync')
+
+    def test_set_cache_mode_invalid_mode(self):
+        self.flags(disk_cachemodes=['file=FAKE'])
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        fake_conf = FakeConfigGuestDisk()
+
+        fake_conf.source_type = 'file'
+        conn.set_cache_mode(fake_conf)
+        self.assertEqual(fake_conf.driver_cache, None)
+
+    def test_set_cache_mode_invalid_object(self):
+        self.flags(disk_cachemodes=['file=directsync'])
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        fake_conf = FakeConfigGuest()
+
+        fake_conf.driver_cache = 'fake'
+        conn.set_cache_mode(fake_conf)
+        self.assertEqual(fake_conf.driver_cache, 'fake')
+
 
 class HostStateTestCase(test.TestCase):
 
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 6f1cfef6f..c19c79d79 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -203,6 +203,10 @@ libvirt_opts = [
     cfg.StrOpt('xen_hvmloader_path',
                 default='/usr/lib/xen/boot/hvmloader',
                 help='Location where the Xen hvmloader is kept'),
+    cfg.ListOpt('disk_cachemodes',
+                 default=[],
+                 help='Specific cachemodes to use for different disk types '
+                      'e.g: ["file=directsync","block=none"]'),
     ]
 
 CONF = cfg.CONF
@@ -316,6 +320,25 @@ class LibvirtDriver(driver.ComputeDriver):
         self.image_cache_manager = imagecache.ImageCacheManager()
         self.image_backend = imagebackend.Backend(CONF.use_cow_images)
 
+        self.disk_cachemodes = {}
+
+        self.valid_cachemodes = ["default",
+                                 "none",
+                                 "writethrough",
+                                 "writeback",
+                                 "directsync",
+                                 "writethrough",
+                                 "unsafe",
+                                ]
+
+        for mode_str in CONF.disk_cachemodes:
+            disk_type, sep, cache_mode = mode_str.partition('=')
+            if cache_mode not in self.valid_cachemodes:
+                LOG.warn(_("Invalid cachemode %(cache_mode)s specified "
+                           "for disk type %(disk_type)s.") % locals())
+                continue
+            self.disk_cachemodes[disk_type] = cache_mode
+
     @property
     def disk_cachemode(self):
         if self._disk_cachemode is None:
@@ -337,6 +360,18 @@ class LibvirtDriver(driver.ComputeDriver):
             self._host_state = HostState(self)
         return self._host_state
 
+    def set_cache_mode(self, conf):
+        """Set cache mode on LibvirtConfigGuestDisk object."""
+        try:
+            source_type = conf.source_type
+            driver_cache = conf.driver_cache
+        except AttributeError:
+            return
+
+        cache_mode = self.disk_cachemodes.get(source_type,
+                                              driver_cache)
+        conf.driver_cache = cache_mode
+
     def has_min_version(self, lv_ver=None, hv_ver=None, hv_type=None):
         def _munge_version(ver):
             return ver[0] * 1000000 + ver[1] * 1000 + ver[2]
@@ -875,6 +910,7 @@ class LibvirtDriver(driver.ComputeDriver):
         conf = self.volume_driver_method('connect_volume',
                                          connection_info,
                                          disk_info)
+        self.set_cache_mode(conf)
 
         try:
             # NOTE(vish): We can always affect config because our
@@ -1995,6 +2031,9 @@ class LibvirtDriver(driver.ComputeDriver):
                                                         'raw')
                 devices.append(diskconfig)
 
+        for d in devices:
+            self.set_cache_mode(d)
+
         return devices
 
     def get_guest_config_sysinfo(self, instance):
-- 
cgit