openstack/nova
Code Issues Proposed changes

api: Block unsupported actions with vDPA

Browse Source 
There are a number of operations that are known not to work with vDPA
interfaces and another few that may work but haven't been tested. Start
blocking these. In all cases where an operation is blocked a HTTP 409
(Conflict) is returned. This will allow lifecycle operations to be
enabled as they are tested or bugs are addressed.

Change-Id: I7f3cbc57a374b2f271018a2f6ef33ef579798db8
Blueprint: libvirt-vdpa-support
This commit is contained in:
Sean Mooney 2021-03-12 19:28:46 +00:00 committed by Stephen Finucane
parent ab04eb2196
commit 45798adf5a
18 changed files with 168 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
nova/api/openstack/compute/attach_interfaces.py
View File

@ -178,9 +178,12 @@ class InterfaceAttachmentController(wsgi.Controller):
exception.InterfaceAttachPciClaimFailed,
exception.InterfaceAttachResourceAllocationFailed) as e:
raise exc.HTTPBadRequest(explanation=e.format_message())
except (exception.InstanceIsLocked,
exception.FixedIpAlreadyInUse,
exception.PortInUse) as e:
except (
exception.OperationNotSupportedForVDPAInterface,
exception.InstanceIsLocked,
exception.FixedIpAlreadyInUse,
exception.PortInUse,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except (exception.PortNotFound,
exception.NetworkNotFound) as e:
@ -214,7 +217,10 @@ class InterfaceAttachmentController(wsgi.Controller):
instance, port_id=port_id)
except exception.PortNotFound as e:
raise exc.HTTPNotFound(explanation=e.format_message())
except exception.InstanceIsLocked as e:
except (
exception.OperationNotSupportedForVDPAInterface,
exception.InstanceIsLocked,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except NotImplementedError:
common.raise_feature_not_supported()

5
nova/api/openstack/compute/evacuate.py
View File

@ -130,7 +130,10 @@ class EvacuateController(wsgi.Controller):
raise exc.HTTPBadRequest(explanation=e.format_message())
except exception.ForbiddenWithAccelerators as e:
raise exc.HTTPForbidden(explanation=e.format_message())
except exception.OperationNotSupportedForVTPM as e:
except (
exception.OperationNotSupportedForVTPM,
exception.OperationNotSupportedForVDPAInterface,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
if (not api_version_request.is_supported(req, min_version='2.14') and

2
nova/api/openstack/compute/migrate_server.py
View File

@ -66,6 +66,7 @@ class MigrateServerController(wsgi.Controller):
exception.InstanceIsLocked,
exception.InstanceNotReady,
exception.ServiceUnavailable,
exception.OperationNotSupportedForVDPAInterface,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceInvalidState as state_error:
@ -142,6 +143,7 @@ class MigrateServerController(wsgi.Controller):
except (
exception.OperationNotSupportedForSEV,
exception.OperationNotSupportedForVTPM,
exception.OperationNotSupportedForVDPAInterface,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceIsLocked as e:

1
nova/api/openstack/compute/rescue.py
View File

@ -66,6 +66,7 @@ class RescueController(wsgi.Controller):
except (
exception.InstanceIsLocked,
exception.OperationNotSupportedForVTPM,
exception.OperationNotSupportedForVDPAInterface,
exception.InvalidVolume,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())

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

@ -951,6 +951,7 @@ class ServersController(wsgi.Controller):
raise exc.HTTPForbidden(
explanation=error.format_message())
except (
exception.OperationNotSupportedForVDPAInterface,
exception.InstanceIsLocked,
exception.InstanceNotReady,
exception.MixedInstanceNotSupportByComputeService,
@ -1106,6 +1107,7 @@ class ServersController(wsgi.Controller):
except (
exception.InstanceIsLocked,
exception.OperationNotSupportedForVTPM,
exception.OperationNotSupportedForVDPAInterface,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceInvalidState as state_error:

1
nova/api/openstack/compute/shelve.py
View File

@ -52,6 +52,7 @@ class ShelveController(wsgi.Controller):
except (
exception.InstanceIsLocked,
exception.OperationNotSupportedForVTPM,
exception.OperationNotSupportedForVDPAInterface,
exception.UnexpectedTaskStateError,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())

7
nova/api/openstack/compute/suspend_server.py
View File

@ -38,8 +38,11 @@ class SuspendServerController(wsgi.Controller):
target={'user_id': server.user_id,
'project_id': server.project_id})
self.compute_api.suspend(context, server)
except (exception.OperationNotSupportedForSEV,
exception.InstanceIsLocked) as e:
except (
exception.OperationNotSupportedForSEV,
exception.OperationNotSupportedForVDPAInterface,
exception.InstanceIsLocked,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceInvalidState as state_error:
common.raise_http_conflict_for_instance_invalid_state(state_error,

79
nova/compute/api.py
View File

@ -265,6 +265,27 @@ def reject_vtpm_instances(operation):
return outer
def reject_vdpa_instances(operation):
"""Reject requests to decorated function if instance has vDPA interfaces.
Raise OperationNotSupportedForVDPAInterfaces if operations involves one or
more vDPA interfaces.
"""
def outer(f):
@functools.wraps(f)
def inner(self, context, instance, *args, **kw):
if any(
vif['vnic_type'] == network_model.VNIC_TYPE_VDPA
for vif in instance.get_network_info()
):
raise exception.OperationNotSupportedForVDPAInterface(
instance_uuid=instance.uuid, operation=operation)
return f(self, context, instance, *args, **kw)
return inner
return outer
def load_cells():
global CELLS
if not CELLS:
@ -3948,6 +3969,9 @@ class API(base.Base):
# TODO(stephenfin): This logic would be so much easier to grok if we
# finally split resize and cold migration into separate code paths
# FIXME(sean-k-mooney): Cold migrate and resize to different hosts
# probably works but they have not been tested so block them for now
@reject_vdpa_instances(instance_actions.RESIZE)
@block_accelerators()
@check_instance_lock
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
@ -3962,6 +3986,7 @@ class API(base.Base):
host_name is always None in the resize case.
host_name can be set in the cold migration case only.
"""
allow_cross_cell_resize = self._allow_cross_cell_resize(
context, instance)
@ -4165,6 +4190,9 @@ class API(base.Base):
allow_same_host = CONF.allow_resize_to_same_host
return allow_same_host
# FIXME(sean-k-mooney): Shelve works but unshelve does not due to bug
# #1851545, so block it for now
@reject_vdpa_instances(instance_actions.SHELVE)
@reject_vtpm_instances(instance_actions.SHELVE)
@block_accelerators(until_service=54)
@check_instance_lock
@ -4184,7 +4212,6 @@ class API(base.Base):
instance.system_metadata.update(
{'image_base_image_ref': instance.image_ref}
)
instance.save(expected_task_state=[None])
self._record_action_start(context, instance, instance_actions.SHELVE)
@ -4352,6 +4379,10 @@ class API(base.Base):
return self.compute_rpcapi.get_instance_diagnostics(context,
instance=instance)
# FIXME(sean-k-mooney): Suspend does not work because we do not unplug
# the vDPA devices before calling managed save as we do with SR-IOV
# devices
@reject_vdpa_instances(instance_actions.SUSPEND)
@block_accelerators()
@reject_sev_instances(instance_actions.SUSPEND)
@check_instance_lock
@ -5015,19 +5046,27 @@ class API(base.Base):
self._record_action_start(
context, instance, instance_actions.ATTACH_INTERFACE)
# NOTE(gibi): Checking if the requested port has resource request as
# such ports are only supported if the compute service version is >= 55
# TODO(gibi): Remove this check in X as there we can be sure that all
# computes are new enough
if port_id:
port = self.network_api.show_port(context, port_id)
if port['port'].get(constants.RESOURCE_REQUEST):
port = self.network_api.show_port(context, port_id)['port']
# NOTE(gibi): Checking if the requested port has resource request
# as such ports are only supported if the compute service version
# is >= 55.
# TODO(gibi): Remove this check in X as there we can be sure
# that all computes are new enough.
if port.get(constants.RESOURCE_REQUEST):
svc = objects.Service.get_by_host_and_binary(
context, instance.host, 'nova-compute')
if svc.version < 55:
raise exception.AttachInterfaceWithQoSPolicyNotSupported(
instance_uuid=instance.uuid)
if port.get('binding:vnic_type', "normal") == "vdpa":
# FIXME(sean-k-mooney): Attach works but detach results in a
# QEMU error; blocked until this is resolved
raise exception.OperationNotSupportedForVDPAInterface(
instance_uuid=instance.uuid,
operation=instance_actions.ATTACH_INTERFACE)
return self.compute_rpcapi.attach_interface(context,
instance=instance, network_id=network_id, port_id=port_id,
requested_ip=requested_ip, tag=tag)
@ -5038,6 +5077,29 @@ class API(base.Base):
task_state=[None])
def detach_interface(self, context, instance, port_id):
"""Detach an network adapter from an instance."""
# FIXME(sean-k-mooney): Detach currently results in a failure to remove
# the interface from the live libvirt domain, so while the networking
# is torn down on the host the vDPA device is still attached to the VM.
# This is likely a libvirt/qemu bug so block detach until that is
# resolved.
for vif in instance.get_network_info():
if vif['id'] == port_id:
if vif['vnic_type'] == 'vdpa':
raise exception.OperationNotSupportedForVDPAInterface(
instance_uuid=instance.uuid,
operation=instance_actions.DETACH_INTERFACE)
break
else:
# NOTE(sean-k-mooney) This should never happen but just in case the
# info cache does not have the port we are detaching we can fall
# back to neutron.
port = self.network_api.show_port(context, port_id)['port']
if port.get('binding:vnic_type', 'normal') == 'vdpa':
raise exception.OperationNotSupportedForVDPAInterface(
instance_uuid=instance.uuid,
operation=instance_actions.DETACH_INTERFACE)
self._record_action_start(
context, instance, instance_actions.DETACH_INTERFACE)
self.compute_rpcapi.detach_interface(context, instance=instance,
@ -5079,6 +5141,7 @@ class API(base.Base):
return _metadata
@reject_vdpa_instances(instance_actions.LIVE_MIGRATION)
@block_accelerators()
@reject_vtpm_instances(instance_actions.LIVE_MIGRATION)
@reject_sev_instances(instance_actions.LIVE_MIGRATION)
@ -5210,6 +5273,8 @@ class API(base.Base):
self.compute_rpcapi.live_migration_abort(context,
instance, migration.id)
# FIXME(sean-k-mooney): rebuild works but we have not tested evacuate yet
@reject_vdpa_instances(instance_actions.EVACUATE)
@reject_vtpm_instances(instance_actions.EVACUATE)
@block_accelerators(until_service=SUPPORT_ACCELERATOR_SERVICE_FOR_REBUILD)
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED,

8
nova/exception.py
View File

@ -537,6 +537,14 @@ class OperationNotSupportedForVTPM(NovaException):
code = 409
class OperationNotSupportedForVDPAInterface(NovaException):
msg_fmt = _(
"Operation '%(operation)s' not supported for instance with "
"vDPA ports ((instance_uuid)s)."
)
code = 409
class InvalidHypervisorType(Invalid):
msg_fmt = _("The supplied hypervisor type of is invalid.")

9
nova/tests/unit/api/openstack/compute/test_evacuate.py
View File

@ -119,6 +119,15 @@ class EvacuateTestV21(test.NoDBTestCase):
webob.exc.HTTPConflict,
{'host': 'foo', 'onSharedStorage': 'False', 'adminPass': 'bar'})
@mock.patch('nova.compute.api.API.evacuate')
def test_evacuate__with_vdpa_interface(self, mock_evacuate):
mock_evacuate.side_effect = \
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo')
self._check_evacuate_failure(
webob.exc.HTTPConflict,
{'host': 'foo', 'onSharedStorage': 'False', 'adminPass': 'bar'})
def test_evacuate_with_active_service(self):
def fake_evacuate(*args, **kwargs):
raise exception.ComputeServiceInUse("Service still in use")

30
nova/tests/unit/api/openstack/compute/test_migrate_server.py
View File

@ -283,6 +283,13 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
expected_exc=webob.exc.HTTPConflict,
check_response=False)
def test_migrate_live_sev_not_supported(self):
self._test_migrate_live_failed_with_exception(
exception.OperationNotSupportedForSEV(
instance_uuid=uuids.instance, operation='foo'),
expected_exc=webob.exc.HTTPConflict,
check_response=False)
def test_migrate_live_vtpm_not_supported(self):
self._test_migrate_live_failed_with_exception(
exception.OperationNotSupportedForVTPM(
@ -290,6 +297,13 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
expected_exc=webob.exc.HTTPConflict,
check_response=False)
def test_migrate_live_vdpa_interfaces_not_supported(self):
self._test_migrate_live_failed_with_exception(
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo'),
expected_exc=webob.exc.HTTPConflict,
check_response=False)
def test_migrate_live_pre_check_error(self):
self._test_migrate_live_failed_with_exception(
exception.MigrationPreCheckError(reason=''))
@ -594,22 +608,6 @@ class MigrateServerTestsV268(MigrateServerTestsV256):
method_translations=method_translations,
args_map=args_map)
@mock.patch('nova.virt.hardware.get_mem_encryption_constraint',
new=mock.Mock(return_value=True))
@mock.patch.object(
objects.instance.Instance, 'image_meta',
new=objects.ImageMeta.from_dict({}))
def test_live_migrate_sev_rejected(self):
instance = self._stub_instance_get()
body = {'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto'}}
ex = self.assertRaises(webob.exc.HTTPConflict,
self.controller._migrate_live,
self.req, fakes.FAKE_UUID, body=body)
self.assertIn("Operation 'live-migration' not supported for "
"SEV-enabled instance (%s)" % instance.uuid,
str(ex))
def test_live_migrate_with_forced_host(self):
body = {'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto',

2
nova/tests/unit/api/openstack/compute/test_rescue.py
View File

@ -69,6 +69,8 @@ class RescueTestV21(test.NoDBTestCase):
exception.InstanceIsLocked(instance_uuid=uuids.instance),
exception.OperationNotSupportedForVTPM(
instance_uuid=uuids.instance, operation='foo'),
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo'),
exception.InvalidVolume(reason='foo'),
)
@mock.patch.object(compute.api.API, 'rescue')

2
nova/tests/unit/api/openstack/compute/test_server_actions.py
View File

@ -386,6 +386,8 @@ class ServerActionsControllerTestV21(test.TestCase):
exception.InstanceIsLocked(instance_uuid=uuids.instance),
exception.OperationNotSupportedForVTPM(
instance_uuid=uuids.instance, operation='foo'),
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo'),
)
@mock.patch('nova.compute.api.API.rebuild')
def test_rebuild__http_conflict_error(self, exc, mock_rebuild):

2
nova/tests/unit/api/openstack/compute/test_shelve.py
View File

@ -42,6 +42,8 @@ class ShelveControllerTest(test.NoDBTestCase):
exception.InstanceIsLocked(instance_uuid=uuids.instance),
exception.OperationNotSupportedForVTPM(
instance_uuid=uuids.instance, operation='foo'),
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo'),
exception.UnexpectedTaskStateError(
instance_uuid=uuids.instance, expected=None,
actual=task_states.SHELVING),

28
nova/tests/unit/api/openstack/compute/test_suspend_server.py
View File

@ -12,17 +12,19 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
import mock
from oslo_utils.fixture import uuidsentinel as uuids
import webob
from nova.api.openstack.compute import suspend_server as \
suspend_server_v21
from nova import exception
from nova import objects
from nova.tests.unit.api.openstack.compute import admin_only_action_common
from nova.tests.unit.api.openstack import fakes
@ddt.ddt
class SuspendServerTestsV21(admin_only_action_common.CommonTests):
suspend_server = suspend_server_v21
controller_name = 'SuspendServerController'
@ -39,16 +41,20 @@ class SuspendServerTestsV21(admin_only_action_common.CommonTests):
def test_suspend_resume(self):
self._test_actions(['_suspend', '_resume'])
@mock.patch('nova.virt.hardware.get_mem_encryption_constraint',
new=mock.Mock(return_value=True))
@mock.patch.object(objects.instance.Instance, 'image_meta')
def test_suspend_sev_rejected(self, mock_image):
instance = self._stub_instance_get()
ex = self.assertRaises(webob.exc.HTTPConflict,
self.controller._suspend,
self.req, fakes.FAKE_UUID, body={})
self.assertIn("Operation 'suspend' not supported for SEV-enabled "
"instance (%s)" % instance.uuid, str(ex))
@ddt.data(
exception.OperationNotSupportedForVDPAInterface(
instance_uuid=uuids.instance, operation='foo'),
exception.OperationNotSupportedForSEV(
instance_uuid=uuids.instance, operation='foo'),
)
@mock.patch('nova.compute.api.API.suspend')
def test_suspend__http_conflict_error(self, exc, mock_suspend):
mock_suspend.side_effect = exc
self.assertRaises(
webob.exc.HTTPConflict,
self.controller._suspend,
self.req, uuids.instance, body={})
self.assertTrue(mock_suspend.called)
def test_suspend_resume_with_non_existed_instance(self):
self._test_actions_with_non_existed_instance(['_suspend', '_resume'])

13
nova/tests/unit/compute/test_api.py
View File

@ -167,6 +167,7 @@ class _ComputeAPIUnitTestMixIn(object):
instance.launched_at = now
instance.disable_terminate = False
instance.info_cache = objects.InstanceInfoCache()
instance.info_cache.network_info = model.NetworkInfo()
instance.flavor = flavor
instance.old_flavor = instance.new_flavor = None
instance.numa_topology = None
@ -7189,10 +7190,16 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
@mock.patch('nova.compute.api.API._record_action_start')
@mock.patch.object(compute_rpcapi.ComputeAPI, 'detach_interface')
def test_detach_interface(self, mock_detach, mock_record):
instance = self._create_instance_obj()
self.compute_api.detach_interface(self.context, instance, None)
instance = self._create_instance_obj(params={
'info_cache': objects.InstanceInfoCache(
network_info=model.NetworkInfo([
model.VIF(id=uuids.port, address='foo'),
]),
),
})
self.compute_api.detach_interface(self.context, instance, uuids.port)
mock_detach.assert_called_with(self.context, instance=instance,
port_id=None)
port_id=uuids.port)
mock_record.assert_called_once_with(
self.context, instance, instance_actions.DETACH_INTERFACE)

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

@ -294,6 +294,8 @@ class BaseTestCase(test.TestCase):
inst.os_type = 'Linux'
inst.system_metadata = (
params and params.get('system_metadata', {}) or {})
inst.info_cache = objects.InstanceInfoCache()
inst.info_cache.network_info = network_model.NetworkInfo()
inst.locked = False
inst.created_at = timeutils.utcnow()
inst.updated_at = timeutils.utcnow()

5
nova/tests/unit/policies/test_servers.py
View File

@ -21,6 +21,7 @@ from nova.api.openstack.compute import servers
from nova.compute import api as compute
from nova.compute import vm_states
from nova import exception
from nova.network import model
from nova.network import neutron
from nova import objects
from nova.objects import fields
@ -80,6 +81,10 @@ class ServersPolicyTest(base.BasePolicyTest):
self.controller, '_get_instance')).mock
self.mock_get_instance.return_value = self.instance
self.mock_get_network_info = self.useFixture(
fixtures.MockPatch('nova.objects.Instance.get_network_info')).mock
self.mock_get_network_info.return_value = model.NetworkInfo()
self.servers = [fakes.stub_instance_obj(
1, vm_state=vm_states.ACTIVE, uuid=uuids.fake,
project_id=self.project_id, user_id='user1'),