Merge "libvirt: Delegate OVS plug to os-vif" into stable/wallaby

Author: Zuul

nova/compute/manager.py

@@ -7997,11 +7997,12 @@ def check_can_live_migrate_destination(self, ctxt, instance,
                          'but source is either too old, or is set to an '
                          'older upgrade level.', instance=instance)
             if self.network_api.supports_port_binding_extension(ctxt):
-                # Create migrate_data vifs
-                migrate_data.vifs = \
-                    migrate_data_obj.\
-                    VIFMigrateData.create_skeleton_migrate_vifs(
-                        instance.get_network_info())
+                # Create migrate_data vifs if not provided by driver.
+                if 'vifs' not in migrate_data:
+                    migrate_data.vifs = (
+                        migrate_data_obj.
+                        VIFMigrateData.create_skeleton_migrate_vifs(
+                            instance.get_network_info()))
                 # Claim PCI devices for VIFs on destination (if needed)
                 port_id_to_pci = self._claim_pci_for_instance_vifs(
                     ctxt, instance)

nova/network/model.py

@@ -384,7 +384,8 @@ def __init__(self, id=None, address=None, network=None, type=None,
                  details=None, devname=None, ovs_interfaceid=None,
                  qbh_params=None, qbg_params=None, active=False,
                  vnic_type=VNIC_TYPE_NORMAL, profile=None,
-                 preserve_on_delete=False, **kwargs):
+                 preserve_on_delete=False, delegate_create=False,
+                 **kwargs):
         super(VIF, self).__init__()
 
         self['id'] = id
@@ -401,14 +402,15 @@ def __init__(self, id=None, address=None, network=None, type=None,
         self['vnic_type'] = vnic_type
         self['profile'] = profile
         self['preserve_on_delete'] = preserve_on_delete
+        self['delegate_create'] = delegate_create
 
         self._set_meta(kwargs)
 
     def __eq__(self, other):
         keys = ['id', 'address', 'network', 'vnic_type',
                 'type', 'profile', 'details', 'devname',
                 'ovs_interfaceid', 'qbh_params', 'qbg_params',
-                'active', 'preserve_on_delete']
+                'active', 'preserve_on_delete', 'delegate_create']
         return all(self[k] == other[k] for k in keys)
 
     def __ne__(self, other):

nova/network/neutron.py

@@ -3066,7 +3066,8 @@ def _build_vif_model(self, context, client, current_neutron_port,
             ovs_interfaceid=ovs_interfaceid,
             devname=devname,
             active=vif_active,
-            preserve_on_delete=preserve_on_delete
+            preserve_on_delete=preserve_on_delete,
+            delegate_create=True,
         )
 
     def _build_network_info_model(self, context, instance, networks=None,

nova/network/os_vif_util.py

@@ -21,6 +21,7 @@
 from os_vif import objects
 from oslo_config import cfg
 from oslo_log import log as logging
+import pbr.version
 
 from nova import exception
 from nova.i18n import _
@@ -347,6 +348,27 @@ def _nova_to_osvif_vif_ovs(vif):
             vif_name=vif_name,
             bridge_name=_get_hybrid_bridge_name(vif))
     else:
