Merge "api: Block unsupported actions with vDPA"

nova/api/openstack/compute/attach_interfaces.py

@@ -178,9 +178,12 @@ class InterfaceAttachmentController(wsgi.Controller):
                 exception.InterfaceAttachPciClaimFailed,
                 exception.InterfaceAttachResourceAllocationFailed) as e:
             raise exc.HTTPBadRequest(explanation=e.format_message())
-        except (exception.InstanceIsLocked,
+        except (
-                exception.FixedIpAlreadyInUse,
+            exception.OperationNotSupportedForVDPAInterface,
-                exception.PortInUse) as e:
+            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()

nova/api/openstack/compute/evacuate.py

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

nova/api/openstack/compute/migrate_server.py

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

nova/api/openstack/compute/rescue.py

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

nova/api/openstack/compute/servers.py

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

nova/api/openstack/compute/shelve.py

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

nova/api/openstack/compute/suspend_server.py

@@ -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,
+        except (
-                exception.InstanceIsLocked) as e:
+            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,

nova/compute/api.py

@@ -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)
+            port = self.network_api.show_port(context, port_id)['port']
-            if port['port'].get(constants.RESOURCE_REQUEST):
+            # 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,

nova/exception.py

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

nova/tests/unit/api/openstack/compute/test_evacuate.py

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

nova/tests/unit/api/openstack/compute/test_migrate_server.py

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

nova/tests/unit/api/openstack/compute/test_rescue.py

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

nova/tests/unit/api/openstack/compute/test_server_actions.py

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

nova/tests/unit/api/openstack/compute/test_shelve.py

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

nova/tests/unit/api/openstack/compute/test_suspend_server.py

@@ -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',
+    @ddt.data(
-                new=mock.Mock(return_value=True))
+        exception.OperationNotSupportedForVDPAInterface(
-    @mock.patch.object(objects.instance.Instance, 'image_meta')
+            instance_uuid=uuids.instance, operation='foo'),
-    def test_suspend_sev_rejected(self, mock_image):
+        exception.OperationNotSupportedForSEV(
-        instance = self._stub_instance_get()
+            instance_uuid=uuids.instance, operation='foo'),
-        ex = self.assertRaises(webob.exc.HTTPConflict,
+    )
-                               self.controller._suspend,
+    @mock.patch('nova.compute.api.API.suspend')
-                               self.req, fakes.FAKE_UUID, body={})
+    def test_suspend__http_conflict_error(self, exc, mock_suspend):
-        self.assertIn("Operation 'suspend' not supported for SEV-enabled "
+        mock_suspend.side_effect = exc
-                      "instance (%s)" % instance.uuid, str(ex))
+        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'])

nova/tests/unit/compute/test_api.py

@@ -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()
+        instance = self._create_instance_obj(params={
-        self.compute_api.detach_interface(self.context, instance, None)
+            '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)
 

nova/tests/unit/compute/test_compute.py

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

nova/tests/unit/policies/test_servers.py

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