openstack
/
nova
Code Issues Proposed changes

Move is_volume_backed_instance to compute.utils 

This function is a method of ComputeAPI, so its callers must have an
instance of the class. This make the usage difficult in modules which
do not need to call ComputeAPI by other reason.

Besides this function does not use any of the class members. So there is
no reason for it to be the class method.

This patch moves this function to compute.utils because it does not use
anything of the class members.

This patch also adds support for the _get_root_bdm and
is_volume_backed_instance to read from a dictionary instead of an
Instance object. Because of this we can call is_volume_backed_instance
from build_request_spec and fix bug #1469179.

Change-Id: I6d446088faf500ed39a4504794d09d0f86e2bbc3
Co-Authored-By: Feodor Tersin <ftersin@hotmail.com>
Co-Authored-By: Ankit Agrawal <ankit11.agrawal@nttdata.com>
Related-Bug: #1469179
Related-Bug: #1334974
Related-Bug: #1358566
This commit is contained in:
Tobias Urdin 2016-01-20 21:55:33 +01:00 committed by Ankit Agrawal
parent fe8a119e8d
commit d6210a4d0c
10 changed files with 170 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/openstack/compute/legacy_v2/servers.py
View File

@ -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',

3
nova/api/openstack/compute/servers.py
View File

@ -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(

26
nova/compute/api.py
View File

@ -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])

2
nova/compute/manager.py
View File

@ -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)

24
nova/compute/utils.py
View File

@ -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]

8
nova/tests/unit/api/openstack/compute/test_serversV21.py
View File

@ -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,

105
nova/tests/unit/compute/test_compute.py
View File

@ -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.

18
nova/tests/unit/compute/test_compute_api.py
View File

@ -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'),

11
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -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

118
nova/tests/unit/compute/test_compute_utils.py
View File

@ -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)