Merge pull request #42 from stackhpc/upstream/xena-2023-04-10

Synchronise xena with upstream

Author: markgoddard

nova/db/main/api.py

@@ -4332,6 +4332,12 @@ def _get_fk_stmts(metadata, conn, table, column, records):
             fk_column = fk_table.c.id
 
         for fk in fk_table.foreign_keys:
+            if table != fk.column.table:
+                # if the foreign key doesn't actually point to the table we're
+                # archiving entries from then it's not relevant; trying to
+                # resolve this would result in a cartesian product
+                continue
+
             # We need to find the records in the referring (child) table that
             # correspond to the records in our (parent) table so we can archive
             # them.
@@ -4378,6 +4384,7 @@ def _get_fk_stmts(metadata, conn, table, column, records):
                 # deque.
                 fk_delete = fk_table.delete().where(fk_column.in_(fk_records))
                 deletes.appendleft(fk_delete)
+
         # Repeat for any possible nested child tables.
         i, d = _get_fk_stmts(metadata, conn, fk_table, fk_column, fk_records)
         inserts.extendleft(i)

nova/objects/cell_mapping.py

@@ -279,11 +279,15 @@ def _get_by_project_id_from_db(context, project_id):
         # SELECT DISTINCT cell_id FROM instance_mappings \
         #   WHERE project_id = $project_id;
         cell_ids = context.session.query(
-            api_db_models.InstanceMapping.cell_id).filter_by(
-            project_id=project_id).distinct().subquery()
+            api_db_models.InstanceMapping.cell_id
+        ).filter_by(
+            project_id=project_id
+        ).distinct()
         # SELECT cell_mappings WHERE cell_id IN ($cell_ids);
-        return context.session.query(api_db_models.CellMapping).filter(
-            api_db_models.CellMapping.id.in_(cell_ids)).all()
+        return context.session.query(
+            api_db_models.CellMapping).filter(
+            api_db_models.CellMapping.id.in_(cell_ids)
+        ).all()
 
     @classmethod
     def get_by_project_id(cls, context, project_id):

nova/tests/fixtures/nova.py

@@ -833,6 +833,16 @@ def setUp(self):
 
         self.addCleanup(warnings.resetwarnings)
 
+        # Enable general SQLAlchemy warnings also to ensure we're not doing
+        # silly stuff. It's possible that we'll need to filter things out here
+        # with future SQLAlchemy versions, but that's a good thing
+
+        warnings.filterwarnings(
+            'error',
+            module='nova',
+            category=sqla_exc.SAWarning,
+        )
+
 
 class ConfPatcher(fixtures.Fixture):
     """Fixture to patch and restore global CONF.

nova/tests/functional/regressions/test_bug_1951656.py

@@ -0,0 +1,73 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from oslo_utils import uuidutils
+
+
+from nova.tests.fixtures import libvirt as fakelibvirt
+from nova.tests.functional.libvirt import test_vgpu
+from nova.virt.libvirt import utils as libvirt_utils
+
+
+class VGPUTestsLibvirt7_7(test_vgpu.VGPUTestBase):
+
+    def _create_mdev(self, physical_device, mdev_type, uuid=None):
+        # We need to fake the newly created sysfs object by adding a new
+        # FakeMdevDevice in the existing persisted Connection object so
+        # when asking to get the existing mdevs, we would see it.
+        if not uuid:
+            uuid = uuidutils.generate_uuid()
+        mdev_name = libvirt_utils.mdev_uuid2name(uuid)
+        libvirt_parent = self.pci2libvirt_address(physical_device)
+
+        # Libvirt 7.7 now creates mdevs with a parent_addr suffix.
+        new_mdev_name = '_'.join([mdev_name, libvirt_parent])
+
+        # Here, we get the right compute thanks by the self.current_host that
+        # was modified just before
+        connection = self.computes[
+            self._current_host].driver._host.get_connection()
+        connection.mdev_info.devices.update(
+            {mdev_name: fakelibvirt.FakeMdevDevice(dev_name=new_mdev_name,
+                                                   type_id=mdev_type,
+                                                   parent=libvirt_parent)})
+        return uuid
+
+    def setUp(self):
+        super(VGPUTestsLibvirt7_7, self).setUp()
+        extra_spec = {"resources:VGPU": "1"}
+        self.flavor = self._create_flavor(extra_spec=extra_spec)
+
+        # Start compute1 supporting only nvidia-11
+        self.flags(
+            enabled_mdev_types=fakelibvirt.NVIDIA_11_VGPU_TYPE,
+            group='devices')
+
+        self.compute1 = self.start_compute_with_vgpu('host1')
+
+    def test_create_servers_with_vgpu(self):
+
+        # Create a single instance against a specific compute node.
+        self._create_server(
+            image_uuid='155d900f-4e14-4e4c-a73d-069cbf4541e6',
+            flavor_id=self.flavor, host=self.compute1.host,
+            networks='auto', expected_state='ACTIVE')
+
+        self.assert_mdev_usage(self.compute1, expected_amount=1)
+
+        self._create_server(
+            image_uuid='155d900f-4e14-4e4c-a73d-069cbf4541e6',
+            flavor_id=self.flavor, host=self.compute1.host,
+            networks='auto', expected_state='ACTIVE')
+
+        self.assert_mdev_usage(self.compute1, expected_amount=2)

nova/tests/unit/virt/libvirt/test_config.py

@@ -3135,6 +3135,32 @@ def test_config_mdev_device(self):
                               config.LibvirtConfigNodeDeviceMdevInformation)
         self.assertEqual("nvidia-11", obj.mdev_information.type)
         self.assertEqual(12, obj.mdev_information.iommu_group)
+        self.assertIsNone(obj.mdev_information.uuid)
+
+    def test_config_mdev_device_uuid(self):
+        xmlin = """
+        <device>
+          <name>mdev_b2107403_110c_45b0_af87_32cc91597b8a_0000_41_00_0</name>
+          <path>/sys/devices/pci0000:40/0000:40:03.1/0000:41:00.0/b2107403-110c-45b0-af87-32cc91597b8a</path>
+          <parent>pci_0000_41_00_0</parent>
+          <driver>
+            <name>vfio_mdev</name>
+          </driver>
+          <capability type='mdev'>
+            <type id='nvidia-442'/>
+            <uuid>b2107403-110c-45b0-af87-32cc91597b8a</uuid>
+            <iommuGroup number='57'/>
+          </capability>
+        </device>"""
+
+        obj = config.LibvirtConfigNodeDevice()
+        obj.parse_str(xmlin)
+        self.assertIsInstance(obj.mdev_information,
+                              config.LibvirtConfigNodeDeviceMdevInformation)
+        self.assertEqual("nvidia-442", obj.mdev_information.type)
+        self.assertEqual(57, obj.mdev_information.iommu_group)
+        self.assertEqual("b2107403-110c-45b0-af87-32cc91597b8a",
+                         obj.mdev_information.uuid)
 
     def test_config_vdpa_device(self):
         xmlin = """

