Synchronise 2023.1 with upstream

nova/api/openstack/compute/aggregates.py

@@ -289,7 +289,7 @@ def images(self, req, id, body):
         for image_req in body.get('cache'):
             image_ids.append(image_req['id'])
 
-        if image_ids != list(set(image_ids)):
+        if sorted(image_ids) != sorted(list(set(image_ids))):
             raise exc.HTTPBadRequest(
                 explanation=_('Duplicate images in request'))
 

nova/network/neutron.py

@@ -1595,12 +1595,13 @@ def _get_vf_pci_device_profile(self, pci_dev):
         pf_mac = pci_dev.sriov_cap.get('pf_mac_address')
         vf_num = pci_dev.sriov_cap.get('vf_num')
         card_serial_number = pci_dev.card_serial_number
-        if all((pf_mac, vf_num, card_serial_number)):
-            vf_profile.update({
-                'card_serial_number': card_serial_number,
-                'pf_mac_address': pf_mac,
-                'vf_num': vf_num,
-            })
+
+        if card_serial_number is not None:
+            vf_profile['card_serial_number'] = card_serial_number
+        if pf_mac is not None:
+            vf_profile['pf_mac_address'] = pf_mac
+        if vf_num is not None:
+            vf_profile['vf_num'] = vf_num
 
         # Update port binding capabilities using PCI device's network
         # capabilities if they exist.

nova/tests/functional/libvirt/test_pci_sriov_servers.py

@@ -521,6 +521,8 @@ def fake_create(cls, xml, host):
                 'pci_vendor_info': '8086:1515',
                 'pci_slot': '0000:81:00.2',
                 'physical_network': 'physnet4',
+                'pf_mac_address': '52:54:00:1e:59:c6',
+                'vf_num': 1
             },
             port['binding:profile'],
         )
@@ -1017,6 +1019,8 @@ def test_live_migrate_server_with_neutron(self):
                 # matching one)
                 'pci_slot': '0000:81:00.4',
                 'physical_network': 'physnet4',
+                'pf_mac_address': '52:54:00:1e:59:c6',
+                'vf_num': 1
             },
             port['binding:profile'],
         )
@@ -1062,6 +1066,8 @@ def test_live_migrate_server_with_neutron(self):
                 'pci_vendor_info': '8086:1515',
                 'pci_slot': '0000:81:00.2',
                 'physical_network': 'physnet4',
+                'pf_mac_address': '52:54:00:1e:59:c6',
+                'vf_num': 1,
             },
             port['binding:profile'],
         )

nova/tests/unit/api/openstack/compute/test_aggregates.py

@@ -738,3 +738,28 @@ def test_images_with_invalid_id(self):
                                       version='2.81')
         self.assertRaises(exc.HTTPBadRequest, self.controller.images,
                           req, 'foo', body=body)
+
+    def test_images_with_duplicate_id(self):
+        body = {"cache": [{"id": "faae1bd3-c848-41d6-b4dd-97d5b8be8b7e"},
+                          {"id": "faae1bd3-c848-41d6-b4dd-97d5b8be8b7e"}]}
+        req = fakes.HTTPRequest.blank('/v2/os-aggregates',
+                                      use_admin_context=True,
+                                      version='2.81')
+        self.assertRaises(exc.HTTPBadRequest, self.controller.images,
+                          req, '1', body=body)
+
+    def test_images_with_disorder_id(self):
+        body = {"cache": [{"id": "faae1bd3-c848-41d6-b4dd-97d5b8be8b7e"},
+                          {"id": "290de658-cf55-4cce-b025-9a1a9f93676a"},
+                          {"id": "896f7f54-4e4e-4c21-a2b7-47cff4e99ab0"},
+                          {"id": "d982bb82-04a0-4e9b-b40e-470f20a7b5d1"}]}
+        req = fakes.HTTPRequest.blank('/v2/os-aggregates',
+                                      use_admin_context=True,
+                                      version='2.81')
+        context = req.environ['nova.context']
+        with mock.patch.object(self.controller.api,
+                               'get_aggregate') as mock_get:
+            with mock.patch.object(self.controller.conductor_tasks,
+                                   'cache_images'):
+                self.controller.images(req, '1', body=body)
+                mock_get.assert_called_once_with(context, '1')

nova/tests/unit/image/test_format_inspector.py

@@ -179,10 +179,16 @@ def _create_allocated_vmdk(self, size_mb, subformat=None):
             shell=True)
 
         # Convert it to VMDK
