XenAPI: Monitor the GC when coalescing

Add a new host plugin to monitor whether the garbage collector is running.
This is then used by the wait_for_vhd_coalesce function to know whether
waiting any more is likely to produce the expected results.

Increased the number of times we loop waiting for coalesce as the loop
now has knowledge that something is still happening.

DocImpact
Change to parameter default.

Change-Id: Idcc0738945bd1aee8cdb52c9cfabd798f85c31db
Closes-bug: 1258169

etc/nova/nova.conf.sample

@@ -3398,7 +3398,7 @@
 # Max number of times to poll for VHD to coalesce. Used only
 # if compute_driver=xenapi.XenAPIDriver (integer value)
 # Deprecated group/name - [DEFAULT]/xenapi_vhd_coalesce_max_attempts
-#vhd_coalesce_max_attempts=5
+#vhd_coalesce_max_attempts=20
 
 # Base path to the storage repository (string value)
 # Deprecated group/name - [DEFAULT]/xenapi_sr_base_path

nova/tests/virt/xenapi/test_vm_utils.py

@@ -1769,6 +1769,70 @@ class SnapshotAttachedHereTestCase(VMUtilsTestBase):
                 task_state="image_pending_upload")
         mock_safe_destroy_vdis.assert_called_once_with(session, ["snap_ref"])
 
+    def _test_wait_for_vhd_coalesce(self, mock_another_child_vhd,
+            mock_get_vhd_parent_uuid, mock_sleep, gc_completes=True):
+
+        vhd_chain = ['vdi_base_ref', 'vdi_coalescable_ref', 'vdi_leaf_ref']
+        instance = {"uuid": "uuid"}
+        sr_ref = 'sr_ref'
+        session = mock.Mock()
+
+        def fake_call_plugin_serialized(plugin, function, **kwargs):
+            fake_call_plugin_serialized.count += 1
+            if fake_call_plugin_serialized.count == 3:
+                vhd_chain.remove('vdi_coalescable_ref')
+            fake_call_plugin_serialized.running = (
+                fake_call_plugin_serialized.count < 3 or not gc_completes)
+            return str(fake_call_plugin_serialized.running)
+
+        def fake_get_vhd_parent_uuid(session, vdi_ref):
+            index = vhd_chain.index(vdi_ref)
+            if index > 0:
+                return vhd_chain[index - 1].replace('ref', 'uuid')
+            return None
+
+        def fake_call_xenapi(method, *args):
+            if method == 'VDI.get_by_uuid':
+                return args[0].replace('uuid', 'ref')
+
+        fake_call_plugin_serialized.count = 0
+        fake_call_plugin_serialized.running = True
+        session.call_plugin_serialized.side_effect = (
+            fake_call_plugin_serialized)
+        session.call_xenapi.side_effect = fake_call_xenapi
+        mock_get_vhd_parent_uuid.side_effect = fake_get_vhd_parent_uuid
+
+        mock_another_child_vhd.return_value = False
+
+        self.assertEqual(('vdi_base_uuid', None),
+             vm_utils._wait_for_vhd_coalesce(session, instance, sr_ref,
+                                             'vdi_leaf_ref', 'vdi_base_uuid'))
+        self.assertEqual(3, session.call_plugin_serialized.call_count)
+        session.call_plugin_serialized.has_calls(session, "vdi_ref")
+        self.assertEqual(2, mock_sleep.call_count)
+
+        self.assertEqual(gc_completes, not fake_call_plugin_serialized.running)
+
+    @mock.patch.object(greenthread, 'sleep')
+    @mock.patch.object(vm_utils, '_get_vhd_parent_uuid')
+    @mock.patch.object(vm_utils, '_another_child_vhd')
+    def test_wait_for_vhd_coalesce(self, mock_another_child_vhd,
+            mock_get_vhd_parent_uuid, mock_sleep):
+        self._test_wait_for_vhd_coalesce(mock_another_child_vhd,
+                                         mock_get_vhd_parent_uuid,
+                                         mock_sleep,
+                                         gc_completes=True)
+
+    @mock.patch.object(greenthread, 'sleep')
+    @mock.patch.object(vm_utils, '_get_vhd_parent_uuid')
+    @mock.patch.object(vm_utils, '_another_child_vhd')
+    def test_wait_for_vhd_coalesce_still_gc(self, mock_another_child_vhd,
+            mock_get_vhd_parent_uuid, mock_sleep):
+        self._test_wait_for_vhd_coalesce(mock_another_child_vhd,
+                                         mock_get_vhd_parent_uuid,
+                                         mock_sleep,
+                                         gc_completes=False)
+
 
 class ImportMigratedDisksTestCase(VMUtilsTestBase):
     @mock.patch.object(vm_utils, '_import_migrate_ephemeral_disks')

