allow upgrade of pre-victoria InstanceNUMACells

This change ensures that if we are upgrading a
InstanceNUMACell object created before victoria
<1.5 that we properly set pcpuset=set() when
loading the object form the db.

This is requried to support instances with a numa
topology that do not use cpu pinning.

Depends-On: https://review.opendev.org/c/openstack/python-openstackclient/+/929236
Closes-Bug: #2080556
Change-Id: Iea55aabe71c250d8c8e93c61421450b909a7fa3d
(cherry picked from commit 2a87032)
(cherry picked from commit 13c780a)
(cherry picked from commit e7d61fe)

Author: SeanMooney

nova/objects/instance_numa.py

@@ -14,6 +14,7 @@
 
 import itertools
 
+from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import versionutils
 
@@ -24,6 +25,8 @@
 from nova.objects import fields as obj_fields
 from nova.virt import hardware
 
+LOG = logging.getLogger(__name__)
+
 
 # TODO(berrange): Remove NovaObjectDictCompat
 @base.NovaObjectRegistry.register
@@ -61,6 +64,11 @@ def obj_make_compatible(self, primitive, target_version):
                     obj_fields.CPUAllocationPolicy.DEDICATED):
                 primitive['cpuset'] = primitive['pcpuset']
             primitive.pop('pcpuset', None)
+            LOG.warning(
+                f'Downgrading InstanceNUMACell to version {target_version} '
+                f'may cause the loss of pinned CPUs if mixing different '
+                f'verisons of nova on different hosts. This should not '
+                f'happen on any supported version after Victoria.')
 
         if target_version < (1, 4):
             primitive.pop('cpuset_reserved', None)
@@ -191,17 +199,15 @@ def _migrate_legacy_dedicated_instance_cpuset(cls, obj):
             if len(cell.cpuset) == 0:
                 continue
 
-            if cell.cpu_policy != obj_fields.CPUAllocationPolicy.DEDICATED:
-                # FIXME(sean-k-mooney): we should be setting the pcpuset
-                # to an empty set here
-                # if not 'pcpuset' in cell:
-                #     cell.pcpuset = set()
-                #     update_db = True
-                continue
+            if cell.cpu_policy == obj_fields.CPUAllocationPolicy.DEDICATED:
+                cell.pcpuset = cell.cpuset
+                cell.cpuset = set()
+                update_db = True
+            else:
+                if 'pcpuset' not in cell:
+                    cell.pcpuset = set()
+                    update_db = True
 
-            cell.pcpuset = cell.cpuset
-            cell.cpuset = set()
-            update_db = True
         return update_db
 
     # TODO(huaqiang): Remove after Yoga once we are sure these objects have

nova/tests/unit/objects/test_instance_numa.py

@@ -409,13 +409,7 @@ def test_obj_from_db_obj_no_pinning(self):
                 numa_topology.cells,
                 fake_topo_obj_w_cell_v1_4['cells']):
             self.assertEqual(topo_cell.cpuset, obj_cell.cpuset)
-            # 'pcpuset' should be an empty set however
-            # obj_from_db_obj() or more specifically
-            # _migrate_legacy_dedicated_instance_cpuset() does not set
-            # 'pcpuset' to an empty set when it is not in the json data.
-            # self.assertEqual(set(), obj_cell.pcpuset)
-            self.assertRaises(
-                NotImplementedError, getattr, obj_cell, 'pcpuset')
+            self.assertEqual(set(), obj_cell.pcpuset)
 
 
 class TestInstanceNUMATopology(

releasenotes/notes/instance-numa-object-upgrade-afa5bb96149ca2f5.yaml

@@ -0,0 +1,16 @@
+---
+upgrade:
+  - |
+    In the victoria release, the instance_numa_topology object
+    was extended to enabled mix cpus (pinned and unpinned cpus)
+    in the same instance. This change added a new field pcpuset
+    to the instance_numa_topology object. While the change included
+    object conversion code to handle the upgrade, it did not account
+    for instances that have a numa_topology but were not pinned.
+    i.e. a flavor with hw:mem_page_size or hw:numa_nodes set but
+    without hw:cpu_policy set to dedicated. As a result, instances
+    created between liberty and victoria releases with such a flavor
+    cannot be started  after upgrade to victoria. This has now
+    been fixed. instances created post victoria are not affected by
+    this issue. see: https://bugs.launchpad.net/nova/+bug/2080556
+    for more details.