optimize libvirt raw image handling. Bug 924970

Tests were seen to time-out on libvirt when raw images were
used, which was due to large disk images being copied around
inefficiently.  A system with standard disks was seen to take
an extra 60s/10G which was a problem with large root and
ephemeral disks.  The changes below attempt to minimize the
I/O in dealing with cached raw images.  These changes should
also help to minimize the disk space used for such images,
by avoiding the naïve copy which undoes the sparseness.

* nova/virt/libvirt/connection.py (_cache_image): Do the
resize here, rather than in _fetch_image(), so that we
can control when the resizing is done, to minimize the
amount of data that needs to be copied.  Also if we're
generating rather than fetching the image, then just
generate in the instance dir too, as this should be faster.
* nova/tests/fake_libvirt_utils.py: Remove the resize
functionality since it's no longer used.
* nova/tests/test_libvirt.py: Likewise.
* nova/virt/libvirt/utils.py (fetch_image): Likewise.
(copy_image): Shell out to cp since it deals better
with sparse files.  Note the above changes avoid sparse
copies, so this is just an ancillary improvement in the area.

Change-Id: I678d125c61aab56c62c668559eb2220d56702952

nova/tests/fake_libvirt_utils.py

@@ -99,6 +99,5 @@ def get_fs_info(path):
             'free': 84 * (1024 ** 3)}
 
 
-def fetch_image(context, target, image_id, user_id, project_id,
-                 size=None):
+def fetch_image(context, target, image_id, user_id, project_id):
     pass

nova/tests/test_libvirt.py

@@ -210,12 +210,17 @@ def fake_exists(fname):
         def fake_execute(*args, **kwargs):
             pass
 
+        def fake_extend(image, size):
+            pass
+
         self.stubs.Set(os.path, 'exists', fake_exists)
         self.stubs.Set(utils, 'execute', fake_execute)
         connection.libvirt_utils = fake_libvirt_utils
+        connection.disk.extend = fake_extend
 
     def tearDown(self):
         connection.libvirt_utils = libvirt_utils
+        connection.disk.extend = disk.extend
         super(CacheConcurrencyTestCase, self).tearDown()
 
     def test_same_fname_concurrency(self):
@@ -224,11 +229,11 @@ def test_same_fname_concurrency(self):
         wait1 = eventlet.event.Event()
         done1 = eventlet.event.Event()
         eventlet.spawn(conn._cache_image, _concurrency,
-                       'target', 'fname', False, wait1, done1)
+                       'target', 'fname', False, None, wait1, done1)
         wait2 = eventlet.event.Event()
         done2 = eventlet.event.Event()
         eventlet.spawn(conn._cache_image, _concurrency,
-                       'target', 'fname', False, wait2, done2)
+                       'target', 'fname', False, None, wait2, done2)
         wait2.send()
         eventlet.sleep(0)
         try:
@@ -245,11 +250,11 @@ def test_different_fname_concurrency(self):
         wait1 = eventlet.event.Event()
         done1 = eventlet.event.Event()
         eventlet.spawn(conn._cache_image, _concurrency,
-                       'target', 'fname2', False, wait1, done1)
+                       'target', 'fname2', False, None, wait1, done1)
         wait2 = eventlet.event.Event()
         done2 = eventlet.event.Event()
         eventlet.spawn(conn._cache_image, _concurrency,
-                       'target', 'fname1', False, wait2, done2)
+                       'target', 'fname1', False, None, wait2, done2)
         wait2.send()
         eventlet.sleep(0)
         try:
@@ -292,8 +297,14 @@ def setUp(self):
         self.call_libvirt_dependant_setup = False
         connection.libvirt_utils = fake_libvirt_utils
 
+        def fake_extend(image, size):
+            pass
+
+        connection.disk.extend = fake_extend
+
     def tearDown(self):
         connection.libvirt_utils = libvirt_utils