nova/virt/xenapi/client/session.py

@@ -59,7 +59,7 @@ class XenAPISession(object):
     # changed in development environments.
     # MAJOR VERSION: Incompatible changes with the plugins
     # MINOR VERSION: Compatible changes, new plguins, etc
-    PLUGIN_REQUIRED_VERSION = '1.0'
+    PLUGIN_REQUIRED_VERSION = '1.1'
 
     def __init__(self, url, user, pw):
         import XenAPI

nova/virt/xenapi/driver.py

@@ -88,7 +88,7 @@ xenapi_opts = [
                 help='Ensure compute service is running on host XenAPI '
                      'connects to.'),
     cfg.IntOpt('vhd_coalesce_max_attempts',
-               default=5,
+               default=20,
                deprecated_name='xenapi_vhd_coalesce_max_attempts',
                deprecated_group='DEFAULT',
                help='Max number of times to poll for VHD to coalesce. '

nova/virt/xenapi/fake.py

@@ -694,7 +694,10 @@ class SessionBase(object):
         return base64.b64encode(zlib.compress("dom_id: %s" % dom_id))
 
     def _plugin_nova_plugin_version_get_version(self, method, args):
-        return pickle.dumps("1.0")
+        return pickle.dumps("1.1")
+
+    def _plugin_xenhost_query_gc(self, method, args):
+        return pickle.dumps("False")
 
     def host_call_plugin(self, _1, _2, plugin, method, args):
         func = getattr(self, '_plugin_%s_%s' % (plugin, method), None)

nova/virt/xenapi/vm_utils.py

@@ -2046,6 +2046,22 @@ def _child_vhds(session, sr_ref, vdi_uuid):
     return children
 
 
+def _another_child_vhd(session, vdi_ref, sr_ref, original_parent_uuid):
+    # Search for any other vdi which parents to original parent and is not
+    # in the active vm/instance vdi chain.
+    vdi_rec = session.call_xenapi('VDI.get_record', vdi_ref)
+    vdi_uuid = vdi_rec['uuid']
+    parent_vdi_uuid = _get_vhd_parent_uuid(session, vdi_ref, vdi_rec)
+    for _ref, rec in _get_all_vdis_in_sr(session, sr_ref):
+        if ((rec['uuid'] != vdi_uuid) and
+            (rec['uuid'] != parent_vdi_uuid) and
+            (rec['sm_config'].get('vhd-parent') == original_parent_uuid)):
+            # Found another vhd which too parents to original parent.
+            return True
+    # Found no other vdi with the same parent.
+    return False
+
+
 def _wait_for_vhd_coalesce(session, instance, sr_ref, vdi_ref,
                            original_parent_uuid):
     """Spin until the parent VHD is coalesced into its parent VHD
@@ -2064,34 +2080,24 @@ def _wait_for_vhd_coalesce(session, instance, sr_ref, vdi_ref,
     if not original_parent_uuid:
         return
 
-    def _another_child_vhd():
-        # Search for any other vdi which parents to original parent and is not
-        # in the active vm/instance vdi chain.
-        vdi_rec = session.call_xenapi('VDI.get_record', vdi_ref)
-        vdi_uuid = vdi_rec['uuid']
-        parent_vdi_uuid = _get_vhd_parent_uuid(session, vdi_ref, vdi_rec)
-        for _ref, rec in _get_all_vdis_in_sr(session, sr_ref):
-            if ((rec['uuid'] != vdi_uuid) and
-               (rec['uuid'] != parent_vdi_uuid) and
-               (rec['sm_config'].get('vhd-parent') == original_parent_uuid)):
-                # Found another vhd which too parents to original parent.
-                return True
-        # Found no other vdi with the same parent.
-        return False
-
     # Check if original parent has any other child. If so, coalesce will
     # not take place.
-    if _another_child_vhd():
+    if _another_child_vhd(session, vdi_ref, sr_ref, original_parent_uuid):
         parent_uuid = _get_vhd_parent_uuid(session, vdi_ref)
         parent_ref = session.call_xenapi("VDI.get_by_uuid", parent_uuid)
         base_uuid = _get_vhd_parent_uuid(session, parent_ref)
         return parent_uuid, base_uuid
 
+    sr_uuid = session.call_xenapi("SR.get_uuid", sr_ref)
+
     max_attempts = CONF.xenserver.vhd_coalesce_max_attempts
     for i in xrange(max_attempts):
         # NOTE(sirp): This rescan is necessary to ensure the VM's `sm_config`
         # matches the underlying VHDs.
         _scan_sr(session, sr_ref)
+        gc_running = session.call_plugin_serialized('xenhost', 'query_gc',
+                                                    sr_uuid=sr_uuid,
+                                                    vdi_uuid=None)
         parent_uuid = _get_vhd_parent_uuid(session, vdi_ref)
         if parent_uuid and (parent_uuid != original_parent_uuid):
             LOG.debug(_("Parent %(parent_uuid)s doesn't match original parent"
@@ -2099,6 +2105,10 @@ def _wait_for_vhd_coalesce(session, instance, sr_ref, vdi_ref,
                       {'parent_uuid': parent_uuid,
                        'original_parent_uuid': original_parent_uuid},
                       instance=instance)
+            if not gc_running:
+                msg = _("VHD coalesce: Garbage collection not running"
+                        ", giving up...")
+                raise exception.NovaException(msg)
         else:
             parent_ref = session.call_xenapi("VDI.get_by_uuid", parent_uuid)
             base_uuid = _get_vhd_parent_uuid(session, parent_ref)

plugins/xenserver/xenapi/etc/xapi.d/plugins/nova_plugin_version

@@ -23,7 +23,8 @@ import utils
 # MINOR VERSION: Compatible changes, new plugins, etc
 
 # 1.0 - Initial version.
-PLUGIN_VERSION = "1.0"
+# 1.1 - New call to check GC status
+PLUGIN_VERSION = "1.1"
 
 def get_version(session):
     return PLUGIN_VERSION

plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost

@@ -28,6 +28,8 @@ except ImportError:
 import logging
 import re
 import time
+import sys
+import xmlrpclib
 
 import utils
 
@@ -395,8 +397,18 @@ def cleanup(dct):
 #            "external-auth-service-name", "")
     return out
 
+def query_gc(session, sr_uuid, vdi_uuid):
+    result = _run_command(["/opt/xensource/sm/cleanup.py",
+                           "-q", "-u", sr_uuid])
+    # Example output: "Currently running: True"
+    return result[19:].strip() == "True"
 
 if __name__ == "__main__":
+    # Support both serialized and non-serialized plugin approaches
+    _, methodname = xmlrpclib.loads(sys.argv[1])
+    if methodname in ['query_gc']:
+        utils.register_plugin_calls(query_gc)
+
     XenAPIPlugin.dispatch(
             {"host_data": host_data,
             "set_host_enabled": set_host_enabled,