-        subprocess.check_output(
-            'qemu-img convert -f raw -O vmdk -o subformat=%s -S 0 %s %s' % (
-                subformat, raw, fn),
-            shell=True)
+        # these tests depend on qemu-img
+        # being installed and in the path,
+        # if it is not installed, skip
+        try:
+            subprocess.check_output(
+                'qemu-img convert -f raw -O vmdk -o subformat=%s -S 0 %s %s'
+                % (subformat, raw, fn),
+                shell=True)
+        except Exception:
+            self.skipTest("qemu-img not installed")
         return fn
 
     def _test_format_at_block_size(self, format_name, img, block_size):

nova/tests/unit/network/test_neutron.py

@@ -8627,6 +8627,73 @@ def test_unbind_ports_clean_arq(self, mock_neutron, mock_show,
         self.assertEqual(call_args['port']['binding:profile'],
             {'key': 'val'})
 
+    def test__get_vf_pci_device_profile_without_serial_num(self):
+        mydev = objects.PciDevice(
+            address='foo',
+            compute_node_id='123',
+            extra_info={
+                'capabilities': jsonutils.dumps({
+                    'sriov': {
+                        'pf_mac_address': '52:54:00:1e:59:c6',
+                        'vf_num': 1,
+                    },
+                    'network': ['gso', 'sg', 'tso', 'tx'],
+                }),
+            },
+        )
+        self.assertEqual(self.api._get_vf_pci_device_profile(mydev),
+                         {'pf_mac_address': '52:54:00:1e:59:c6',
+                          'vf_num': 1,
+                          'capabilities': ['gso', 'sg', 'tso', 'tx']})
+
+    def test__get_vf_pci_device_profile_without_pf_mac_addr(self):
+        mydev = objects.PciDevice(
+            address='foo',
+            compute_node_id='123',
+            extra_info={
+                'capabilities': jsonutils.dumps({
+                    'vpd': {'card_serial_number': 'MT2113X00000'},
+                    'sriov': {'vf_num': 1},
+                    'network': ['gso', 'sg', 'tso', 'tx'],
+                }),
+            },
+        )
+        self.assertEqual(self.api._get_vf_pci_device_profile(mydev),
+                         {'card_serial_number': 'MT2113X00000',
+                          'vf_num': 1,
+                          'capabilities': ['gso', 'sg', 'tso', 'tx']})
+
+    def test__get_vf_pci_device_profile_without_vf_num(self):
+        mydev = objects.PciDevice(
+            address='foo',
+            compute_node_id='123',
+            extra_info={
+                'capabilities': jsonutils.dumps({
+                    'vpd': {'card_serial_number': 'MT2113X00000'},
+                    'sriov': {'pf_mac_address': '52:54:00:1e:59:c6'},
+                    'network': ['gso', 'sg', 'tso', 'tx'],
+                }),
+            },
+        )
+        self.assertEqual(self.api._get_vf_pci_device_profile(mydev),
+                         {'card_serial_number': 'MT2113X00000',
+                          'pf_mac_address': '52:54:00:1e:59:c6',
+                          'capabilities': ['gso', 'sg', 'tso', 'tx']})
+
+    def test__get_vf_pci_device_profile_with_dev_capabilities(self):
+        mydev = objects.PciDevice(
+            address='foo',
+            compute_node_id='123',
+            extra_info={
+                'capabilities': jsonutils.dumps({
+                    'sriov': {},
+                    'network': ['gso', 'sg', 'tso', 'tx'],
+                }),
+            },
+        )
+        self.assertEqual(self.api._get_vf_pci_device_profile(mydev),
+                         {'capabilities': ['gso', 'sg', 'tso', 'tx']})
+
 
 class TestAllocateForInstance(test.NoDBTestCase):
     def setUp(self):

nova/tests/unit/virt/libvirt/cpu/test_api.py

@@ -244,3 +244,21 @@ def test_validate_all_dedicated_cpus_no_cpu(self):
         self.api.validate_all_dedicated_cpus()
         # no assert we want to make sure the validation won't raise if
         # no dedicated cpus are configured
+
+    @mock.patch.object(core, 'get_governor')
+    @mock.patch.object(core, 'get_online')
+    def test_validate_all_dedicated_cpus_for_cpu_state_with_off_cores(
+            self, mock_get_online, mock_get_governor):
+        self.flags(cpu_power_management=True, group='libvirt')
+        self.flags(cpu_dedicated_set='1-3', group='compute')
+        self.flags(cpu_power_management_strategy='cpu_state', group='libvirt')
+        # CPU1 and CPU3 are online while CPU2 is offline
+        mock_get_online.side_effect = (True, False, True)
+        mock_get_governor.return_value = 'performance'
+        self.api.validate_all_dedicated_cpus()
+
+        mock_get_online.assert_has_calls([mock.call(1), mock.call(2),
+                                          mock.call(3)])
+        # we only have two calls as CPU2 was skipped
+        mock_get_governor.assert_has_calls([mock.call(1),
+                                            mock.call(3)])

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

@@ -9385,7 +9385,7 @@ def test_set_quiesced_agent_connection_fails(self):
 
     def test_create_snapshot_metadata(self):
         base = objects.ImageMeta.from_dict(
-            {'disk_format': 'raw'})
+            {'disk_format': 'qcow2'})
         instance_data = {'kernel_id': 'kernel',
                     'project_id': 'prj_id',
                     'ramdisk_id': 'ram_id',
@@ -9417,10 +9417,12 @@ def test_create_snapshot_metadata(self):
             {'disk_format': 'ami',
              'container_format': 'test_container'})
         expected['properties']['os_type'] = instance['os_type']
