Merge "api: Block unsupported actions with vDPA"

Author: Zuul

nova/api/openstack/compute/attach_interfaces.py

@@ -178,9 +178,12 @@ def create(self, req, server_id, body):
                 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 @@ def delete(self, req, server_id, id):
                 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 @@ def _evacuate(self, req, id, body):
             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 @@ def _migrate(self, req, id, body):
             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 @@ def _migrate_live(self, req, id, body):
         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 @@ def _rescue(self, req, id, body):
         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 @@ def _resize(self, req, instance_id, flavor_id, auto_disk_config=None):
             raise exc.HTTPForbidden(
                 explanation=error.format_message())
         except (
+            exception.OperationNotSupportedForVDPAInterface,
             exception.InstanceIsLocked,
             exception.InstanceNotReady,
             exception.MixedInstanceNotSupportByComputeService,
@@ -1106,6 +1107,7 @@ def _action_rebuild(self, req, id, body):
         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 @@ def _shelve(self, req, id, body):
         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 @@ def _suspend(self, req, id, body):
                         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,

nova/compute/api.py

@@ -265,6 +265,27 @@ def inner(self, context, instance, *args, **kw):
     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 @@ def _validate_host_for_cold_migrate(
 
     # 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 @@ def resize(self, context, instance, flavor_id=None, clean_shutdown=True,
         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 @@ def _allow_resize_to_same_host(self, cold_migrate, instance):
             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 @@ def shelve(self, context, instance, clean_shutdown=True):
         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 @@ def get_instance_diagnostics(self, context, instance):
         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 @@ def attach_interface(self, context, instance, network_id, port_id,
         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 @@ def attach_interface(self, context, instance, network_id, port_id,
                           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 @@ def update_instance_metadata(self, context, instance,
 
         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 @@ def live_migrate_abort(self, context, instance, migration_id,
         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 @@ def test_evacuate__with_vtpm(self, mock_evacuate):
             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")