From ce8e95c7a3fe9d94d6b03ee70c147e5e92aa3b5b Mon Sep 17 00:00:00 2001
From: John Garbutt <john.garbutt@rackspace.com>
Date: Fri, 13 Sep 2013 17:04:06 +0100
Subject: [PATCH] xenapi: stop using get_all_vdis_in_sr in spawn

Currently when trying to find a cached image, the very expensive call to
get_all_vdis_in_sr is used, where we could instead fetch the VDI
directly using a more targeted query.

Now we are fetching the VDI by name_label we must ensure to clear the
name_label on newly created VDIs to ensure they do not get picked up by
later calls to _find_cached_image().

Fixes bug 1221292

The above code that checks for _find_cached_images has race conditions
where its possible to end up with two VDIs returned. To stop this
happening the code to create the cached images is now synchronized on
the image being fetched.

Fixes bug 1226073

Change-Id: I534fb8f42b00b5d39dc17dd5fee297144b5f379a
---
 nova/virt/xenapi/vm_utils.py | 70 ++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index a4c8e12e581c..5575d7c9a051 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -791,8 +791,15 @@ def _find_cached_images(session, sr_ref):
 
 def _find_cached_image(session, image_id, sr_ref):
     """Returns the vdi-ref of the cached image."""
-    cached_images = _find_cached_images(session, sr_ref)
-    return cached_images.get(image_id)
+    name_label = _get_image_vdi_label(image_id)
+    recs = session.call_xenapi("VDI.get_all_records_where",
+                               'field "name__label"="%s"'
+                               % name_label)
+    number_found = len(recs)
+    if number_found > 0:
+        if number_found > 1:
+            LOG.warn(_("Multiple base images for image: %s") % image_id)
+        return recs.keys()[0]
 
 
 def resize_disk(session, instance, vdi_ref, instance_type):
@@ -1087,43 +1094,60 @@ def destroy_kernel_ramdisk(session, instance, kernel, ramdisk):
         session.call_plugin('kernel', 'remove_kernel_ramdisk', args)
 
 
+def _get_image_vdi_label(image_id):
+    return 'Glance Image %s' % image_id
+
+
 def _create_cached_image(context, session, instance, name_label,
                          image_id, image_type):
     sr_ref = safe_find_sr(session)
     sr_type = session.call_xenapi('SR.get_record', sr_ref)["type"]
-    vdis = {}
 
     if CONF.use_cow_images and sr_type != "ext":
         LOG.warning(_("Fast cloning is only supported on default local SR "
                       "of type ext. SR on this system was found to be of "
                       "type %s. Ignoring the cow flag."), sr_type)
 
-    cache_vdi_ref = _find_cached_image(session, image_id, sr_ref)
-    if cache_vdi_ref is None:
-        vdis = _fetch_image(context, session, instance, name_label,
-                            image_id, image_type)
+    @utils.synchronized('xenapi-image-cache' + image_id)
+    def _create_cached_image_impl(context, session, instance, name_label,
+                                  image_id, image_type, sr_ref):
+        cache_vdi_ref = _find_cached_image(session, image_id, sr_ref)
+        if cache_vdi_ref is None:
+            vdis = _fetch_image(context, session, instance, name_label,
+                                image_id, image_type)
 
-        cache_vdi_ref = session.call_xenapi(
-                'VDI.get_by_uuid', vdis['root']['uuid'])
+            cache_vdi_ref = session.call_xenapi(
+                    'VDI.get_by_uuid', vdis['root']['uuid'])
 
-        session.call_xenapi('VDI.set_name_label', cache_vdi_ref,
-                            'Glance Image %s' % image_id)
-        session.call_xenapi('VDI.set_name_description', cache_vdi_ref, 'root')
-        session.call_xenapi('VDI.add_to_other_config',
-                            cache_vdi_ref, 'image-id', str(image_id))
+            session.call_xenapi('VDI.set_name_label', cache_vdi_ref,
+                                _get_image_vdi_label(image_id))
+            session.call_xenapi('VDI.set_name_description', cache_vdi_ref,
+                                'root')
+            session.call_xenapi('VDI.add_to_other_config',
+                                cache_vdi_ref, 'image-id', str(image_id))
 
-    if CONF.use_cow_images and sr_type == 'ext':
-        new_vdi_ref = _clone_vdi(session, cache_vdi_ref)
-    elif sr_type == 'ext':
-        new_vdi_ref = _safe_copy_vdi(session, sr_ref, instance, cache_vdi_ref)
-    else:
-        new_vdi_ref = session.call_xenapi("VDI.copy", cache_vdi_ref, sr_ref)
+        if CONF.use_cow_images and sr_type == 'ext':
+            new_vdi_ref = _clone_vdi(session, cache_vdi_ref)
+        elif sr_type == 'ext':
+            new_vdi_ref = _safe_copy_vdi(session, sr_ref, instance,
+                                         cache_vdi_ref)
+        else:
+            new_vdi_ref = session.call_xenapi("VDI.copy", cache_vdi_ref,
+                                              sr_ref)
 
-    session.call_xenapi('VDI.remove_from_other_config',
-                        new_vdi_ref, 'image-id')
+        session.call_xenapi('VDI.set_name_label', cache_vdi_ref, '')
+        session.call_xenapi('VDI.set_name_description', cache_vdi_ref, '')
+        session.call_xenapi('VDI.remove_from_other_config',
+                            new_vdi_ref, 'image-id')
 
+        vdi_uuid = session.call_xenapi('VDI.get_uuid', new_vdi_ref)
+        return vdi_uuid
+
+    vdi_uuid = _create_cached_image_impl(context, session, instance,
+            name_label, image_id, image_type, sr_ref)
+
+    vdis = {}
     vdi_type = ImageType.get_role(image_type)
-    vdi_uuid = session.call_xenapi('VDI.get_uuid', new_vdi_ref)
     vdis[vdi_type] = dict(uuid=vdi_uuid, file=None)
     return vdis