Add live_migratable flag to PCI device specification

The target goal of these series of patch is to enable VFIO devices
migration with kernel variant drivers.

Partially-Implements: blueprint migrate-vfio-devices-using-kernel-variant-drivers
Change-Id: I23af0d36448e9b659f6383d602d9dfa0e2798e60

nova/objects/pci_device.py

@@ -179,6 +179,8 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
                 #     - "mac_address": the MAC address of the PF
                 #     - "managed": "true"/"false" if the device is managed by
                 #       hypervisor
+                #     - "live_migratable": true/false if the device can be live
+                #       migratable
                 extra_info = self.extra_info
                 data = v if isinstance(v, str) else jsonutils.dumps(v)
                 extra_info.update({k: data})
@@ -188,10 +190,12 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
         # As with the previous case, we must explicitly assign to
         # self.extra_info so that obj_what_changed detects the modification
         # and triggers a save later.
-        if "managed" not in dev_dict and "managed" in self.extra_info:
+        spec_tags = ["managed", "live_migratable"]
-            extra_info = self.extra_info
+        for tag in spec_tags:
-            del extra_info["managed"]
+            if tag not in dev_dict and tag in self.extra_info:
-            self.extra_info = extra_info
+                extra_info = self.extra_info
+                del extra_info[tag]
+                self.extra_info = extra_info
 
     def __init__(self, *args, **kwargs):
         super(PciDevice, self).__init__(*args, **kwargs)

nova/pci/devspec.py

@@ -318,14 +318,9 @@ class PciDeviceSpec(PciAddressSpec):
         self._remote_managed = strutils.bool_from_string(
             self.tags.get(PCI_REMOTE_MANAGED_TAG))
 
-        if self.tags.get("managed", None) is not None:
+        self._normalize_device_spec_tag("managed")
-            try:
+        self._normalize_device_spec_tag("live_migratable")
-                self.tags["managed"] = (
+
-                    "true" if strutils.bool_from_string(
-                        self.tags.get("managed"), strict=True) else "false"
-                )
-            except ValueError as e:
-                raise exception.PciConfigInvalidSpec(reason=str(e))
         if self._remote_managed:
             if address_obj is None:
                 # Note that this will happen if a netdev was specified in the
@@ -409,7 +404,20 @@ class PciDeviceSpec(PciAddressSpec):
     def get_tags(self) -> ty.Dict[str, str]:
         return self.tags
 
+    def _normalize_device_spec_tag(self, tag):
+        if self.tags.get(tag, None) is not None:
+            try:
+                self.tags[tag] = (
+                    "true" if strutils.bool_from_string(
+                        self.tags.get(tag), strict=True) else "false")
+            except ValueError as e:
+                raise exception.PciConfigInvalidSpec(
+                    reason=f"Cannot parse tag '{tag}': " + str(e)
+                )
+
     def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):
-        managed = self.tags.get("managed")
+        spec_tags = ["managed", "live_migratable"]
-        if managed is not None:
+        for tag in spec_tags:
-            dev.update({'managed': managed})
+            tag_value = self.tags.get(tag)
+            if tag_value is not None:
+                dev.update({tag: tag_value})

nova/tests/unit/objects/test_pci_device.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import copy
+import ddt
 from unittest import mock
 
 from oslo_serialization import jsonutils
@@ -113,6 +114,7 @@ fake_db_dev_old = {
     }
 
 
+@ddt.ddt
 class _TestPciDeviceObject(object):
     def _create_fake_instance(self):
         self.inst = instance.Instance()
@@ -190,13 +192,15 @@ class _TestPciDeviceObject(object):
         self.assertEqual(self.pci_device.obj_what_changed(),
                          set(['vendor_id', 'product_id', 'parent_addr']))
 
-    def test_update_and_remove_extra_info_key(self):
+    @ddt.data({"tag": "managed"},
+              {"tag": "live_migratable"})
+    def test_update_and_remove_extra_info_key(self, data):
         self.dev_dict = copy.copy(dev_dict)