nova/virt/libvirt/config.py

@@ -3289,6 +3289,7 @@ def __init__(self, **kwargs):
                                         root_name="capability", **kwargs)
         self.type = None
         self.iommu_group = None
+        self.uuid = None
 
     def parse_dom(self, xmldoc):
         super(LibvirtConfigNodeDeviceMdevInformation,
@@ -3298,6 +3299,8 @@ def parse_dom(self, xmldoc):
                 self.type = c.get('id')
             if c.tag == "iommuGroup":
                 self.iommu_group = int(c.get('number'))
+            if c.tag == "uuid":
+                self.uuid = c.text
 
 
 class LibvirtConfigGuestRng(LibvirtConfigGuestDevice):

nova/virt/libvirt/driver.py

@@ -7790,15 +7790,52 @@ def _get_mdev_capable_devices(self, types=None):
 
     def _get_mediated_device_information(self, devname):
         """Returns a dict of a mediated device."""
-        virtdev = self._host.device_lookup_by_name(devname)
+        # LP #1951656 - In Libvirt 7.7, the mdev name now includes the PCI
+        # address of the parent device (e.g. mdev_<uuid>_<pci_address>) due to
+        # the mdevctl allowing for multiple mediated devs having the same UUID
+        # defined (only one can be active at a time). Since the guest
+        # information doesn't have the parent ID, try to lookup which
+        # mediated device is available that matches the UUID. If multiple
+        # devices are found that match the UUID, then this is an error
+        # condition.
+        try:
+            virtdev = self._host.device_lookup_by_name(devname)
+        except libvirt.libvirtError as ex:
+            if ex.get_error_code() != libvirt.VIR_ERR_NO_NODE_DEVICE:
+                raise
+            mdevs = [dev for dev in self._host.list_mediated_devices()
+                     if dev.startswith(devname)]
+            # If no matching devices are found, simply raise the original
+            # exception indicating that no devices are found.
+            if not mdevs:
+                raise
+            elif len(mdevs) > 1:
+                msg = ("The mediated device name %(devname)s refers to a UUID "
+                       "that is present in multiple libvirt mediated devices. "
+                       "Matching libvirt mediated devices are %(devices)s. "
+                       "Mediated device UUIDs must be unique for Nova." %
+                       {'devname': devname,
+                        'devices': ', '.join(mdevs)})
+                raise exception.InvalidLibvirtMdevConfig(reason=msg)
+
+            LOG.debug('Found requested device %s as %s. Using that.',
+                      devname, mdevs[0])
+            virtdev = self._host.device_lookup_by_name(mdevs[0])
         xmlstr = virtdev.XMLDesc(0)
         cfgdev = vconfig.LibvirtConfigNodeDevice()
         cfgdev.parse_str(xmlstr)
+        # Starting with Libvirt 7.3, the uuid information is available in the
+        # node device information. If its there, use that. Otherwise,
+        # fall back to the previous behavior of parsing the uuid from the
+        # devname.
+        if cfgdev.mdev_information.uuid:
+            mdev_uuid = cfgdev.mdev_information.uuid
+        else:
+            mdev_uuid = libvirt_utils.mdev_name2uuid(cfgdev.name)
 
         device = {
             "dev_id": cfgdev.name,
-            # name is like mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4
-            "uuid": libvirt_utils.mdev_name2uuid(cfgdev.name),
+            "uuid": mdev_uuid,
             # the physical GPU PCI device
             "parent": cfgdev.parent,
             "type": cfgdev.mdev_information.type,

nova/virt/libvirt/host.py

@@ -1417,7 +1417,7 @@ def list_mdev_capable_devices(self, flags=0):
     def list_mediated_devices(self, flags=0):
         """Lookup mediated devices.
 
-        :returns: a list of virNodeDevice instance
+        :returns: a list of strings with the name of the instance
         """
         return self._list_devices("mdev", flags=flags)
 

nova/virt/libvirt/utils.py

@@ -577,17 +577,31 @@ def get_default_machine_type(arch: str) -> ty.Optional[str]:
 
 
 def mdev_name2uuid(mdev_name: str) -> str:
-    """Convert an mdev name (of the form mdev_<uuid_with_underscores>) to a
-    uuid (of the form 8-4-4-4-12).
+    """Convert an mdev name (of the form mdev_<uuid_with_underscores> or
+    mdev_<uuid_with_underscores>_<pciaddress>) to a uuid
+    (of the form 8-4-4-4-12).
+
+    :param mdev_name: the name of the mdev to parse the UUID from
+    :returns: string containing the uuid
     """
-    return str(uuid.UUID(mdev_name[5:].replace('_', '-')))
+    mdev_uuid = mdev_name[5:].replace('_', '-')
+    # Unconditionnally remove the PCI address from the name
+    mdev_uuid = mdev_uuid[:36]
+    return str(uuid.UUID(mdev_uuid))
+
 
+def mdev_uuid2name(mdev_uuid: str, parent: str = None) -> str:
+    """Convert an mdev uuid (of the form 8-4-4-4-12) and optionally its parent
+    device to a name (of the form mdev_<uuid_with_underscores>[_<pciid>]).
 
-def mdev_uuid2name(mdev_uuid: str) -> str:
-    """Convert an mdev uuid (of the form 8-4-4-4-12) to a name (of the form
-    mdev_<uuid_with_underscores>).
+    :param mdev_uuid: the uuid of the mediated device
+    :param parent: the parent device id for the mediated device
+    :returns: name of the mdev to reference in libvirt
     """
-    return "mdev_" + mdev_uuid.replace('-', '_')
+    name = "mdev_" + mdev_uuid.replace('-', '_')
+    if parent and parent.startswith('pci_'):
+        name = name + parent[4:]
+    return name
 
 
 def get_flags_by_flavor_specs(flavor: 'objects.Flavor') -> ty.Set[str]:

nova/weights.py

@@ -19,9 +19,14 @@
 
 import abc
 
+from oslo_log import log as logging
+
 from nova import loadables
 
 
+LOG = logging.getLogger(__name__)
+
+
 def normalize(weight_list, minval=None, maxval=None):
     """Normalize the values in a list between 0 and 1.0.
 
@@ -127,13 +132,40 @@ def get_weighed_objects(self, weighers, obj_list, weighing_properties):
         for weigher in weighers:
             weights = weigher.weigh_objects(weighed_objs, weighing_properties)
 
+            LOG.debug(
+                "%s: raw weights %s",
+                weigher.__class__.__name__,
+                {(obj.obj.host, obj.obj.nodename): weight
+                 for obj, weight in zip(weighed_objs, weights)}
+            )
+
             # Normalize the weights
-            weights = normalize(weights,
-                                minval=weigher.minval,
-                                maxval=weigher.maxval)
+            weights = list(
+                normalize(
+                    weights, minval=weigher.minval, maxval=weigher.maxval))
+
+            LOG.debug(
+                "%s: normalized weights %s",
+                weigher.__class__.__name__,
+                {(obj.obj.host, obj.obj.nodename): weight
+                 for obj, weight in zip(weighed_objs, weights)}
+            )
+
+            log_data = {}
 
             for i, weight in enumerate(weights):
                 obj = weighed_objs[i]
-                obj.weight += weigher.weight_multiplier(obj.obj) * weight
+                multiplier = weigher.weight_multiplier(obj.obj)
+                weigher_score = multiplier * weight
+                obj.weight += weigher_score
+
+                log_data[(obj.obj.host, obj.obj.nodename)] = (
+                    f"{multiplier} * {weight}")
+
+            LOG.debug(
+                "%s: score (multiplier * weight) %s",
+                weigher.__class__.__name__,
+                {name: log for name, log in log_data.items()}
+            )
 
         return sorted(weighed_objs, key=lambda x: x.weight, reverse=True)