+        # NOTE(stephenfin): The 'create_port' attribute was added in os-vif
+        # 1.15.0 and isn't available with the lowest version supported here
+        if (
+            pbr.version.VersionInfo('os-vif').semantic_version() >=
+            pbr.version.SemanticVersion.from_pip_string('1.15.0')
+        ):
+            profile.create_port = vif.get('delegate_create', False)
+        elif vif.get('delegate_create', False):
+            # NOTE(stephenfin): This should never happen, since if the
+            # 'delegate_create' attribute is true then it was set by the
+            # destination compute node as part of the live migration operation:
+            # the same destination compute node that we are currently running
+            # on. We include it for sanity-preservation
+            LOG.warning(
+                'os-vif 1.15.0 or later is required to support '
+                'delegated port creation but you have %s; consider '
+                'updating this package to resolve bugs #1734320 and '
+                '#1815989',
+                pbr.version.VersionInfo('os-vif').version_string(),
+            )
+
         obj = _get_vif_instance(
             vif,
             objects.vif.VIFOpenVSwitch,

nova/objects/migrate_data.py

@@ -22,8 +22,8 @@
 from nova.objects import base as obj_base
 from nova.objects import fields
 
-
 LOG = log.getLogger(__name__)
+OS_VIF_DELEGATION = 'os_vif_delegation'
 
 
 @obj_base.NovaObjectRegistry.register
@@ -60,6 +60,8 @@ class VIFMigrateData(obj_base.NovaObject):
 
     @property
     def vif_details(self):
+        if 'vif_details_json' not in self:
+            return {}
         return jsonutils.loads(self.vif_details_json)
 
     @vif_details.setter
@@ -68,12 +70,27 @@ def vif_details(self, vif_details_dict):
 
     @property
     def profile(self):
+        if 'profile_json' not in self:
+            return {}
         return jsonutils.loads(self.profile_json)
 
     @profile.setter
     def profile(self, profile_dict):
         self.profile_json = jsonutils.dumps(profile_dict)
 
+    @property
+    def supports_os_vif_delegation(self):
+        return self.profile.get(OS_VIF_DELEGATION, False)
+
+    # TODO(stephenfin): add a proper delegation field instead of storing this
+    # info in the profile catch-all blob
+    @supports_os_vif_delegation.setter
+    def supports_os_vif_delegation(self, supported):
+        # we can't simply set the attribute using dict notation since the
+        # getter returns a copy of the data, not the data itself
+        self.profile = dict(
+            self.profile or {}, **{OS_VIF_DELEGATION: supported})
+
     def get_dest_vif(self):
         """Get a destination VIF representation of this object.
 
@@ -91,6 +108,7 @@ def get_dest_vif(self):
         vif['vnic_type'] = self.vnic_type
         vif['profile'] = self.profile
         vif['details'] = self.vif_details
+        vif['delegate_create'] = self.supports_os_vif_delegation
         return vif
 
     @classmethod

nova/tests/functional/libvirt/test_pci_sriov_servers.py

@@ -577,16 +577,15 @@ def test_get_server_diagnostics_server_with_VF(self):
         self.start_compute(pci_info=pci_info)
 
         # create the SR-IOV port
-        self.neutron.create_port({'port': self.neutron.network_4_port_1})
+        port = self.neutron.create_port(
+            {'port': self.neutron.network_4_port_1})
 
-        # create a server using the VF and multiple networks
-        extra_spec = {'pci_passthrough:alias': f'{self.VFS_ALIAS_NAME}:1'}
-        flavor_id = self._create_flavor(extra_spec=extra_spec)
+        flavor_id = self._create_flavor()
         server = self._create_server(
             flavor_id=flavor_id,
             networks=[
                 {'uuid': base.LibvirtNeutronFixture.network_1['id']},
-                {'port': base.LibvirtNeutronFixture.network_4_port_1['id']},
+                {'port': port['port']['id']},
             ],
         )
 
@@ -600,13 +599,16 @@ def test_get_server_diagnostics_server_with_VF(self):
             base.LibvirtNeutronFixture.network_1_port_2['mac_address'],
             diagnostics['nic_details'][0]['mac_address'],
         )
-        self.assertIsNotNone(diagnostics['nic_details'][0]['tx_packets'])
+
+        for key in ('rx_packets', 'tx_packets'):
+            self.assertIn(key, diagnostics['nic_details'][0])
 
         self.assertEqual(
             base.LibvirtNeutronFixture.network_4_port_1['mac_address'],
             diagnostics['nic_details'][1]['mac_address'],
         )
-        self.assertIsNone(diagnostics['nic_details'][1]['tx_packets'])
+        for key in ('rx_packets', 'tx_packets'):
+            self.assertIn(key, diagnostics['nic_details'][1])
 
     def test_create_server_after_change_in_nonsriov_pf_to_sriov_pf(self):
         # Starts a compute with PF not configured with SRIOV capabilities

nova/tests/unit/fake_network.py

@@ -120,7 +120,7 @@ def update_cache_fake(*args, **kwargs):
             qbg_params=None,
             active=False,
             vnic_type='normal',
-            profile=None,
+            profile={},
             preserve_on_delete=False,
             meta={'rxtx_cap': 30},
         )

nova/tests/unit/network/test_neutron.py

@@ -3271,6 +3271,7 @@ def test_build_network_info_model(self, mock_get_client,
                 requested_ports[index].get(
                     constants.BINDING_PROFILE) or {},
                 nw_info.get('profile'))
+            self.assertTrue(nw_info.get('delegate_create'))
             index += 1
 
         self.assertFalse(nw_infos[0]['active'])

nova/tests/unit/network/test_os_vif_util.py

@@ -458,7 +458,8 @@ def test_nova_to_osvif_vif_agilio_ovs_fallthrough(self):
                 subnets=[]),
             details={
                 model.VIF_DETAILS_PORT_FILTER: True,
-            }
+            },
+            delegate_create=False,
         )
 
         actual = os_vif_util.nova_to_osvif_vif(vif)
@@ -471,7 +472,8 @@ def test_nova_to_osvif_vif_agilio_ovs_fallthrough(self):
             plugin="ovs",
             port_profile=osv_objects.vif.VIFPortProfileOpenVSwitch(
                 interface_id="dc065497-3c8d-4f44-8fb4-e1d33c16a536",
-                datapath_type=None),
+                datapath_type=None,
+                create_port=False),
             preserve_on_delete=False,
             vif_name="nicdc065497-3c",
             network=osv_objects.network.Network(
@@ -588,6 +590,7 @@ def test_nova_to_osvif_vif_ovs_plain(self):
                 model.VIF_DETAILS_OVS_DATAPATH_TYPE:
                     model.VIF_DETAILS_OVS_DATAPATH_SYSTEM
             },
+            delegate_create=True,
         )
 
         actual = os_vif_util.nova_to_osvif_vif(vif)
@@ -600,7 +603,8 @@ def test_nova_to_osvif_vif_ovs_plain(self):
             plugin="ovs",
             port_profile=osv_objects.vif.VIFPortProfileOpenVSwitch(
                 interface_id="dc065497-3c8d-4f44-8fb4-e1d33c16a536",
-                datapath_type=model.VIF_DETAILS_OVS_DATAPATH_SYSTEM),
+                datapath_type=model.VIF_DETAILS_OVS_DATAPATH_SYSTEM,
+                create_port=True),
             preserve_on_delete=False,
             vif_name="nicdc065497-3c",
             network=osv_objects.network.Network(

nova/tests/unit/objects/test_migrate_data.py

@@ -316,3 +316,24 @@ def test_create_skeleton_migrate_vifs(self):
         self.assertEqual(len(vifs), len(mig_vifs))
         self.assertEqual([vif['id'] for vif in vifs],
                          [mig_vif.port_id for mig_vif in mig_vifs])
+
+    def test_supports_os_vif_delegation(self):
+        # first try setting on a object without 'profile' defined
+        migrate_vif = objects.VIFMigrateData(
+            port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
+            vif_type=network_model.VIF_TYPE_OVS, vif_details={},
+            host='fake-dest-host')
+        migrate_vif.supports_os_vif_delegation = True
+        self.assertTrue(migrate_vif.supports_os_vif_delegation)
+        self.assertEqual(migrate_vif.profile, {'os_vif_delegation': True})
+
+        # now do the same but with profile defined
+        migrate_vif = objects.VIFMigrateData(
+            port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
+            vif_type=network_model.VIF_TYPE_OVS, vif_details={},
+            host='fake-dest-host', profile={'interface_name': 'eth0'})
+        migrate_vif.supports_os_vif_delegation = True
+        self.assertTrue(migrate_vif.supports_os_vif_delegation)
+        self.assertEqual(
+            migrate_vif.profile,
+            {'os_vif_delegation': True, 'interface_name': 'eth0'})