-        expected['disk_format'] = base.disk_format
+        # The disk_format of the snapshot should be the *actual* format of the
+        # thing we upload, regardless of what type of image we booted from.
+        expected['disk_format'] = img_fmt
         expected['container_format'] = base.container_format
         ret = drvr._create_snapshot_metadata(base, instance, img_fmt, snp_name)
-        self.assertEqual(ret, expected)
+        self.assertEqual(expected, ret)
 
     def test_get_volume_driver(self):
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -29000,7 +29002,8 @@ def test_ami(self):
           utils.get_system_metadata_from_image(
             {'disk_format': 'ami'})
 
-        self._test_snapshot(disk_format='ami')
+        # If we're uploading a qcow2, we must set the disk_format as such
+        self._test_snapshot(disk_format='qcow2')
 
     @mock.patch('nova.virt.libvirt.utils.get_disk_type_from_path',
                 new=mock.Mock(return_value=None))

nova/tests/unit/virt/libvirt/volume/test_mount.py

@@ -580,6 +580,32 @@ def test_umount_log_failure(self, mock_log):
 
         mock_log.assert_called()
 
+    @mock.patch.object(mount.LOG, 'exception')
+    def test_mount_failure(self, mock_log_exc):
+        m = self._get_clean_hostmountstate()
+        err = processutils.ProcessExecutionError
+        self.mock_mount.side_effect = err
+
+        # Mount vol_a
+        self.assertRaises(err, self._sentinel_mount, m, mock.sentinel.vol_a)
+
+        # Verify the mountpoint got removed after the failure
+        self.assertEqual({}, m.mountpoints)
+
+        # Now try a scenario where the mount failed because the volume was
+        # already mounted
+        self.mock_ismount.side_effect = [False, True]
+
+        # Mount vol_a
+        self._sentinel_mount(m, mock.sentinel.vol_a)
+
+        # Verify the mountpoint is present despite the error
+        self.assertEqual(1, len(m.mountpoints))
+        self.assertIn(mock.sentinel.mountpoint, m.mountpoints)
+
+        # Verify we logged an exception
+        mock_log_exc.assert_called()
+
 
 class MountManagerTestCase(test.NoDBTestCase):
     class FakeHostMountState(object):

nova/tests/unit/virt/test_virt_drivers.py

@@ -838,6 +838,11 @@ def setUp(self):
         # since we don't care about it.
         self.stub_out('os_vif.unplug', lambda a, kw: None)
         self.stub_out('nova.compute.utils.get_machine_ips', lambda: [])
+        self.stub_out('nova.virt.libvirt.utils.get_disk_size',
+                      lambda *a, **k: 123456)
+        self.stub_out('nova.virt.libvirt.utils.get_disk_backing_file',
+                      lambda *a, **k: None)
+        self.stub_out('nova.privsep.path.chown', lambda *a, **k: None)
 
     def test_init_host_image_type_rbd_force_raw_images_true(self):
         CONF.set_override('images_type', 'rbd', group='libvirt')

nova/virt/libvirt/cpu/api.py

@@ -172,27 +172,29 @@ def validate_all_dedicated_cpus(self) -> None:
         if not CONF.libvirt.cpu_power_management:
             return
         cpu_dedicated_set = hardware.get_cpu_dedicated_set() or set()
