diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 118d0e48da8a..2202a80525ee 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -1228,6 +1228,10 @@ class Domain(object): if emulator_pin is not None: definition['emulator_pin'] = emulator_pin.get('cpuset') + iothreads = tree.find('./iothreads') + if iothreads is not None: + definition['iothreads'] = iothreads.text + iothread_pin = tree.find('./cputune/iothreadpin') if iothread_pin is not None: definition['iothread_pin'] = iothread_pin.get('cpuset') @@ -1711,6 +1715,10 @@ class Domain(object): cputune = '%s%s%s' % ( emulatorpin, iothreadpin, cputune) + iothreads = '' + if 'iothreads' in self._def: + iothreads = '%s' % self._def['iothreads'] + numatune = '' for cellid, nodeset in self._def['memnodes'].items(): numatune += '' % (int(cellid), @@ -1736,6 +1744,7 @@ class Domain(object): %(memory)s %(memory)s %(vcpu)s + %(iothreads)s hvm %(loader)s @@ -1787,6 +1796,7 @@ class Domain(object): 'memory': self._def['memory'], 'vcpuset': vcpuset, 'vcpu': self._def['vcpu']['number'], + 'iothreads': iothreads, 'arch': self._def['os']['arch'], 'loader': loader, 'disks': disks, @@ -2085,6 +2095,27 @@ class Connection(object): self._emit_lifecycle(dom, VIR_DOMAIN_EVENT_DEFINED, 0) return dom + # TODO(lajoskatona): Move this validation to defineXML once fix for + # bug/2140537 is merged. + # This method is only used temporarily from + # nova/tests/functional/regressions/test_bug_2140537.py + def _defineXMLIOThreads(self, xml): + xml_doc = etree.fromstring(xml.encode('utf-8')) + iothreadpin = xml_doc.find('./cputune/iothreadpin') + + if iothreadpin is not None and iothreadpin.get('iothread') is None: + raise make_libvirtError( + libvirtError, + "XML error: Missing required attribute 'iothread' " + "in element 'iothreadpin'", + error_code=VIR_ERR_XML_ERROR, + error_domain=VIR_FROM_DOMAIN) + + dom = Domain(connection=self, running=False, transient=False, xml=xml) + self._vms[dom.name()] = dom + self._emit_lifecycle(dom, VIR_DOMAIN_EVENT_DEFINED, 0) + return dom + def createXML(self, xml, flags): dom = Domain(connection=self, running=True, transient=True, xml=xml) self._vms[dom.name()] = dom diff --git a/nova/tests/functional/regressions/test_bug_2140537.py b/nova/tests/functional/regressions/test_bug_2140537.py new file mode 100644 index 000000000000..f7be2b0d4fd0 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2140537.py @@ -0,0 +1,300 @@ +# 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. + +"""Regression test for bug 2140537. + +https://bugs.launchpad.net/nova/+bug/2140537 + +When creating an instance with hw:cpu_policy=dedicated, pinned CPUs, +Nova was incorrectly setting 1 in the libvirt XML +without a corresponding element. This caused libvirt to +reject the XML with: "XML error: Missing required attribute 'iothread' +in element 'iothreadpin'". +""" + +from lxml import etree +from unittest import mock + +from nova.tests.fixtures import libvirt as fakelibvirt +from nova.tests.functional import integrated_helpers +from nova.tests.functional.libvirt import base + + +class TestIOThreadPinningPinnedCPU( + base.LibvirtMigrationMixin, + base.ServersTestBase, + integrated_helpers.InstanceHelperMixin +): + """Regression test for bug 2140537. + + Instances with dedicated CPU policy (pinned CPUs) should NOT have + IOThreads configured and have IOThreadPin in their domain xml, + as they are incompatible with CPU pinning. + """ + + microversion = 'latest' + ADMIN_API = True + ADDITIONAL_FILTERS = ['NUMATopologyFilter'] + + def setUp(self): + # TODO(lajoskatona): remove this patch when the fix for + # bug/2140537 is merged, and the libvirt fixture has the + # necessary validation for XML fields for IOThreads. + patcher = mock.patch.object( + fakelibvirt.Connection, 'defineXML', + fakelibvirt.Connection._defineXMLIOThreads) + patcher.start() + self.addCleanup(patcher.stop) + + super().setUp() + self.hostname = self.start_compute( + hostname='host1', + host_info=fakelibvirt.HostInfo( + cpu_nodes=2, cpu_sockets=1, cpu_cores=4, cpu_threads=1)) + self.host = self.computes['host1'] + + def get_host(self, server_id): + server = self.api.get_server(server_id) + return server['OS-EXT-SRV-ATTR:host'] + + def _get_xml_element(self, xml, xpath): + """Get element from XML using xpath.""" + xml_doc = etree.fromstring(xml.encode('utf-8')) + element = xml_doc.find(xpath) + return element + + def test_iothread_pinning_pinned_cpu(self): + """Test that pinned CPU instances fail with iothreadpin bug. + + This reproduces bug 2140537: When creating an instance with + hw:cpu_policy=dedicated, Nova incorrectly creates an + element without the required 'iothread' attribute, causing libvirt + to reject the XML. + """ + # Configure host with dedicated CPUs + self.flags( + cpu_shared_set='0,1', cpu_dedicated_set='2,3', group='compute') + self.restart_compute_service('host1') + + # Create VM with dedicated CPUs + extra_spec = {'hw:cpu_policy': 'dedicated'} + flavor = self._create_flavor(vcpu=1, extra_spec=extra_spec) + + # BUG: This fails with libvirt error about missing 'iothread' attribute + # The server creation will fail and go to ERROR state + server = self._create_server( + flavor_id=flavor, host='host1', networks='none', + expected_state='ERROR') + + # Verify the server is in ERROR state due to the libvirt XML error + self.assertEqual('ERROR', server['status']) + + # Check the fault message contains the libvirt error + self.assertIn('Exceeded maximum number of retries', + server['fault']['message']) + # Check the logs for the exception + self.assertIn("Missing required attribute 'iothread'", + self.stdlog.logger.output) + self.assertIn("element 'iothreadpin'", self.stdlog.logger.output) + + def test_iothread_pinning_explicit_numa(self): + """Test iothread pinning with explicit multi-node NUMA topology.""" + self.flags( + cpu_shared_set='0-1', cpu_dedicated_set='2-7', group='compute') + self.restart_compute_service('host1') + + # Create flavor with explicit 2-node NUMA topology + extra_spec = { + 'hw:cpu_policy': 'dedicated', + 'hw:numa_nodes': '2', + } + flavor = self._create_flavor(vcpu=4, extra_spec=extra_spec) + + # Server should go ACTIVE + # server = self._create_server( + # flavor_id=flavor, host='host1', networks='none', + # expected_state='ACTIVE') + server = self._create_server( + flavor_id=flavor, host='host1', networks='none', + expected_state='ERROR') + + # conn = self.host.driver._host.get_connection() + # dom = conn.lookupByUUIDString(server['id']) + # srv_xml = dom.XMLDesc(0) + + # # Should have iothreads element + # srv_iothread = self._get_xml_element(srv_xml, './iothreads') + # self.assertIsNotNone(srv_iothread) + # self.assertEqual('1', srv_iothread.text) + + # # Should have emulatorpin and iothreadpin + # srv_emulatorpin = self._get_xml_element( + # srv_xml, './cputune/emulatorpin') + # srv_iothreadpin = self._get_xml_element( + # srv_xml, './cputune/iothreadpin') + # self.assertIsNotNone(srv_emulatorpin) + # self.assertIsNotNone(srv_iothreadpin) + + # # iothreadpin should have iothread attribute set to 1 + # self.assertEqual('1', srv_iothreadpin.get('iothread')) + + # # Both should be pinned to the union of NUMA nodes + # self.assertEqual(srv_emulatorpin.get('cpuset'), + # srv_iothreadpin.get('cpuset')) + + self.assertEqual('ERROR', server['status']) + + # Check the fault message contains the libvirt error + self.assertIn('Exceeded maximum number of retries', + server['fault']['message']) + # Check the logs for the exception + self.assertIn("Missing required attribute 'iothread'", + self.stdlog.logger.output) + self.assertIn("element 'iothreadpin'", self.stdlog.logger.output) + + def test_iothread_pinning_isolated_emulator(self): + """Test iothread pinning with isolated emulator threads policy.""" + self.flags( + cpu_shared_set='0-1', cpu_dedicated_set='2-7', group='compute') + self.restart_compute_service('host1') + + # Create flavor with isolated emulator threads + extra_spec = { + 'hw:cpu_policy': 'dedicated', + 'hw:emulator_threads_policy': 'isolate', + } + flavor = self._create_flavor(vcpu=2, extra_spec=extra_spec) + + server = self._create_server( + flavor_id=flavor, host='host1', networks='none', + expected_state='ERROR') + # Server should go ACTIVE + # server = self._create_server( + # flavor_id=flavor, host='host1', networks='none', + # expected_state='ACTIVE') + + # conn = self.host.driver._host.get_connection() + # dom = conn.lookupByUUIDString(server['id']) + # srv_xml = dom.XMLDesc(0) + + # # Should have iothreads element + # srv_iothread = self._get_xml_element(srv_xml, './iothreads') + # self.assertIsNotNone(srv_iothread) + # self.assertEqual('1', srv_iothread.text) + + # # Should have emulatorpin and iothreadpin + # srv_emulatorpin = self._get_xml_element( + # srv_xml, './cputune/emulatorpin') + # srv_iothreadpin = self._get_xml_element( + # srv_xml, './cputune/iothreadpin') + # self.assertIsNotNone(srv_emulatorpin) + # self.assertIsNotNone(srv_iothreadpin) + + # # iothreadpin should have iothread attribute set to 1 + # self.assertEqual('1', srv_iothreadpin.get('iothread')) + + # # Both should be pinned to the same reserved/isolated CPU + # self.assertEqual(srv_emulatorpin.get('cpuset'), + # srv_iothreadpin.get('cpuset')) + + # # Should be pinned to a single CPU (the reserved one) + # # With vcpu=2 and isolate policy, one extra CPU is reserved + # cpuset = srv_iothreadpin.get('cpuset') + # # The cpuset should be a single CPU from cpu_dedicated_set + # self.assertIsNotNone(cpuset) + self.assertEqual('ERROR', server['status']) + + # Check the fault message contains the libvirt error + self.assertIn('Exceeded maximum number of retries', + server['fault']['message']) + # Check the logs for the exception + self.assertIn("Missing required attribute 'iothread'", + self.stdlog.logger.output) + self.assertIn("element 'iothreadpin'", self.stdlog.logger.output) + + def test_iothread_pinning_shared_emulator(self): + """Test iothread pinning with shared emulator threads policy.""" + self.flags( + cpu_shared_set='0-1', cpu_dedicated_set='2-7', group='compute') + self.restart_compute_service('host1') + + # Create flavor with shared emulator threads + extra_spec = { + 'hw:cpu_policy': 'dedicated', + 'hw:emulator_threads_policy': 'share', + } + flavor = self._create_flavor(vcpu=2, extra_spec=extra_spec) + + server = self._create_server( + flavor_id=flavor, host='host1', networks='none', + expected_state='ERROR') + # Server should go ACTIVE + # server = self._create_server( + # flavor_id=flavor, host='host1', networks='none', + # expected_state='ACTIVE') + + # conn = self.host.driver._host.get_connection() + # dom = conn.lookupByUUIDString(server['id']) + # srv_xml = dom.XMLDesc(0) + + # # Should have iothreads element + # srv_iothread = self._get_xml_element(srv_xml, './iothreads') + # self.assertIsNotNone(srv_iothread) + # self.assertEqual('1', srv_iothread.text) + + # # Should have emulatorpin and iothreadpin + # srv_emulatorpin = self._get_xml_element( + # srv_xml, './cputune/emulatorpin') + # srv_iothreadpin = self._get_xml_element( + # srv_xml, './cputune/iothreadpin') + # self.assertIsNotNone(srv_emulatorpin) + # self.assertIsNotNone(srv_iothreadpin) + + # # iothreadpin should have iothread attribute set to 1 + # self.assertEqual('1', srv_iothreadpin.get('iothread')) + + # # Both should be pinned to cpu_shared_set (0-1) + # self.assertEqual(srv_emulatorpin.get('cpuset'), + # srv_iothreadpin.get('cpuset')) + # self.assertEqual('0-1', srv_iothreadpin.get('cpuset')) + + self.assertEqual('ERROR', server['status']) + + # Check the fault message contains the libvirt error + self.assertIn('Exceeded maximum number of retries', + server['fault']['message']) + # Check the logs for the exception + self.assertIn("Missing required attribute 'iothread'", + self.stdlog.logger.output) + self.assertIn("element 'iothreadpin'", self.stdlog.logger.output) + + def test_iothread_no_pinning(self): + # No CPU pinning (shared CPUs only) + # Expects: NO cputune element at all (no emulatorpin, no iothreadpin) + flavor = self._create_flavor(vcpu=1,) + server = self._create_server( + flavor_id=flavor, host='host1', networks='none', + expected_state='ACTIVE') + conn = self.host.driver._host.get_connection() + dom = conn.lookupByUUIDString(server['id']) + srv_xml = dom.XMLDesc(0) + + srv_iothread = self._get_xml_element(srv_xml, './iothreads') + srv_iothreadpin = self._get_xml_element( + srv_xml, './cputune/iothreadpin') + + # iothreads should be set to 1 for all instances + self.assertIsNotNone(srv_iothread) + self.assertEqual('1', srv_iothread.text) + + # And no pinning for shared CPUs, no iothreadpin + self.assertIsNone(srv_iothreadpin)