From cad49c3f2aac14b9934ee160a650d4a7b9809d66 Mon Sep 17 00:00:00 2001 From: Xavier Queralt Date: Wed, 30 Oct 2013 20:21:03 +0100 Subject: [PATCH] Clean BDM when snapshoting volume-backed instances When snapshoting a volume-backed instance, its block device mapping is copied entirely into the image properties of the resulting snapshot. This may lead to some errors when booting from that snapshot because the fields 'id', 'insance_uuid' and 'connection_info' must be generated for each instance instead of reusing the ones from the original instance. For example, sharing the connection_info will cause the new instance to try to connect to the original volume instead of creating a new one. This patch cleans all the database-specific plus the connection_info fields from the block device mapping when creating the snapshot. Change-Id: I4190e9519d641c61b00b5d86c3885f2ae4fa86f3 Partial-Bug: #1246327 (cherry picked from commit eddd8a14cd2393ce7bdb0ec1c7366177371192b2) --- nova/compute/api.py | 12 +++- nova/tests/compute/test_compute_api.py | 79 ++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 9f443961110..663689e5490 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1927,6 +1927,7 @@ def snapshot_volume_backed(self, context, instance, image_meta, name, properties['root_device_name'] = instance['root_device_name'] properties.update(extra_properties or {}) + # TODO(xqueralt): Use new style BDM in volume snapshots bdms = self.get_instance_bdms(context, instance) mapping = [] @@ -1934,6 +1935,11 @@ def snapshot_volume_backed(self, context, instance, image_meta, name, if bdm['no_device']: continue + # Clean the BDM of the database related fields to prevent + # duplicates in the future (e.g. the id was being preserved) + for field in block_device.BlockDeviceDict._db_only_fields: + bdm.pop(field, None) + volume_id = bdm.get('volume_id') if volume_id: # create snapshot based on volume_id @@ -1945,7 +1951,11 @@ def snapshot_volume_backed(self, context, instance, image_meta, name, snapshot = self.volume_api.create_snapshot_force( context, volume['id'], name, volume['display_description']) bdm['snapshot_id'] = snapshot['id'] - bdm['volume_id'] = None + + # Clean the extra volume related fields that will be generated + # when booting from the new snapshot. + bdm.pop('volume_id') + bdm.pop('connection_info') mapping.append(bdm) diff --git a/nova/tests/compute/test_compute_api.py b/nova/tests/compute/test_compute_api.py index 8a461547d49..ec9a84d495d 100644 --- a/nova/tests/compute/test_compute_api.py +++ b/nova/tests/compute/test_compute_api.py @@ -13,10 +13,12 @@ """Unit tests for compute API.""" +import copy import datetime import iso8601 import mox +from nova import block_device from nova.compute import api as compute_api from nova.compute import cells_api as compute_cells_api from nova.compute import flavors @@ -37,6 +39,7 @@ from nova import quota from nova import test from nova.tests.image import fake as fake_image +from nova.tests import matchers from nova.tests.objects import test_migration from nova.tests.objects import test_service @@ -1367,6 +1370,82 @@ def test_backup_with_base_image_ref(self): self._test_snapshot_and_backup(is_snapshot=False, with_base_ref=True) + def test_snapshot_volume_backed(self): + instance = self._create_instance_obj() + instance['root_device_name'] = 'vda' + + instance_bdms = [] + + image_meta = { + 'id': 'fake-image-id', + 'properties': {'mappings': []}, + 'status': 'fake-status', + 'location': 'far-away', + } + + expect_meta = { + 'name': 'test-snapshot', + 'properties': {'root_device_name': 'vda', 'mappings': 'DONTCARE'}, + 'size': 0, + } + + def fake_get_instance_bdms(context, instance): + return copy.deepcopy(instance_bdms) + + def fake_image_create(context, image_meta, data): + self.assertThat(image_meta, matchers.DictMatches(expect_meta)) + + def fake_volume_get(context, volume_id): + return {'id': volume_id, 'display_description': ''} + + def fake_volume_create_snapshot(context, volume_id, name, description): + return {'id': '%s-snapshot' % volume_id} + + self.stubs.Set(self.compute_api, 'get_instance_bdms', + fake_get_instance_bdms) + self.stubs.Set(self.compute_api.image_service, 'create', + fake_image_create) + self.stubs.Set(self.compute_api.volume_api, 'get', + fake_volume_get) + self.stubs.Set(self.compute_api.volume_api, 'create_snapshot_force', + fake_volume_create_snapshot) + + # No block devices defined + self.compute_api.snapshot_volume_backed( + self.context, instance, copy.deepcopy(image_meta), 'test-snapshot') + + bdm = {'no_device': False, 'volume_id': '1', + 'connection_info': 'inf', 'device_name': '/dev/vda'} + + for key in block_device.BlockDeviceDict._db_only_fields: + bdm[key] = 'MUST DELETE' + + instance_bdms.append(bdm) + + expect_meta['properties']['block_device_mapping'] = [] + + expect_meta['properties']['block_device_mapping'].append( + {'no_device': False, 'snapshot_id': '1-snapshot', + 'device_name': '/dev/vda'}) + + # All the db_only fields and the volume ones are removed + self.compute_api.snapshot_volume_backed( + self.context, instance, copy.deepcopy(image_meta), 'test-snapshot') + + image_mappings = [{'device': 'vda', 'virtual': 'ephemeral0'}, + {'device': 'vdb', 'virtual': 'swap'}, + {'device': 'vdc', 'virtual': 'ephemeral1'}] + + image_meta['properties']['mappings'] = image_mappings + + expect_meta['properties']['block_device_mapping'].extend([ + {'no_device': True, 'device_name': '/dev/vdb'}, + {'no_device': True, 'device_name': '/dev/vdc'}]) + + # Check that the mappgins from the image properties are included + self.compute_api.snapshot_volume_backed( + self.context, instance, copy.deepcopy(image_meta), 'test-snapshot') + def test_volume_snapshot_create(self): volume_id = '1' create_info = {'id': 'eyedee'}