-        governors = set()
-        cpu_states = set()
+        pcpus = []
         for pcpu in cpu_dedicated_set:
             if (pcpu == 0 and
                     CONF.libvirt.cpu_power_management_strategy == 'cpu_state'):
                 LOG.warning('CPU0 is in cpu_dedicated_set, '
                             'but it is not eligible for state management '
                             'and will be ignored')
                 continue
-            pcpu = self.core(pcpu)
             # we need to collect the governors strategy and the CPU states
-            governors.add(pcpu.governor)
-            cpu_states.add(pcpu.online)
+            pcpus.append(self.core(pcpu))
         if CONF.libvirt.cpu_power_management_strategy == 'cpu_state':
+            # NOTE(sbauza): offline cores can't have a governor, it returns a
+            # DeviceBusy exception.
+            governors = set([pcpu.governor for pcpu in pcpus
+                             if pcpu.online])
             # all the cores need to have the same governor strategy
             if len(governors) > 1:
                 msg = _("All the cores need to have the same governor strategy"
                         "before modifying the CPU states. You can reboot the "
                         "compute node if you prefer.")
                 raise exception.InvalidConfiguration(msg)
         elif CONF.libvirt.cpu_power_management_strategy == 'governor':
+            cpu_states = set([pcpu.online for pcpu in pcpus])
             # all the cores need to be online
             if False in cpu_states:
                 msg = _("All the cores need to be online before modifying the "

nova/virt/libvirt/driver.py

@@ -2967,11 +2967,7 @@ def _create_snapshot_metadata(self, image_meta, instance,
         if instance.os_type:
             metadata['properties']['os_type'] = instance.os_type
 
-        # NOTE(vish): glance forces ami disk format to be ami
-        if image_meta.disk_format == 'ami':
-            metadata['disk_format'] = 'ami'
-        else:
-            metadata['disk_format'] = img_fmt
+        metadata['disk_format'] = img_fmt
 
         if image_meta.obj_attr_is_set("container_format"):
             metadata['container_format'] = image_meta.container_format

nova/virt/libvirt/volume/mount.py

@@ -306,7 +306,7 @@ def mount(self, fstype, export, vol_name, mountpoint, instance, options):
 
                 try:
                     nova.privsep.fs.mount(fstype, export, mountpoint, options)
-                except processutils.ProcessExecutionError():
+                except processutils.ProcessExecutionError:
                     # Check to see if mountpoint is mounted despite the error
                     # eg it was already mounted
                     if os.path.ismount(mountpoint):

tools/check-cherry-picks.sh

@@ -1,7 +1,8 @@
 #!/bin/sh
 #
 # A tool to check the cherry-pick hashes from the current git commit message
-# to verify that they're all on either master or stable/ branches
+# to verify that they're all on either master, stable/ or unmaintained/
+# branches
 #
 
 commit_hash=""
@@ -14,7 +15,7 @@ if [ $parent_number -eq 2 ]; then
     commit_hash=$(git show --format='%P' --quiet | awk '{print $NF}')
 fi
 
-if git show --format='%aE' HEAD~ --quiet | grep -qi 'infra-root@openstack.org'; then
+if git show --format='%aE' --quiet $commit_hash | grep -qi 'infra-root@openstack.org'; then
     echo 'Bot generated change; ignoring'
     exit 0
 fi
@@ -23,17 +24,17 @@ hashes=$(git show --format='%b' --quiet $commit_hash | sed -nr 's/^.cherry picke
 checked=0
 branches+=""
 for hash in $hashes; do
-    branch=$(git branch -a --contains "$hash" 2>/dev/null| grep -oE '(master|stable/[a-z0-9.]+)')
+    branch=$(git branch -a --contains "$hash" 2>/dev/null| grep -oE '(master|stable/[a-z0-9.]+|unmaintained/[a-z0-9.]+)')
     if [ $? -ne 0 ]; then
-        echo "Cherry pick hash $hash not on any master or stable branches"
+        echo "Cherry pick hash $hash not on any master, stable or unmaintained branches"
         exit 1
     fi
     branches+=" $branch"
     checked=$(($checked + 1))
 done
 
 if [ $checked -eq 0 ]; then
-    if ! grep -q '^defaultbranch=stable/' .gitreview; then
+    if ! grep -qE '^defaultbranch=(stable|unmaintained)/' .gitreview; then
         echo "Checked $checked cherry-pick hashes: OK"
         exit 0
     else