-        self.dev_dict['managed'] = "true"
+        self.dev_dict[data['tag']] = "true"
         self.pci_device = pci_device.PciDevice.create(None, self.dev_dict)
 
         self.pci_device.obj_reset_changes()
-        changes = {'managed': 'no'}
+        changes = {data['tag']: 'no'}
         self.pci_device.update_device(changes)
         self.assertEqual(
             self.pci_device.obj_what_changed(),
@@ -207,7 +211,7 @@ class _TestPciDeviceObject(object):
                 ]
             ),
         )
-        self.assertEqual(self.pci_device.extra_info, {"managed": "no"})
+        self.assertEqual(self.pci_device.extra_info, {data['tag']: "no"})
 
         self.pci_device.obj_reset_changes()
         changes = {}
@@ -221,7 +225,7 @@ class _TestPciDeviceObject(object):
                 ]
             ),
         )
-        self.assertNotIn("managed", self.pci_device.extra_info)
+        self.assertNotIn(data['tag'], self.pci_device.extra_info)
 
     def test_update_device_same_value(self):
         self.pci_device = pci_device.PciDevice.create(None, dev_dict)

nova/tests/unit/pci/test_manager.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import copy
+import ddt
 from unittest import mock
 
 from oslo_serialization import jsonutils
@@ -124,6 +125,7 @@ fake_pci_requests = [
      'spec': [{'vendor_id': 'v1'}]}]
 
 
+@ddt.ddt
 class PciDevTrackerTestCase(test.NoDBTestCase):
     def _create_fake_instance(self):
         self.inst = objects.Instance()
@@ -256,17 +258,54 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
         self.assertEqual(5, len(tracker.pci_devs))
 
     @mock.patch("nova.pci.utils.is_physical_function", return_value=False)
-    def test_update_devices_from_hypervisor_resources_with_managed(
+    @ddt.data({"tag": "managed"},
-        self, _mock_vf
+              {"tag": "live_migratable"})
+    def test_update_devices_from_hypervisor_resources_with_tag(
+        self, data, _mock_vf
     ):
+        spec = [
+            '{"product_id":"8086", "vendor_id":"10c9", '
+            '"address":"0000:00:02.2", "' + data["tag"] + '":"no"}',
+            '{"product_id":"8086", "vendor_id":"10c9", '
+            '"address":"0000:00:02.3", "' + data["tag"] + '":"yes"}',
+        ]
         self.flags(
             group="pci",
-            device_spec=[
+            device_spec=spec,
-                '{"product_id":"8086", "vendor_id":"10c9", '
+        )
-                '"address":"0000:00:02.2", "managed":"no"}',
+        fake_pci_devs = [copy.deepcopy(fake_pci_6), copy.deepcopy(fake_pci_7)]
-                '{"product_id":"8086", "vendor_id":"10c9", '
+        fake_pci_devs_json = jsonutils.dumps(fake_pci_devs)
-                '"address":"0000:00:02.3", "managed":"yes"}',
+        tracker = manager.PciDevTracker(
-            ],
+            self.fake_context, objects.ComputeNode(id=1, numa_topology=None)
+        )
+        tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)
+        self.assertEqual(5, len(tracker.pci_devs))
+
+        pci_addr_extra_info = {
+            dev.address: dev.extra_info for dev in tracker.pci_devs
+        }
+
+        self.assertEqual(
+            pci_addr_extra_info["0000:00:02.2"][data["tag"]], "false")
+        self.assertEqual(
+            pci_addr_extra_info["0000:00:02.3"][data["tag"]], "true"
+        )
+
+    @mock.patch("nova.pci.utils.is_physical_function", return_value=False)
+    def test_update_devices_from_hypervisor_resources_with_multiple_tags(
+        self, _mock_vf
+    ):
+        spec = [
+            '{"product_id":"8086", "vendor_id":"10c9", '
+            '"address":"0000:00:02.2", "managed":"no", '
+            '"live_migratable":"no"}',
+            '{"product_id":"8086", "vendor_id":"10c9", '
+            '"address":"0000:00:02.3", "managed":"no", '
+            '"live_migratable":"yes"}',
+        ]
+        self.flags(
+            group="pci",
+            device_spec=spec,
         )
         fake_pci_devs = [copy.deepcopy(fake_pci_6), copy.deepcopy(fake_pci_7)]
         fake_pci_devs_json = jsonutils.dumps(fake_pci_devs)
