From ca03fe87371d8c13fc2895f8ff3e7e1ca88cfe79 Mon Sep 17 00:00:00 2001 From: Kashyap Chamarthy Date: Mon, 23 Oct 2017 16:27:01 +0200 Subject: [PATCH] libvirt: Don't disregard cache mode for instance boot disks One of the things this commit: commit 14c38ac0f253036da79f9d07aedf7dfd5778fde8 Author: Kashyap Chamarthy Date: Thu Jul 20 19:01:23 2017 +0200 libvirt: Post-migration, set cache value for Cinder volume(s) [...] did was to supposedly remove "duplicate" calls to _set_cache_mode(). But that came back to bite us. Now, while the Cinder volumes are taken care of w.r.t handling its cache value during migration, but the above referred commit (14c38ac) seemed to introduce a regression because it disregards the 'disk_cachemodes' Nova config parameter altogether for boot disks -- i.e. even though if a user set the cache mode to be 'writeback', it's ignored and instead 'none' is set unconditionally. Add the _set_cache_mode() calls back in _get_guest_storage_config(). Co-Authored-By: melanie witt Closes-Bug: #1727558 Conflicts: nova/virt/libvirt/driver.py NOTE(melwitt): The conflict is from a helper function _get_scsi_controller in ocata that doesn't exist in newton. Change-Id: I7370cc2942a6c8c51ab5355b50a9e5666cca042e (cherry picked from commit 24e79bcbf7790d1f4fea2cbdf066599cc746c2dc) (cherry picked from commit 60d6e87cac10ff1f95a028c6176e768214ec8b77) (cherry picked from commit fc10b54f25023d7e780b110928bda3a19e4c03f0) --- nova/tests/unit/virt/libvirt/test_driver.py | 43 +++++++++++++++++++++ nova/virt/libvirt/driver.py | 3 ++ 2 files changed, 46 insertions(+) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 0cc4b56969fe..adc2ac30d086 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -4835,6 +4835,49 @@ class LibvirtConnTestCase(test.NoDBTestCase): self.assertEqual(10000, cfg.cputune.shares) self.assertEqual(20000, cfg.cputune.period) + def _test_get_guest_config_disk_cachemodes(self, images_type): + # Verify that the configured cachemodes are propagated to the device + # configurations. + if images_type == 'flat': + cachemode = 'file=directsync' + elif images_type == 'lvm': + cachemode = 'block=writethrough' + elif images_type == 'rbd': + cachemode = 'network=writeback' + self.flags(disk_cachemodes=[cachemode], group='libvirt') + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + instance_ref = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, + instance_ref, + image_meta) + cfg = drvr._get_guest_config(instance_ref, [], + image_meta, disk_info) + for d in cfg.devices: + if isinstance(d, vconfig.LibvirtConfigGuestDisk): + expected = cachemode.split('=') + self.assertEqual(expected[0], d.source_type) + self.assertEqual(expected[1], d.driver_cache) + + def test_get_guest_config_disk_cachemodes_file(self): + self.flags(images_type='flat', group='libvirt') + self._test_get_guest_config_disk_cachemodes('flat') + + def test_get_guest_config_disk_cachemodes_block(self): + self.flags(images_type='lvm', group='libvirt') + self.flags(images_volume_group='vols', group='libvirt') + self._test_get_guest_config_disk_cachemodes('lvm') + + @mock.patch.object(rbd_utils, 'rbd') + @mock.patch.object(rbd_utils, 'rados') + @mock.patch.object(rbd_utils.RBDDriver, 'get_mon_addrs', + return_value=(mock.Mock(), mock.Mock())) + def test_get_guest_config_disk_cachemodes_network( + self, mock_get_mon_addrs, mock_rados, mock_rbd): + self.flags(images_type='rbd', group='libvirt') + self._test_get_guest_config_disk_cachemodes('rbd') + @mock.patch.object( host.Host, "is_cpu_control_policy_capable", return_value=True) def test_get_guest_config_with_bogus_cpu_quota(self, is_able): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 6ed4fd05845c..3f4d7939de63 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -3592,6 +3592,9 @@ class LibvirtDriver(driver.ComputeDriver): vol['connection_info'] = connection_info vol.save() + for d in devices: + self._set_cache_mode(d) + if image_meta.properties.get('hw_scsi_model'): hw_scsi_model = image_meta.properties.hw_scsi_model scsi_controller = vconfig.LibvirtConfigGuestController()