diff --git a/nova/tests/virt/xenapi/test_vm_utils.py b/nova/tests/virt/xenapi/test_vm_utils.py index 812545b946..67d3953d44 100644 --- a/nova/tests/virt/xenapi/test_vm_utils.py +++ b/nova/tests/virt/xenapi/test_vm_utils.py @@ -1855,112 +1855,66 @@ class SnapshotAttachedHereTestCase(VMUtilsTestBase): mock_safe_destroy_vdis.assert_called_once_with(session, ["snap_ref"]) @mock.patch.object(greenthread, 'sleep') - @mock.patch.object(vm_utils, '_get_vhd_parent_uuid') - @mock.patch.object(vm_utils, '_count_parents_children') - @mock.patch.object(vm_utils, '_scan_sr') - def test_wait_for_vhd_coalesce(self, mock_scan_sr, - mock_count_parents_children, mock_get_vhd_parent_uuid, - mock_sleep): + def test_wait_for_vhd_coalesce_leaf_node(self, mock_sleep): + instance = {"uuid": "fake"} + vm_utils._wait_for_vhd_coalesce("session", instance, + "sr_ref", "vdi_ref", ["uuid"]) + self.assertFalse(mock_sleep.called) - cfg.CONF.import_opt('vhd_coalesce_max_attempts', - 'nova.virt.xenapi.driver', group="xenserver") - max_sr_scan_count = cfg.CONF.xenserver.vhd_coalesce_max_attempts - 1 + @mock.patch.object(vm_utils, '_count_children') + @mock.patch.object(greenthread, 'sleep') + def test_wait_for_vhd_coalesce_parent_snapshot(self, mock_sleep, + mock_count): + mock_count.return_value = 2 + instance = {"uuid": "fake"} - vhd_chain = ['vdi_base_ref', 'vdi_coalescable_ref', 'vdi_leaf_ref'] - instance = {"uuid": "uuid"} - sr_ref = 'sr_ref' - session = mock.Mock() + vm_utils._wait_for_vhd_coalesce("session", instance, + "sr_ref", "vdi_ref", ["uuid1", "uuid2"]) - def fake_scan_sr(session, sr_ref): - fake_scan_sr.count += 1 - if fake_scan_sr.count == max_sr_scan_count: - vhd_chain.remove('vdi_coalescable_ref') - - 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_scan_sr.count = 0 - fake_scan_sr.running = True - mock_scan_sr.side_effect = fake_scan_sr - session.call_xenapi.side_effect = fake_call_xenapi - mock_get_vhd_parent_uuid.side_effect = fake_get_vhd_parent_uuid - - mock_count_parents_children.return_value = 1 - - vm_utils._wait_for_vhd_coalesce(session, instance, sr_ref, - 'vdi_leaf_ref', ['vdi_base_uuid']) - self.assertEqual(max_sr_scan_count, mock_scan_sr.call_count) - session.call_plugin_serialized.has_calls(session, "vdi_ref") - # We'll sleep one fewer times than we scan the SR due to - # the scan at the start - self.assertEqual(max_sr_scan_count - 1, mock_sleep.call_count) + self.assertFalse(mock_sleep.called) + self.assertTrue(mock_count.called) @mock.patch.object(greenthread, 'sleep') @mock.patch.object(vm_utils, '_get_vhd_parent_uuid') - @mock.patch.object(vm_utils, '_count_parents_children') + @mock.patch.object(vm_utils, '_count_children') @mock.patch.object(vm_utils, '_scan_sr') - def test_wait_for_vhd_coalesce_1317792(self, mock_scan_sr, - mock_count_parents_children, - mock_get_vhd_parent_uuid, - mock_sleep): + def test_wait_for_vhd_coalesce_raises(self, mock_scan_sr, + mock_count, mock_get_vhd_parent_uuid, mock_sleep): + mock_count.return_value = 1 + instance = {"uuid": "fake"} - cfg.CONF.import_opt('vhd_coalesce_max_attempts', - 'nova.virt.xenapi.driver', group="xenserver") + self.assertRaises(exception.NovaException, + vm_utils._wait_for_vhd_coalesce, "session", instance, + "sr_ref", "vdi_ref", ["uuid1", "uuid2"]) - vhd_chain = ['vdi_base_ref', 'vdi_coalescable_ref1', - 'vdi_coalescable_ref2', 'vdi_leaf_ref'] - instance = {"uuid": "uuid"} - sr_ref = 'sr_ref' - session = mock.Mock() + self.assertTrue(mock_count.called) + self.assertEqual(20, mock_sleep.call_count) + self.assertEqual(20, mock_scan_sr.call_count) - def fake_scan_sr(session, sr_ref): - fake_scan_sr.count += 1 - if fake_scan_sr.count == 1: - vhd_chain.remove('vdi_coalescable_ref1') - elif fake_scan_sr.count == 2: - vhd_chain.remove('vdi_coalescable_ref2') + @mock.patch.object(greenthread, 'sleep') + @mock.patch.object(vm_utils, '_get_vhd_parent_uuid') + @mock.patch.object(vm_utils, '_count_children') + @mock.patch.object(vm_utils, '_scan_sr') + def test_wait_for_vhd_coalesce_success(self, mock_scan_sr, + mock_count, mock_get_vhd_parent_uuid, mock_sleep): + mock_count.return_value = 1 + instance = {"uuid": "fake"} + mock_get_vhd_parent_uuid.side_effect = ["bad", "uuid2"] - 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 + vm_utils._wait_for_vhd_coalesce("session", instance, + "sr_ref", "vdi_ref", ["uuid1", "uuid2"]) - def fake_call_xenapi(method, *args): - if method == 'VDI.get_by_uuid': - return args[0].replace('uuid', 'ref') - - fake_scan_sr.count = 0 - fake_scan_sr.running = True - mock_scan_sr.side_effect = fake_scan_sr - session.call_xenapi.side_effect = fake_call_xenapi - mock_get_vhd_parent_uuid.side_effect = fake_get_vhd_parent_uuid - - mock_count_parents_children.return_value = 1 - - vm_utils._wait_for_vhd_coalesce( - session, instance, sr_ref, - 'vdi_leaf_ref', ['vdi_base_uuid', 'vdi_coalescable_uuid1']) - session.call_plugin_serialized.has_calls(session, "vdi_ref") + self.assertEqual(1, mock_sleep.call_count) + self.assertEqual(2, mock_scan_sr.call_count) @mock.patch.object(vm_utils, '_get_all_vdis_in_sr') - @mock.patch.object(vm_utils, '_get_vhd_parent_uuid') - def test_count_parents_children(self, mock_get_parent_uuid, - mock_get_all_vdis_in_sr): - mock_get_parent_uuid.return_value = 'parent1' + def test_count_children(self, mock_get_all_vdis_in_sr): vdis = [('child1', {'sm_config': {'vhd-parent': 'parent1'}}), ('child2', {'sm_config': {'vhd-parent': 'parent2'}}), ('child3', {'sm_config': {'vhd-parent': 'parent1'}})] mock_get_all_vdis_in_sr.return_value = vdis - self.assertEqual(2, vm_utils._count_parents_children('session', - 'child3', 'sr')) + self.assertEqual(2, vm_utils._count_children('session', + 'parent1', 'sr')) class ImportMigratedDisksTestCase(VMUtilsTestBase): diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 870de22ea1..f0327ac3f0 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -719,7 +719,9 @@ def _snapshot_attached_here_impl(session, instance, vm_ref, label, userdevice, if post_snapshot_callback is not None: post_snapshot_callback(task_state=task_states.IMAGE_PENDING_UPLOAD) try: - # Ensure no VHDs will vanish while we migrate them + # When the VDI snapshot is taken a new parent is introduced. + # If we have taken a snapshot before, the new parent can be coalesced. + # We need to wait for this to happen before trying to copy the chain. _wait_for_vhd_coalesce(session, instance, sr_ref, vm_vdi_ref, vdi_uuid_chain) @@ -2044,10 +2046,9 @@ def _child_vhds(session, sr_ref, vdi_uuid, old_snapshots_only=False): return list(children) -def _count_parents_children(session, vdi_ref, sr_ref): +def _count_children(session, parent_vdi_uuid, sr_ref): # Search for any other vdi which has the same parent as us to work out # whether we have siblings and therefore if coalesce is possible - parent_vdi_uuid = _get_vhd_parent_uuid(session, vdi_ref) children = 0 for _ref, rec in _get_all_vdis_in_sr(session, sr_ref): if (rec['sm_config'].get('vhd-parent') == parent_vdi_uuid): @@ -2067,31 +2068,47 @@ def _wait_for_vhd_coalesce(session, instance, sr_ref, vdi_ref, in vdi_uuid_list may also be coalesced (except the base UUID - which is guaranteed to remain) """ - # NOTE(bobba): If we don't have any VDIs in the target list, then no - # coalescing can be guaranteed (e.g. file based SRs don't do leaf - # coalescing) - if len(vdi_uuid_list) == 0: + # If the base disk was a leaf node, there will be no coalescing + # after a VDI snapshot. + if len(vdi_uuid_list) == 1: + LOG.debug("Old chain is single VHD, coalesce not possible.", + instance=instance) return - # Check if we have siblings. If so, coalesce will not take place. - if _count_parents_children(session, vdi_ref, sr_ref) > 1: + # If the parent of the original disk has other children, + # there will be no coalesce because of the VDI snapshot. + # For example, the first snapshot for an instance that has been + # spawned from a cached image, will not coalesce, because of this rule. + parent_vdi_uuid = vdi_uuid_list[1] + if _count_children(session, parent_vdi_uuid, sr_ref) > 1: + LOG.debug("Parent has other children, coalesce is unlikely.", + instance=instance) return + # When the VDI snapshot is taken, a new parent is created. + # Assuming it is not one of the above cases, that new parent + # can be coalesced, so we need to wait for that to happen. max_attempts = CONF.xenserver.vhd_coalesce_max_attempts + # Remove the leaf node from list, to get possible good parents + # when the coalesce has completed. + # Its possible that other coalesce operation happen, so we need + # to consider the full chain, rather than just the most recent parent. + good_parent_uuids = vdi_uuid_list[1:] for i in xrange(max_attempts): # NOTE(sirp): This rescan is necessary to ensure the VM's `sm_config` # matches the underlying VHDs. + # This can also kick XenServer into performing a pending coalesce. _scan_sr(session, sr_ref) parent_uuid = _get_vhd_parent_uuid(session, vdi_ref) - if parent_uuid and (parent_uuid not in vdi_uuid_list): + if parent_uuid and (parent_uuid not in good_parent_uuids): LOG.debug("Parent %(parent_uuid)s not yet in parent list" - " %(vdi_uuid_list)s, waiting for coalesce...", + " %(good_parent_uuids)s, waiting for coalesce...", {'parent_uuid': parent_uuid, - 'vdi_uuid_list': vdi_uuid_list}, + 'good_parent_uuids': good_parent_uuids}, instance=instance) else: - parent_ref = session.call_xenapi("VDI.get_by_uuid", parent_uuid) - _get_vhd_parent_uuid(session, parent_ref) + LOG.debug("Coalesce detected, because parent is: %s" % parent_uuid, + instance=instance) return greenthread.sleep(CONF.xenserver.vhd_coalesce_poll_interval)