@@ -284,24 +323,34 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
             pci_addr_extra_info["0000:00:02.2"]["managed"], "false"
         )
         self.assertEqual(
-            pci_addr_extra_info["0000:00:02.3"]["managed"], "true"
+            pci_addr_extra_info["0000:00:02.2"]["live_migratable"], "false"
+        )
+        self.assertEqual(
+            pci_addr_extra_info["0000:00:02.3"]["managed"], "false"
+        )
+        self.assertEqual(
+            pci_addr_extra_info["0000:00:02.3"]["live_migratable"], "true"
         )
 
     @mock.patch("nova.pci.manager.LOG.debug")
     @mock.patch("nova.pci.utils.is_physical_function", return_value=False)
-    def test_update_devices_from_hypervisor_resources_with_managed_invalid(
+    @ddt.data({"tag": "managed"},
-        self, _mock_vf, mock_debug
+              {"tag": "live_migratable"})
+    def test_update_devices_from_hypervisor_resources_with_invalid_tag(
+        self, data, _mock_vf, mock_debug
     ):
+        spec = [
+                '{"product_id":"8086", "vendor_id":"10c9", '
+                '"address":"0000:00:02.2", "' + data["tag"] + '":"no"}',
+                '{"product_id":"8086", "vendor_id":"10c9", '
+                '"address":"0000:00:02.3", "' + data["tag"] + '":"yes"}',
+                '{"product_id":"8086", "vendor_id":"10c9", '
+                '"address":"0000:00:02.4", "' + data["tag"] + '":"invalid"}',
+            ]
+
         self.flags(
             group="pci",
-            device_spec=[
+            device_spec=spec,
-                '{"product_id":"8086", "vendor_id":"10c9", '
-                '"address":"0000:00:02.2", "managed":"no"}',
-                '{"product_id":"8086", "vendor_id":"10c9", '
-                '"address":"0000:00:02.3", "managed":"yes"}',
-                '{"product_id":"8086", "vendor_id":"10c9", '
-                '"address":"0000:00:02.4", "managed":"invalid"}',
-            ],
         )
 
         exc = self.assertRaises(
@@ -312,7 +361,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
         )
 
         self.assertEqual(
-            "Invalid [pci]device_spec config: Unrecognized value 'invalid', "
+            "Invalid [pci]device_spec config: Cannot parse tag "
+            f"'{data['tag']}': Unrecognized value 'invalid', "
             "acceptable values are: '0', '1', 'f', 'false', 'n', 'no', 'off', "
             "'on', 't', 'true', 'y', 'yes'",
             str(exc)

nova/tests/unit/pci/test_whitelist.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 from nova import exception
 from nova.pci import whitelist
 from nova import test
@@ -28,6 +29,7 @@ dev_dict = {
     }
 
 
+@ddt.ddt
 class WhitelistTestCase(test.NoDBTestCase):
     def test_whitelist(self):
         white_list = '{"product_id":"0001", "vendor_id":"8086"}'
@@ -83,57 +85,57 @@ class WhitelistTestCase(test.NoDBTestCase):
         parsed = whitelist.Whitelist([white_list])
         self.assertTrue(parsed.device_assignable(dev_dict))
 
-    def test_device_managed(self):
+    @ddt.data(
-        white_list = (
+        {"tag": "managed", "value": "yes", "expected": "true"},
-            '{"product_id":"0001", "vendor_id":"8086", "managed": "yes"}'
+        {"tag": "managed", "value": "true", "expected": "true"},
+        {"tag": "managed", "value": "1", "expected": "true"},
+        {"tag": "managed", "value": "no", "expected": "false"},
+        {"tag": "managed", "value": "false", "expected": "false"},
+        {"tag": "managed", "value": "0", "expected": "false"},
+        {"tag": "live_migratable", "value": "yes", "expected": "true"},
+        {"tag": "live_migratable", "value": "true", "expected": "true"},
+        {"tag": "live_migratable", "value": "1", "expected": "true"},
+        {"tag": "live_migratable", "value": "no", "expected": "false"},
+        {"tag": "live_migratable", "value": "false", "expected": "false"},
+        {"tag": "live_migratable", "value": "0", "expected": "false"},
+    )
+    def test_device_tags(self, data):
+        wl = (
+            '{"product_id":"0001", "vendor_id":"8086", "' +
+            data["tag"] +
+            '":"' +
+            data["value"] +
+            '"}'
         )
+        white_list = wl
         parsed = whitelist.Whitelist([white_list])
         self.assertEqual(1, len(parsed.specs))
-        self.assertTrue(parsed.specs[0].tags["managed"], "true")
+        self.assertEqual(parsed.specs[0].tags[data["tag"]], data["expected"])
 
-    def test_device_managed_true(self):
+    @ddt.data({"tag": "managed"},
-        white_list = (
+              {"tag": "live_migratable"})
-            '{"product_id":"0001", "vendor_id":"8086", "managed": "true"}'
+    def test_device_managed_not_set(self, data):
-        )
-        parsed = whitelist.Whitelist([white_list])
-        self.assertEqual(1, len(parsed.specs))
-        self.assertTrue(parsed.specs[0].tags["managed"], "true")
-
-    def test_device_managed_int(self):
-        white_list = (
-            '{"product_id":"0001", "vendor_id":"8086", "managed": 1}'
-        )
-        parsed = whitelist.Whitelist([white_list])
-        self.assertEqual(1, len(parsed.specs))
-        self.assertTrue(parsed.specs[0].tags["managed"], "true")
-
-    def test_device_not_managed(self):
-        white_list = (
-            '{"product_id":"0001", "vendor_id":"8086", "managed": "no"}'
-        )
-        parsed = whitelist.Whitelist([white_list])
-        self.assertEqual(1, len(parsed.specs))
-        self.assertTrue(parsed.specs[0].tags["managed"], "false")
-
-    def test_device_managed_not_set(self):
         white_list = (
             '{"product_id":"0001", "vendor_id":"8086"}'
         )
         parsed = whitelist.Whitelist([white_list])
         self.assertEqual(1, len(parsed.specs))
-        self.assertNotIn("managed", parsed.specs[0].tags)
+        self.assertNotIn(data["tag"], parsed.specs[0].tags)
 
-    def test_device_managed_invalid_value(self):
+    @ddt.data({"tag": "managed"},
-        white_list = (
+              {"tag": "live_migratable"})
-            '{"product_id":"0001", "vendor_id":"8086", "managed": "invalid"}'
+    def test_device_managed_invalid_value(self, data):
-        )
+        wl = '{"product_id":"0001", "vendor_id":"8086", "' + \
+            data["tag"] + '":"' + 'invalid"}'
+        white_list = (wl)
 
         exc = self.assertRaises(
             exception.PciConfigInvalidSpec, whitelist.Whitelist, [white_list]
         )
 
         self.assertEqual(
-            "Invalid [pci]device_spec config: Unrecognized value 'invalid', "
+            "Invalid [pci]device_spec config: Cannot parse tag "
+            f"'{data['tag']}': Unrecognized value 'invalid', "
             "acceptable values are: '0', '1', 'f', 'false', 'n', 'no', 'off', "
             "'on', 't', 'true', 'y', 'yes'",
             str(exc)