Explicitly fail if trying to attach SR-IOV port

Attaching SR-IOV ports to existing instances is not supported
since the compute service does not perform any kind of PCI
device allocation, so we should fail fast with a clear error
if attempted. Note that the compute RPC API "attach_interface"
method is an RPC call from nova-api to nova-compute so the error
raised here will result in a 400 response to the user.

Blueprint sriov-interface-attach-detach would need to be
implemented to support this use case, and could arguably involve
a microversion to indicate when the feature was made available.

A related neutron docs patch https://review.openstack.org/594325
is posted for mentioning the limitation with SR-IOV port attach
as well.

Change-Id: Ibbf2bd3cdd45bcd61eebff883c30ded525b2495d
Closes-Bug: #1708433

doc/source/admin/pci-passthrough.rst

@@ -17,8 +17,10 @@ assigned to only one guest and cannot be shared.
 
 .. note::
 
-   For information on attaching virtual SR-IOV devices to guests, refer to the
+   For information on creating servers with virtual SR-IOV devices, refer to
-   :neutron-doc:`Networking Guide <admin/config-sriov>`.
+   the :neutron-doc:`Networking Guide <admin/config-sriov>`. Attaching
+   SR-IOV ports to existing servers is not currently supported, see
+   `bug 1708433 <https://bugs.launchpad.net/nova/+bug/1708433>`_ for details.
 
 To enable PCI passthrough, follow the steps below:
 

nova/exception.py

@@ -885,6 +885,12 @@ class PortUpdateFailed(Invalid):
     msg_fmt = _("Port update failed for port %(port_id)s: %(reason)s")
 
 
+class AttachSRIOVPortNotSupported(Invalid):
+    msg_fmt = _('Attaching SR-IOV port %(port_id)s to server '
+                '%(instance_uuid)s is not supported. SR-IOV ports must be '
+                'specified during server creation.')
+
+
 class FixedIpExists(NovaException):
     msg_fmt = _("Fixed IP %(address)s already exists.")
 

nova/network/neutronv2/api.py

@@ -650,7 +650,7 @@ class API(base_api.NetworkAPI):
                               port_id)
 
     def _validate_requested_port_ids(self, context, instance, neutron,
-                                     requested_networks):
+                                     requested_networks, attach=False):
         """Processes and validates requested networks for allocation.
 
         Iterates over the list of NetworkRequest objects, validating the
@@ -665,6 +665,9 @@ class API(base_api.NetworkAPI):
         :type neutron: neutronclient.v2_0.client.Client
         :param requested_networks: List of user-requested networks and/or ports
         :type requested_networks: nova.objects.NetworkRequestList
+        :param attach: Boolean indicating if a port is being attached to an
+            existing running instance. Should be False during server create.
+        :type attach: bool
         :returns: tuple of:
             - ports: dict mapping of port id to port dict
             - ordered_networks: list of nova.objects.NetworkRequest objects
@@ -678,6 +681,8 @@ class API(base_api.NetworkAPI):
             attached to another instance.
         :raises nova.exception.PortNotUsableDNS: If a requested port has a
             value assigned to its dns_name attribute.
+        :raises nova.exception.AttachSRIOVPortNotSupported: If a requested port
+            is an SR-IOV port and ``attach=True``.
         """
         ports = {}
         ordered_networks = []
@@ -715,6 +720,16 @@ class API(base_api.NetworkAPI):
                     # Make sure the port is usable
                     _ensure_no_port_binding_failure(port)
 
+                    # Make sure the port can be attached.
+                    if attach:
+                        # SR-IOV port attach is not supported.
+                        vnic_type = port.get('binding:vnic_type',
+                                             network_model.VNIC_TYPE_NORMAL)
+                        if vnic_type in network_model.VNIC_TYPES_SRIOV:
+                            raise exception.AttachSRIOVPortNotSupported(
+                                port_id=port['id'],
+                                instance_uuid=instance.uuid)
+
                     # If requesting a specific port, automatically process
                     # the network for that port as if it were explicitly
                     # requested.
@@ -955,7 +970,8 @@ class API(base_api.NetworkAPI):
 
     def allocate_for_instance(self, context, instance, vpn,
                               requested_networks, macs=None,
-                              security_groups=None, bind_host_id=None):
+                              security_groups=None, bind_host_id=None,
+                              attach=False):
         """Allocate network resources for the instance.
 
         :param context: The request context.
@@ -973,6 +989,8 @@ class API(base_api.NetworkAPI):
         :param security_groups: None or security groups to allocate for
             instance.
         :param bind_host_id: the host ID to attach to the ports being created.
+        :param attach: Boolean indicating if a port is being attached to an
+            existing running instance. Should be False during server create.
         :returns: network info as from get_instance_nw_info()
         """
         LOG.debug('allocate_for_instance()', instance=instance)
@@ -993,7 +1011,7 @@ class API(base_api.NetworkAPI):
         #
         requested_ports_dict, ordered_networks = (
             self._validate_requested_port_ids(
-                context, instance, neutron, requested_networks))
+                context, instance, neutron, requested_networks, attach=attach))
 
         nets = self._validate_requested_network_ids(
             context, instance, neutron, requested_networks, ordered_networks)
@@ -1542,7 +1560,7 @@ class API(base_api.NetworkAPI):
                                             tag=tag)])
         return self.allocate_for_instance(context, instance, vpn=False,
                 requested_networks=requested_networks,
-                bind_host_id=bind_host_id)
+                bind_host_id=bind_host_id, attach=True)
 
     def deallocate_port_for_instance(self, context, instance, port_id):
         """Remove a specified port from the instance.

nova/tests/unit/network/test_neutronv2.py

@@ -5922,7 +5922,8 @@ class TestAllocateForInstance(test.NoDBTestCase):
         self.assertEqual(requested_networks[0], ordered_networks[0])
         self.assertEqual('net-2', ordered_networks[1].network_id)
 
-    def _assert_validate_requested_port_ids_raises(self, exception, extras):
+    def _assert_validate_requested_port_ids_raises(self, exception, extras,
+                                                   attach=False):
         api = neutronapi.API()
         mock_client = mock.Mock()
         requested_networks = objects.NetworkRequestList(objects=[
@@ -5936,7 +5937,8 @@ class TestAllocateForInstance(test.NoDBTestCase):
         mock_client.show_port.return_value = {"port": port}
 
         self.assertRaises(exception, api._validate_requested_port_ids,
-            self.context, self.instance, mock_client, requested_networks)
+            self.context, self.instance, mock_client, requested_networks,
+            attach=attach)
 
     def test_validate_requested_port_ids_raise_not_usable(self):
         self._assert_validate_requested_port_ids_raises(
@@ -5958,6 +5960,12 @@ class TestAllocateForInstance(test.NoDBTestCase):
             exception.PortBindingFailed,
             {"binding:vif_type": model.VIF_TYPE_BINDING_FAILED})
 
+    def test_validate_requested_port_ids_raise_sriov(self):
+        self._assert_validate_requested_port_ids_raises(
+            exception.AttachSRIOVPortNotSupported,
+            {"binding:vnic_type": model.VNIC_TYPE_DIRECT},
+            attach=True)
+
     def test_validate_requested_network_ids_success_auto_net(self):
         requested_networks = []
         ordered_networks = []