Merge "libvirt: Fix initialising of LVM ephemeral disks"

nova/tests/unit/virt/libvirt/test_imagebackend.py

@@ -732,6 +732,53 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
 
         self.mox.VerifyAll()
 
+    @mock.patch('os.path.exists', autospec=True)
+    @mock.patch('nova.utils.synchronized', autospec=True)
+    @mock.patch.object(imagebackend, 'lvm', autospec=True)
+    def test_cache_ephemeral(self, mock_lvm, mock_synchronized, mock_exists):
+        # Ignores its arguments and returns the wrapped function unmodified
+        def fake_synchronized(*args, **kwargs):
+            def outer(fn):
+                def wrapper(*wargs, **wkwargs):
+                    fn(*wargs, **wkwargs)
+                return wrapper
+
+            return outer
+
+        mock_synchronized.side_effect = fake_synchronized
+
+        # Fake exists returns true for paths which have been added to the
+        # exists set
+        exists = set()
+
+        def fake_exists(path):
+            return path in exists
+
+        mock_exists.side_effect = fake_exists
+
+        # Fake create_volume causes exists to return true for the volume
+        def fake_create_volume(vg, lv, size, sparse=False):
+            exists.add(os.path.join('/dev', vg, lv))
+
+        mock_lvm.create_volume.side_effect = fake_create_volume
+
+        # Assert that when we call cache() for an ephemeral disk with the
+        # Lvm backend, we call fetch_func with a target of the Lvm disk
+        size_gb = 1
+        size = size_gb * units.Gi
+
+        fetch_func = mock.MagicMock()
+        image = self.image_class(self.INSTANCE, self.NAME)
+        image.cache(fetch_func, self.TEMPLATE,
+                    ephemeral_size=size_gb, size=size)
+
+        mock_lvm.create_volume.assert_called_once_with(self.VG, self.LV, size,
+                                                       sparse=False)
+        fetch_func.assert_called_once_with(target=self.PATH,
+                                           ephemeral_size=size_gb)
+
+        mock_synchronized.assert_called()
+
     def test_create_image(self):
         self._create_image(False)
 

nova/virt/libvirt/imagebackend.py

@@ -200,19 +200,28 @@ class Image(object):
         :filename: Name of the file in the image directory
         :size: Size of created image in bytes (optional)
         """
-        @utils.synchronized(filename, external=True, lock_path=self.lock_path)
-        def fetch_func_sync(target, *args, **kwargs):
-            # The image may have been fetched while a subsequent
-            # call was waiting to obtain the lock.
-            if not os.path.exists(target):
-                fetch_func(target=target, *args, **kwargs)
-
         base_dir = os.path.join(CONF.instances_path,
                                 CONF.image_cache_subdirectory_name)
         if not os.path.exists(base_dir):
             fileutils.ensure_tree(base_dir)
         base = os.path.join(base_dir, filename)
 
+        @utils.synchronized(filename, external=True, lock_path=self.lock_path)
+        def fetch_func_sync(target, *args, **kwargs):
+            # NOTE(mdbooth): This method is called as a callback by the
+            # create_image() method of a specific backend. It assumes that
+            # target will be in the image cache, which is why it holds a
+            # lock, and does not overwrite an existing file. However,
+            # this is not true for all backends. Specifically Lvm writes
+            # directly to the target rather than to the image cache,
+            # and additionally it creates the target in advance.
+            # This guard is only relevant in the context of the lock if the
+            # target is in the image cache. If it isn't, we should
+            # call fetch_func. The lock we're holding is also unnecessary in
+            # that case, but it will not result in incorrect behaviour.
+            if target != base or not os.path.exists(target):
+                fetch_func(target=target, *args, **kwargs)
+
         if not self.exists() or not os.path.exists(base):
             self.create_image(fetch_func_sync, base, size,
                               *args, **kwargs)