diff --git a/nova/api/openstack/compute/legacy_v2/servers.py b/nova/api/openstack/compute/legacy_v2/servers.py index 0ffcf43a676c..a81afa665b17 100644 --- a/nova/api/openstack/compute/legacy_v2/servers.py +++ b/nova/api/openstack/compute/legacy_v2/servers.py @@ -34,6 +34,7 @@ from nova.api.openstack import wsgi from nova import block_device from nova import compute from nova.compute import flavors +from nova.compute import utils as compute_utils import nova.conf from nova import exception from nova.i18n import _ @@ -1068,7 +1069,7 @@ class Controller(wsgi.Controller): context, instance.uuid) try: - if self.compute_api.is_volume_backed_instance(context, instance, + if compute_utils.is_volume_backed_instance(context, instance, bdms): policy.enforce(context, 'compute:snapshot_volume_backed', diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 6a4f9607090f..aef99fc238b3 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -36,6 +36,7 @@ from nova.api.openstack import wsgi from nova.api import validation from nova import compute from nova.compute import flavors +from nova.compute import utils as compute_utils import nova.conf from nova import exception from nova.i18n import _ @@ -1109,7 +1110,7 @@ class ServersController(wsgi.Controller): context, instance.uuid) try: - if self.compute_api.is_volume_backed_instance(context, instance, + if compute_utils.is_volume_backed_instance(context, instance, bdms): authorize(context, action="create_image:allow_volume_backed") image = self.compute_api.snapshot_volume_backed( diff --git a/nova/compute/api.py b/nova/compute/api.py index c981ac5c6415..12bfcfab6e86 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2227,7 +2227,7 @@ class API(base.Base): """ props_copy = dict(extra_properties, backup_type=backup_type) - if self.is_volume_backed_instance(context, instance): + if compute_utils.is_volume_backed_instance(context, instance): LOG.info(_LI("It's not supported to backup volume backed " "instance."), context=context, instance=instance) raise exception.InvalidRequest() @@ -2474,7 +2474,7 @@ class API(base.Base): self._check_auto_disk_config(image=image, **kwargs) flavor = instance.get_flavor() - root_bdm = self._get_root_bdm(context, instance) + root_bdm = compute_utils.get_root_bdm(context, instance) self._checks_for_create_and_rebuild(context, image_id, image, flavor, metadata, files_to_inject, root_bdm) @@ -2654,7 +2654,8 @@ class API(base.Base): flavor_id, read_deleted="no") if (new_instance_type.get('root_gb') == 0 and current_instance_type.get('root_gb') != 0 and - not self.is_volume_backed_instance(context, instance)): + not compute_utils.is_volume_backed_instance(context, + instance)): reason = _('Resize to zero disk flavor is not allowed.') raise exception.CannotResizeDisk(reason=reason) @@ -2765,7 +2766,7 @@ class API(base.Base): self._record_action_start(context, instance, instance_actions.SHELVE) - if not self.is_volume_backed_instance(context, instance): + if not compute_utils.is_volume_backed_instance(context, instance): name = '%s-shelved' % instance.display_name image_meta = self._create_image(context, instance, name, 'snapshot') @@ -2891,7 +2892,7 @@ class API(base.Base): if bdm.volume_id: vol = self.volume_api.get(context, bdm.volume_id) self.volume_api.check_attached(context, vol) - if self.is_volume_backed_instance(context, instance, bdms): + if compute_utils.is_volume_backed_instance(context, instance, bdms): reason = _("Cannot rescue a volume-backed instance") raise exception.InstanceNotRescuable(instance_id=instance.uuid, reason=reason) @@ -3373,21 +3374,6 @@ class API(base.Base): diff=diff) return _metadata - def _get_root_bdm(self, context, instance, bdms=None): - if bdms is None: - bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( - context, instance.uuid) - - return bdms.root_bdm() - - def is_volume_backed_instance(self, context, instance, bdms=None): - root_bdm = self._get_root_bdm(context, instance, bdms) - if root_bdm is not None: - return root_bdm.is_volume - # in case we hit a very old instance without root bdm, we _assume_ that - # instance is backed by a volume, if and only if image_ref is not set - return not instance.image_ref - @check_instance_lock @check_instance_cell @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED]) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index cfe2e2d3f595..d538d0ad8294 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4994,7 +4994,7 @@ class ComputeManager(manager.Manager): :param dest_check_data: result of check_can_live_migrate_destination :returns: a dict containing migration info """ - is_volume_backed = self.compute_api.is_volume_backed_instance(ctxt, + is_volume_backed = compute_utils.is_volume_backed_instance(ctxt, instance) got_migrate_data_object = isinstance(dest_check_data, migrate_data_obj.LiveMigrateData) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 3faf9c0c5e72..b899cd9dbc20 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -186,6 +186,30 @@ def get_next_device_name(instance, device_name_list, return prefix + req_letter +def get_root_bdm(context, instance, bdms=None): + if bdms is None: + if isinstance(instance, objects.Instance): + uuid = instance.uuid + else: + uuid = instance['uuid'] + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, uuid) + + return bdms.root_bdm() + + +def is_volume_backed_instance(context, instance, bdms=None): + root_bdm = get_root_bdm(context, instance, bdms) + if root_bdm is not None: + return root_bdm.is_volume + # in case we hit a very old instance without root bdm, we _assume_ that + # instance is backed by a volume, if and only if image_ref is not set + if isinstance(instance, objects.Instance): + return not instance.image_ref + + return not instance['image_ref'] + + def _get_unused_letter(used_letters): doubles = [first + second for second in string.ascii_lowercase for first in string.ascii_lowercase] diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index d852826503bf..16489c66a6b9 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -4379,8 +4379,8 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): rule, rule_name, self.controller._action_create_image, self.req, FAKE_UUID, body=body) - @mock.patch.object(compute_api.API, 'is_volume_backed_instance', - return_value=True) + @mock.patch('nova.compute.utils.is_volume_backed_instance', + return_value=True) @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(servers.ServersController, '_get_server') def test_create_vol_backed_img_snapshotting_policy_blocks_project(self, @@ -4404,8 +4404,8 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): rules, rule_name, self.controller._action_create_image, self.req, FAKE_UUID, body=body) - @mock.patch.object(compute_api.API, 'is_volume_backed_instance', - return_value=True) + @mock.patch('nova.compute.utils.is_volume_backed_instance', + return_value=True) @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(servers.ServersController, '_get_server') def test_create_vol_backed_img_snapshotting_policy_blocks_role(self, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 3474dac81196..68fc74dc6a42 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8974,111 +8974,6 @@ class ComputeAPITestCase(BaseTestCase): self.assertIsNone( self.compute_api._volume_size(inst_type, blank_bdm)) - def test_is_volume_backed_instance_no_bdm_no_image(self): - ctxt = self.context - - instance = self._create_fake_instance_obj({'image_ref': ''}) - self.assertTrue( - self.compute_api.is_volume_backed_instance(ctxt, instance, None)) - - def test_is_volume_backed_instance_empty_bdm_with_image(self): - ctxt = self.context - instance = self._create_fake_instance_obj({ - 'root_device_name': 'vda', - 'image_ref': FAKE_IMAGE_REF - }) - self.assertFalse( - self.compute_api.is_volume_backed_instance( - ctxt, instance, - block_device_obj.block_device_make_list(ctxt, []))) - - def test_is_volume_backed_instance_bdm_volume_no_image(self): - ctxt = self.context - instance = self._create_fake_instance_obj({ - 'root_device_name': 'vda', - 'image_ref': '' - }) - bdms = block_device_obj.block_device_make_list(ctxt, - [fake_block_device.FakeDbBlockDeviceDict( - {'source_type': 'volume', - 'device_name': '/dev/vda', - 'volume_id': uuids.volume_id, - 'instance_uuid': - 'f8000000-0000-0000-0000-000000000000', - 'boot_index': 0, - 'destination_type': 'volume'})]) - self.assertTrue( - self.compute_api.is_volume_backed_instance(ctxt, instance, bdms)) - - def test_is_volume_backed_instance_bdm_local_no_image(self): - # if the root device is local the instance is not volume backed, even - # if no image_ref is set. - ctxt = self.context - instance = self._create_fake_instance_obj({ - 'root_device_name': 'vda', - 'image_ref': '' - }) - bdms = block_device_obj.block_device_make_list(ctxt, - [fake_block_device.FakeDbBlockDeviceDict( - {'source_type': 'volume', - 'device_name': '/dev/vda', - 'volume_id': uuids.volume_id, - 'destination_type': 'local', - 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', - 'boot_index': 0, - 'snapshot_id': None}), - fake_block_device.FakeDbBlockDeviceDict( - {'source_type': 'volume', - 'device_name': '/dev/vdb', - 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', - 'boot_index': 1, - 'destination_type': 'volume', - 'volume_id': 'c2ec2156-d75e-11e2-985b-5254009297d6', - 'snapshot_id': None})]) - self.assertFalse( - self.compute_api.is_volume_backed_instance(ctxt, instance, bdms)) - - def test_is_volume_backed_instance_bdm_volume_with_image(self): - ctxt = self.context - instance = self._create_fake_instance_obj({ - 'root_device_name': 'vda', - 'image_ref': FAKE_IMAGE_REF - }) - bdms = block_device_obj.block_device_make_list(ctxt, - [fake_block_device.FakeDbBlockDeviceDict( - {'source_type': 'volume', - 'device_name': '/dev/vda', - 'volume_id': uuids.volume_id, - 'boot_index': 0, - 'destination_type': 'volume'})]) - self.assertTrue( - self.compute_api.is_volume_backed_instance(ctxt, instance, bdms)) - - def test_is_volume_backed_instance_bdm_snapshot(self): - ctxt = self.context - instance = self._create_fake_instance_obj({'root_device_name': 'vda'}) - bdms = block_device_obj.block_device_make_list(ctxt, - [fake_block_device.FakeDbBlockDeviceDict( - {'source_type': 'volume', - 'device_name': '/dev/vda', - 'snapshot_id': 'de8836ac-d75e-11e2-8271-5254009297d6', - 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', - 'destination_type': 'volume', - 'boot_index': 0, - 'volume_id': None})]) - self.assertTrue( - self.compute_api.is_volume_backed_instance(ctxt, instance, bdms)) - - @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') - def test_is_volume_backed_instance_empty_bdm_by_uuid(self, mock_bdms): - ctxt = self.context - instance = self._create_fake_instance_obj() - mock_bdms.return_value = \ - block_device_obj.block_device_make_list(ctxt, []) - self.assertFalse( - self.compute_api.is_volume_backed_instance(ctxt, instance, None)) - mock_bdms.assert_called_with(ctxt, instance.uuid) - def test_reservation_id_one_instance(self): """Verify building an instance has a reservation_id that matches return value from create. diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index d0c36e9cbe53..3551efe8f81d 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -620,7 +620,7 @@ class _ComputeAPIUnitTestMixIn(object): display_name='fake-name') instance = self._create_instance_obj(params=params) with test.nested( - mock.patch.object(self.compute_api, 'is_volume_backed_instance', + mock.patch.object(compute_utils, 'is_volume_backed_instance', return_value=boot_from_volume), mock.patch.object(self.compute_api, '_create_image', return_value=dict(id='fake-image-id')), @@ -1694,8 +1694,7 @@ class _ComputeAPIUnitTestMixIn(object): get_flavor_by_flavor_id.return_value = fake_flavor - with mock.patch.object(self.compute_api, - 'is_volume_backed_instance', + with mock.patch.object(compute_utils, 'is_volume_backed_instance', return_value=False): self.assertRaises(exception.CannotResizeDisk, self.compute_api.resize, self.context, @@ -1720,7 +1719,7 @@ class _ComputeAPIUnitTestMixIn(object): get_flavor_by_flavor_id.return_value = fake_flavor - @mock.patch.object(self.compute_api, 'is_volume_backed_instance', + @mock.patch.object(compute_utils, 'is_volume_backed_instance', return_value=True) @mock.patch.object(fake_inst, 'save') def do_test(mock_save, mock_volume): @@ -2162,10 +2161,10 @@ class _ComputeAPIUnitTestMixIn(object): 'backup_instance') if not is_snapshot: - self.mox.StubOutWithMock(self.compute_api, - 'is_volume_backed_instance') + self.mox.StubOutWithMock(compute_utils, + 'is_volume_backed_instance') - self.compute_api.is_volume_backed_instance(self.context, + compute_utils.is_volume_backed_instance(self.context, instance).AndReturn(False) utils.get_image_from_system_metadata( @@ -2286,8 +2285,7 @@ class _ComputeAPIUnitTestMixIn(object): def test_backup_volume_backed_instance(self): instance = self._create_instance_obj() - with mock.patch.object(self.compute_api, - 'is_volume_backed_instance', + with mock.patch.object(compute_utils, 'is_volume_backed_instance', return_value=True) as mock_is_volume_backed: self.assertRaises(exception.InvalidRequest, self.compute_api.backup, self.context, @@ -3359,7 +3357,7 @@ class _ComputeAPIUnitTestMixIn(object): with test.nested( mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid', return_value=bdms), - mock.patch.object(self.compute_api, 'is_volume_backed_instance', + mock.patch.object(compute_utils, 'is_volume_backed_instance', return_value=False), mock.patch.object(instance, 'save'), mock.patch.object(self.compute_api, '_record_action_start'), diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 07715116c426..94daaad8ed3e 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -1860,14 +1860,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): instance = objects.Instance._from_db_object( self.context, objects.Instance(), db_instance) - self.mox.StubOutWithMock(self.compute.compute_api, + self.mox.StubOutWithMock(compute_utils, 'is_volume_backed_instance') self.mox.StubOutWithMock(self.compute, '_get_instance_block_device_info') self.mox.StubOutWithMock(self.compute.driver, 'check_can_live_migrate_source') - self.compute.compute_api.is_volume_backed_instance( + compute_utils.is_volume_backed_instance( self.context, instance).AndReturn(is_volume_backed) self.compute._get_instance_block_device_info( self.context, instance, refresh_conn_info=True @@ -1896,8 +1896,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') - @mock.patch.object(self.compute.compute_api, - 'is_volume_backed_instance') + @mock.patch.object(compute_utils, 'is_volume_backed_instance') @mock.patch.object(self.compute, '_get_instance_block_device_info') def do_test(mock_block_info, mock_volume_backed, @@ -4564,7 +4563,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): @mock.patch.object(compute.driver, 'check_can_live_migrate_source') @mock.patch.object(compute, '_get_instance_block_device_info') - @mock.patch.object(compute.compute_api, 'is_volume_backed_instance') + @mock.patch.object(compute_utils, 'is_volume_backed_instance') def _test(mock_ivbi, mock_gibdi, mock_cclms): mock_cclms.return_value = data self.assertIsInstance( @@ -4583,7 +4582,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): @mock.patch.object(compute, 'network_api') @mock.patch.object(compute.driver, 'pre_live_migration') @mock.patch.object(compute, '_get_instance_block_device_info') - @mock.patch.object(compute.compute_api, 'is_volume_backed_instance') + @mock.patch.object(compute_utils, 'is_volume_backed_instance') def _test(mock_ivbi, mock_gibdi, mock_plm, mock_nwapi, mock_notify): migrate_data = migrate_data_obj.LiveMigrateData() mock_plm.return_value = migrate_data diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 752d4b77fa93..84398568c13f 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -52,6 +52,8 @@ from nova.tests import uuidsentinel as uuids CONF = nova.conf.CONF +FAKE_IMAGE_REF = uuids.image_ref + def create_instance(context, user_id='fake', project_id='fake', params=None): """Create a test instance.""" @@ -719,3 +721,119 @@ class ComputeUtilsQuotaDeltaTestCase(test.TestCase): compute_utils.reserve_quota_delta(self.context, deltas, inst) mock_reserve.assert_called_once_with(project_id=inst.project_id, user_id=inst.user_id, **deltas) + + +class IsVolumeBackedInstanceTestCase(test.TestCase): + def setUp(self): + super(IsVolumeBackedInstanceTestCase, self).setUp() + self.user_id = 'fake' + self.project_id = 'fake' + self.context = context.RequestContext(self.user_id, + self.project_id) + + def test_is_volume_backed_instance_no_bdm_no_image(self): + ctxt = self.context + + instance = create_instance(ctxt, params={'image_ref': ''}) + self.assertTrue( + compute_utils.is_volume_backed_instance(ctxt, instance, None)) + + def test_is_volume_backed_instance_empty_bdm_with_image(self): + ctxt = self.context + instance = create_instance(ctxt, params={ + 'root_device_name': 'vda', + 'image_ref': FAKE_IMAGE_REF + }) + self.assertFalse( + compute_utils.is_volume_backed_instance( + ctxt, instance, + block_device_obj.block_device_make_list(ctxt, []))) + + def test_is_volume_backed_instance_bdm_volume_no_image(self): + ctxt = self.context + instance = create_instance(ctxt, params={ + 'root_device_name': 'vda', + 'image_ref': '' + }) + bdms = block_device_obj.block_device_make_list(ctxt, + [fake_block_device.FakeDbBlockDeviceDict( + {'source_type': 'volume', + 'device_name': '/dev/vda', + 'volume_id': uuids.volume_id, + 'instance_uuid': + 'f8000000-0000-0000-0000-000000000000', + 'boot_index': 0, + 'destination_type': 'volume'})]) + self.assertTrue( + compute_utils.is_volume_backed_instance(ctxt, instance, bdms)) + + def test_is_volume_backed_instance_bdm_local_no_image(self): + # if the root device is local the instance is not volume backed, even + # if no image_ref is set. + ctxt = self.context + instance = create_instance(ctxt, params={ + 'root_device_name': 'vda', + 'image_ref': '' + }) + bdms = block_device_obj.block_device_make_list(ctxt, + [fake_block_device.FakeDbBlockDeviceDict( + {'source_type': 'volume', + 'device_name': '/dev/vda', + 'volume_id': uuids.volume_id, + 'destination_type': 'local', + 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', + 'boot_index': 0, + 'snapshot_id': None}), + fake_block_device.FakeDbBlockDeviceDict( + {'source_type': 'volume', + 'device_name': '/dev/vdb', + 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', + 'boot_index': 1, + 'destination_type': 'volume', + 'volume_id': 'c2ec2156-d75e-11e2-985b-5254009297d6', + 'snapshot_id': None})]) + self.assertFalse( + compute_utils.is_volume_backed_instance(ctxt, instance, bdms)) + + def test_is_volume_backed_instance_bdm_volume_with_image(self): + ctxt = self.context + instance = create_instance(ctxt, params={ + 'root_device_name': 'vda', + 'image_ref': FAKE_IMAGE_REF + }) + bdms = block_device_obj.block_device_make_list(ctxt, + [fake_block_device.FakeDbBlockDeviceDict( + {'source_type': 'volume', + 'device_name': '/dev/vda', + 'volume_id': uuids.volume_id, + 'boot_index': 0, + 'destination_type': 'volume'})]) + self.assertTrue( + compute_utils.is_volume_backed_instance(ctxt, instance, bdms)) + + def test_is_volume_backed_instance_bdm_snapshot(self): + ctxt = self.context + instance = create_instance(ctxt, params={ + 'root_device_name': 'vda' + }) + bdms = block_device_obj.block_device_make_list(ctxt, + [fake_block_device.FakeDbBlockDeviceDict( + {'source_type': 'volume', + 'device_name': '/dev/vda', + 'snapshot_id': 'de8836ac-d75e-11e2-8271-5254009297d6', + 'instance_uuid': 'f8000000-0000-0000-0000-000000000000', + 'destination_type': 'volume', + 'boot_index': 0, + 'volume_id': None})]) + self.assertTrue( + compute_utils.is_volume_backed_instance(ctxt, instance, bdms)) + + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') + def test_is_volume_backed_instance_empty_bdm_by_uuid(self, mock_bdms): + ctxt = self.context + instance = create_instance(ctxt) + mock_bdms.return_value = block_device_obj.block_device_make_list( + ctxt, []) + self.assertFalse( + compute_utils.is_volume_backed_instance(ctxt, instance, None)) + mock_bdms.assert_called_with(ctxt, instance.uuid)