From a93092e0d5c1483f9ad48276708ee35c54ce44fe Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Sat, 6 Aug 2022 16:09:54 +0200 Subject: [PATCH] Update RequestSpec.pci_request for resize Nova uses the RequestSpec.pci_request in the PciPassthroughFilter to decide if the PCI devicesm, requested via the pci_alias in the flavor extra_spec, are available on a potential target host. During resize the new flavor might contain different pci_alias request than the old flavor of the instance. In this case Nova should use the pci_alias from the new flavor to scheduler the destination host of the resize. However this logic was missing and Nova used the old pci_request value based on the old flavor. This patch adds the missing logic. Closes-Bug: #1983753 Closes-Bug: #1941005 Change-Id: I73c9ae27e9c42ee211a53bed3d849650b65f08be --- nova/compute/api.py | 13 ++++++++++ .../libvirt/test_pci_sriov_servers.py | 15 ++++++----- .../regressions/test_bug_1983753.py | 26 ++++--------------- nova/tests/unit/compute/test_api.py | 3 ++- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 26ae3cf0f3d9..66543f57dc0a 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4227,6 +4227,19 @@ class API: if not same_flavor: request_spec.numa_topology = hardware.numa_get_constraints( new_flavor, instance.image_meta) + # if the flavor is changed then we need to recalculate the + # pci_requests as well because the new flavor might request + # different pci_aliases + new_pci_requests = pci_request.get_pci_requests_from_flavor( + new_flavor) + new_pci_requests.instance_uuid = instance.uuid + # The neutron based InstancePCIRequest cannot change during resize, + # so we just need to copy them from the old request + for request in request_spec.pci_requests.requests or []: + if request.source == objects.InstancePCIRequest.NEUTRON_PORT: + new_pci_requests.requests.append(request) + request_spec.pci_requests = new_pci_requests + # TODO(huaqiang): Remove in Wallaby # check nova-compute nodes have been updated to Victoria to resize # instance to a new mixed instance from a dedicated or shared diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index df3e0468b9fd..2ab9d6318f19 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -1389,13 +1389,16 @@ class PCIServersTest(_PCIServersTestBase): # Resize it to a flavor without PCI devices. We expect this to work, as # test_compute1 is available. - # FIXME(artom) This is bug 1941005. flavor_id = self._create_flavor() - ex = self.assertRaises(client.OpenStackApiException, - self._resize_server, server, flavor_id) - self.assertEqual(500, ex.response.status_code) - self.assertIn('NoValidHost', str(ex)) - # self._confirm_resize(server) + with mock.patch( + 'nova.virt.libvirt.driver.LibvirtDriver' + '.migrate_disk_and_power_off', + return_value='{}', + ): + self._resize_server(server, flavor_id) + self._confirm_resize(server) + self.assertPCIDeviceCounts('test_compute0', total=1, free=1) + self.assertPCIDeviceCounts('test_compute1', total=0, free=0) def _confirm_resize(self, server, host='host1'): # NOTE(sbauza): Unfortunately, _cleanup_resize() in libvirt checks the diff --git a/nova/tests/functional/regressions/test_bug_1983753.py b/nova/tests/functional/regressions/test_bug_1983753.py index a11ea8dc63ef..78499335ec96 100644 --- a/nova/tests/functional/regressions/test_bug_1983753.py +++ b/nova/tests/functional/regressions/test_bug_1983753.py @@ -16,7 +16,6 @@ import fixtures from oslo_serialization import jsonutils from nova.tests.fixtures import libvirt as fakelibvirt -from nova.tests.functional.api import client from nova.tests.functional.libvirt import test_pci_sriov_servers @@ -119,19 +118,10 @@ class TestPciResize(test_pci_sriov_servers._PCIServersTestBase): "compute2", total=num_pci_on_dest, free=num_pci_on_dest - 1) def test_resize_from_two_devs_to_one_dev_dest_has_two_devs(self): - # this works self._test_resize_from_two_devs_to_one_dev(num_pci_on_dest=2) def test_resize_from_two_devs_to_one_dev_dest_has_one_dev(self): - # This is bug 1983753 as nova uses the old InstancePciRequest during - # the scheduling and therefore tries to find a compute with two PCI - # devs even though the flavor only requests one. - ex = self.assertRaises( - client.OpenStackApiException, - self._test_resize_from_two_devs_to_one_dev, - num_pci_on_dest=1 - ) - self.assertIn('nova.exception.NoValidHost', str(ex)) + self._test_resize_from_two_devs_to_one_dev(num_pci_on_dest=1) def test_resize_from_vf_to_pf(self): # The fake libvirt will emulate on the host: @@ -181,13 +171,7 @@ class TestPciResize(test_pci_sriov_servers._PCIServersTestBase): # dev. This should fit to compute2 having exactly one PF dev. extra_spec = {"pci_passthrough:alias": "a-pf:1"} flavor_id = self._create_flavor(extra_spec=extra_spec) - # This is bug 1983753 as nova uses the old InstancePciRequest during - # the scheduling and therefore tries to find a compute with a VF dev - # even though the flavor only requests a PF dev. - ex = self.assertRaises( - client.OpenStackApiException, - self._resize_server, - server, - flavor_id=flavor_id, - ) - self.assertIn('nova.exception.NoValidHost', str(ex)) + self._resize_server(server, flavor_id=flavor_id) + self._confirm_resize(server) + self.assertPCIDeviceCounts("compute1", total=1, free=1) + self.assertPCIDeviceCounts("compute2", total=1, free=0) diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index d0e7ac505dc0..f62f8c7477ca 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -2089,7 +2089,8 @@ class _ComputeAPIUnitTestMixIn(object): filter_properties = {'ignore_hosts': [fake_inst['host']]} if request_spec: - fake_spec = objects.RequestSpec() + fake_spec = objects.RequestSpec( + pci_requests=objects.InstancePCIRequests(requests=[])) if requested_destination: cell1 = objects.CellMapping(uuid=uuids.cell1, name='cell1') fake_spec.requested_destination = objects.Destination(