+        connection.disk.extend = disk.extend
         super(LibvirtConnTestCase, self).tearDown()
 
     test_instance = {'memory_kb': '1024000',
@@ -1915,19 +1926,14 @@ def test_get_fs_info(self):
 
     def test_fetch_image(self):
         self.mox.StubOutWithMock(images, 'fetch')
-        self.mox.StubOutWithMock(disk, 'extend')
 
         context = 'opaque context'
         target = '/tmp/targetfile'
         image_id = '4'
         user_id = 'fake'
         project_id = 'fake'
         images.fetch(context, image_id, target, user_id, project_id)
-        images.fetch(context, image_id, target, user_id, project_id)
-        disk.extend(target, '10G')
 
         self.mox.ReplayAll()
         libvirt_utils.fetch_image(context, target, image_id,
                                   user_id, project_id)
-        libvirt_utils.fetch_image(context, target, image_id,
-                                  user_id, project_id, size='10G')

nova/virt/libvirt/connection.py

@@ -799,7 +799,7 @@ def get_vnc_port_for_instance(instance_name):
         return {'host': host, 'port': port, 'internal_access_path': None}
 
     @staticmethod
-    def _cache_image(fn, target, fname, cow=False, *args, **kwargs):
+    def _cache_image(fn, target, fname, cow=False, size=None, *args, **kwargs):
         """Wrapper for a method that creates an image that caches the image.
 
         This wrapper will save the image into a common store and create a
@@ -812,8 +812,12 @@ def _cache_image(fn, target, fname, cow=False, *args, **kwargs):
         to be unique to a given image.
 
         If cow is True, it will make a CoW image instead of a copy.
+
+        If size is specified, we attempt to resize up to that size.
         """
 
+        generating = 'image_id' not in kwargs
+
         if not os.path.exists(target):
             base_dir = os.path.join(FLAGS.instances_path, '_base')
             if not os.path.exists(base_dir):
@@ -825,20 +829,36 @@ def call_if_not_exists(base, fn, *args, **kwargs):
                 if not os.path.exists(base):
                     fn(target=base, *args, **kwargs)
 
-            call_if_not_exists(base, fn, *args, **kwargs)
+            if cow or not generating:
+                call_if_not_exists(base, fn, *args, **kwargs)
+            elif generating:
+                # For raw it's quicker to generate both
+                # FIXME(p-draigbrady) the first call here is probably
+                # redundant, as it's of no benefit to cache in base?
+                call_if_not_exists(base, fn, *args, **kwargs)
+                if os.path.exists(target):
+                    os.unlink(target)
+                fn(target=target, *args, **kwargs)
 
             if cow:
+                if size:
+                    disk.extend(base, size)
                 libvirt_utils.create_cow_image(base, target)
-            else:
+            elif not generating:
                 libvirt_utils.copy_image(base, target)
+                # Resize after the copy, as it's usually much faster
+                # to make sparse updates, rather than potentially
+                # naively copying the whole image file.
+                if size:
+                    # FIXME(p-draigbrady) the first call here is probably
+                    # redundant, as it's of no benefit have full size in base?
+                    disk.extend(base, size)
+                    disk.extend(target, size)
 
     @staticmethod
-    def _fetch_image(context, target, image_id, user_id, project_id,
-                     size=None):
-        """Grab image and optionally attempt to resize it"""
+    def _fetch_image(context, target, image_id, user_id, project_id):
+        """Grab image to raw format"""
         images.fetch_to_raw(context, image_id, target, user_id, project_id)
-        if size:
-            disk.extend(target, size)
 
     @staticmethod
     def _create_local(target, local_size, unit='G', fs_format=None):

nova/virt/libvirt/utils.py

@@ -103,7 +103,11 @@ def copy_image(src, dest):
     :param src: Source image
     :param dest: Destination path
     """
-    shutil.copyfile(src, dest)
+    # We shell out to cp because that will intelligently copy
+    # sparse files.  I.E. holes will not be written to DEST,
+    # rather recreated efficiently.  In addition, since
+    # coreutils 8.11, holes can be read efficiently too.
+    execute('cp', src, dest)
 
 
 def mkfs(fs, path):
@@ -254,9 +258,6 @@ def get_fs_info(path):
             'used': used}
 
 
-def fetch_image(context, target, image_id, user_id, project_id,
-                 size=None):
-    """Grab image and optionally attempt to resize it"""
+def fetch_image(context, target, image_id, user_id, project_id):
+    """Grab image"""
     images.fetch(context, image_id, target, user_id, project_id)
-    if size:
-        disk.extend(target, size)