From c8d34ed3dc1ffc174ee56d3ca55e922afd75f619 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Tue, 3 Feb 2026 16:53:30 +0000 Subject: [PATCH] Fix blockio generation for LUN volumes QEMU's scsi-block device driver does not support physical_block_size and logical_block_size properties. When Cinder reports disk geometry for LUN volumes, Nova was incorrectly including a element in the libvirt XML, causing QEMU to fail with: Property 'scsi-block.physical_block_size' not found This fix adds a check to skip blockio generation when source_device is 'lun', following the existing pattern used for serial at line 1356. Generated-By: claude-code (Claude Opus 4.5) Closes-Bug: #2127196 Change-Id: Idf87e936edd97aac719222942c9842a9aca4c270 Signed-off-by: Sean Mooney --- nova/tests/fixtures/libvirt.py | 26 +++- .../regressions/test_bug_2127196.py | 125 ++++++++++++++++++ nova/tests/unit/virt/libvirt/test_config.py | 25 ++++ .../unit/virt/libvirt/volume/test_volume.py | 30 +++++ nova/virt/libvirt/config.py | 8 +- ...-2127196-lun-blockio-1a2b3c4d5e6f7890.yaml | 12 ++ 6 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 nova/tests/functional/regressions/test_bug_2127196.py create mode 100644 releasenotes/notes/bug-2127196-lun-blockio-1a2b3c4d5e6f7890.yaml diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index de0b88b87c67..118d0e48da8a 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -921,6 +921,13 @@ def _parse_disk_info(element): if alias is not None: disk_info['alias'] = alias.get('name') + blockio = element.find('./blockio') + if blockio is not None: + disk_info['blockio_logical_block_size'] = blockio.get( + 'logical_block_size') + disk_info['blockio_physical_block_size'] = blockio.get( + 'physical_block_size') + return disk_info @@ -1570,7 +1577,24 @@ class Domain(object): """ strformat += """ -
+
""" + + # Add blockio if present + if 'blockio_logical_block_size' in disk or \ + 'blockio_physical_block_size' in disk: + blockio_attrs = [] + if disk.get('blockio_logical_block_size'): + blockio_attrs.append( + "logical_block_size='%s'" % + disk['blockio_logical_block_size']) + if disk.get('blockio_physical_block_size'): + blockio_attrs.append( + "physical_block_size='%s'" % + disk['blockio_physical_block_size']) + strformat += """ + """ % ' '.join(blockio_attrs) + + strformat += """ """ disks += strformat % dict(source_attr=source_attr, **disk) filesystems = '' diff --git a/nova/tests/functional/regressions/test_bug_2127196.py b/nova/tests/functional/regressions/test_bug_2127196.py new file mode 100644 index 000000000000..8c8d1c7e8be7 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2127196.py @@ -0,0 +1,125 @@ +# 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 2127196. + +https://bugs.launchpad.net/nova/+bug/2127196 + +When Cinder reports disk geometry (logical/physical block size) for LUN +volumes, Nova incorrectly includes a element in the libvirt XML. +QEMU's scsi-block device driver (used for device="lun") does not support +these properties, causing QEMU to fail with: + + Property 'scsi-block.physical_block_size' not found + +The fix ensures that is not generated for LUN devices. +""" + +from lxml import etree +from oslo_utils.fixture import uuidsentinel as uuids + +import fixtures + +from nova.tests.fixtures import cinder as cinder_fixture +from nova.tests.fixtures import libvirt as fakelibvirt +from nova.tests.functional import integrated_helpers +from nova.tests.functional.libvirt import base + + +class CinderFixtureWithLunBlockSize(cinder_fixture.CinderFixture): + """CinderFixture that provides a LUN volume with block size info.""" + + # Volume ID for a LUN volume with block size info + LUN_VOLUME_WITH_BLOCKSIZE = uuids.lun_volume_with_blocksize + + def __init__(self, test, az='nova'): + super().__init__(test, az) + # Add connection_info for the LUN volume that includes block size + self.VOLUME_CONNECTION_INFO[self.LUN_VOLUME_WITH_BLOCKSIZE] = { + 'driver_volume_type': 'iscsi', + 'data': { + 'target_lun': '1', + 'logical_block_size': '512', + 'physical_block_size': '512', + } + } + + +class TestLunVolumeBlockio( + base.ServersTestBase, + integrated_helpers.InstanceHelperMixin +): + """Regression test for bug 2127196. + + Tests that blockio is not generated for LUN volumes even when Cinder + provides block size information. + """ + + microversion = 'latest' + ADMIN_API = True + + def setUp(self): + super().setUp() + self.cinder = self.useFixture(CinderFixtureWithLunBlockSize(self)) + self.useFixture(fixtures.MockPatch( + 'nova.compute.manager.ComputeVirtAPI.wait_for_instance_event')) + self.start_compute( + hostname='compute1', + host_info=fakelibvirt.HostInfo( + cpu_nodes=1, cpu_sockets=1, cpu_cores=4, cpu_threads=1)) + + 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_lun_volume_no_blockio(self): + """Test that blockio is not included for LUN volumes. + + When Cinder reports block size information for a LUN volume, + Nova should NOT include in the libvirt XML because + QEMU's scsi-block driver does not support these properties. + """ + # Build a server with a LUN volume as the boot device + server = self._build_server(image_uuid='', networks='none') + server['block_device_mapping_v2'] = [{ + 'boot_index': 0, + 'uuid': CinderFixtureWithLunBlockSize.LUN_VOLUME_WITH_BLOCKSIZE, + 'source_type': 'volume', + 'destination_type': 'volume', + 'device_type': 'lun', + 'disk_bus': 'scsi', + }] + + created_server = self.api.post_server({'server': server}) + self._wait_for_state_change(created_server, 'ACTIVE') + + # Get the libvirt XML + conn = self.computes['compute1'].driver._host.get_connection() + dom = conn.lookupByUUIDString(created_server['id']) + xml = dom.XMLDesc(0) + + # Find the disk element for the LUN volume + xml_doc = etree.fromstring(xml.encode('utf-8')) + disk = xml_doc.find('.//disk[@device="lun"]') + self.assertIsNotNone( + disk, "Expected to find a disk with device='lun' in the XML") + + # Verify that blockio is NOT present for the LUN device + # This is the correct behavior after the fix + blockio = disk.find('blockio') + self.assertIsNone( + blockio, + "Bug 2127196: blockio should NOT be present for LUN devices " + "because QEMU's scsi-block driver does not support " + "physical_block_size and logical_block_size properties") diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index 8bc1c8411684..2816878aaeb1 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/nova/tests/unit/virt/libvirt/test_config.py @@ -1159,6 +1159,31 @@ class LibvirtConfigGuestDiskTest(LibvirtConfigBaseTest): """) + def test_config_block_lun_no_blockio(self): + """Test that blockio is not generated for LUN devices (bug 2127196). + + QEMU's scsi-block device driver does not support physical_block_size + and logical_block_size properties, so blockio must not be included + in the XML when source_device is 'lun'. + """ + obj = config.LibvirtConfigGuestDisk() + obj.source_type = "block" + obj.source_path = "/tmp/hello" + obj.source_device = "lun" + obj.driver_name = "qemu" + obj.target_dev = "/dev/sda" + obj.target_bus = "scsi" + obj.logical_block_size = "512" + obj.physical_block_size = "512" + + xml = obj.to_xml() + self.assertXmlEqual(xml, """ + + + + + """) + def test_config_block_parse(self): xml = """ diff --git a/nova/tests/unit/virt/libvirt/volume/test_volume.py b/nova/tests/unit/virt/libvirt/volume/test_volume.py index f837893f1871..1295b90535ec 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_volume.py +++ b/nova/tests/unit/virt/libvirt/volume/test_volume.py @@ -206,6 +206,36 @@ class LibvirtVolumeTestCase(LibvirtISCSIVolumeBaseTestCase): self.assertEqual('4096', blockio.get('logical_block_size')) self.assertEqual('4096', blockio.get('physical_block_size')) + def test_libvirt_volume_driver_blockio_not_for_lun(self): + """Test blockio is not set for LUN devices (bug 2127196). + + QEMU's scsi-block device driver does not support physical_block_size + and logical_block_size properties, so blockio must not be included + when device type is 'lun'. + """ + libvirt_driver = volume.LibvirtVolumeDriver(self.fake_host) + connection_info = { + 'driver_volume_type': 'fake', + 'data': { + 'device_path': '/foo', + 'logical_block_size': '4096', + 'physical_block_size': '4096', + }, + 'serial': 'fake_serial', + } + disk_info = { + "bus": "scsi", + "dev": "sda", + "type": "lun", + } + conf = libvirt_driver.get_config(connection_info, disk_info) + tree = conf.format_dom() + self.assertEqual('lun', tree.get('device')) + # blockio should NOT be present for LUN devices + self.assertIsNone(tree.find('./blockio')) + # serial should also NOT be present for LUN devices + self.assertIsNone(tree.find('./serial')) + def test_libvirt_volume_driver_iotune(self): libvirt_driver = volume.LibvirtVolumeDriver(self.fake_host) connection_info = { diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py index b129c12bc4f3..849a1c7f6b09 100644 --- a/nova/virt/libvirt/config.py +++ b/nova/virt/libvirt/config.py @@ -1359,8 +1359,12 @@ class LibvirtConfigGuestDisk(LibvirtConfigGuestDevice): self._format_iotune(dev) # Block size tuning - if (self.logical_block_size is not None or - self.physical_block_size is not None): + # NOTE(bug 2127196): Block size properties are not supported for + # device='lun' because QEMU's scsi-block driver does not support + # physical_block_size and logical_block_size properties. + if (self.source_device != 'lun' and + (self.logical_block_size is not None or + self.physical_block_size is not None)): blockio = etree.Element("blockio") if self.logical_block_size is not None: diff --git a/releasenotes/notes/bug-2127196-lun-blockio-1a2b3c4d5e6f7890.yaml b/releasenotes/notes/bug-2127196-lun-blockio-1a2b3c4d5e6f7890.yaml new file mode 100644 index 000000000000..1aa2bd2a78ce --- /dev/null +++ b/releasenotes/notes/bug-2127196-lun-blockio-1a2b3c4d5e6f7890.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + `Bug 2127196`_ is fixed where Nova incorrectly generated a ```` + element in libvirt XML for LUN volumes when Cinder reported disk geometry + (logical and physical block size). QEMU's ``scsi-block`` device driver + does not support ``physical_block_size`` and ``logical_block_size`` + properties, causing instance spawn to fail with the error + ``Property 'scsi-block.physical_block_size' not found``. + The ```` element is now omitted for LUN devices. + + .. _Bug 2127196: https://bugs.launchpad.net/nova/+bug/2127196