womax/nova
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libvirt: Ensure device alias is present

Browse Source 
Our event listener depends on devices having an alias set. We add an
early assertion to prove this is the case. In real-life, this will
always be the case since the devices we can detach - like interfaces and
disks - are among the lists of devices that libvirt will automatically
generate an alias for if the user (nova-compute, in this case) doesn't
provide their own [1]. However, many of our tests were not doing this so
we must update our LibvirtFixture in particular to start doing so.

[1] https://github.com/libvirt/libvirt/blob/v11.10.0/src/qemu/qemu_alias.c#L692-L786

Change-Id: Id98d8029af673ffa89d6472be98f90a6f0975511
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2025-12-18 14:56:01 +00:00
parent c7d37ee3ce
commit aec74c1f23
3 changed files with 66 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nova/tests/fixtures/libvirt.py
Vendored
+44 -3
View File
@@ -917,6 +917,10 @@ def _parse_disk_info(element):
disk_info['target_dev'] = target.get('dev')
disk_info['target_bus'] = target.get('bus')
alias = element.find('./alias')
if alias is not None:
disk_info['alias'] = alias.get('name')
return disk_info
@@ -962,6 +966,10 @@ def _parse_nic_info(element):
if target is not None:
nic_info['target_dev'] = target.get('dev')
alias = element.find('./alias')
if alias is not None:
nic_info['alias'] = alias.get('name')
return nic_info
@@ -979,6 +987,10 @@ def _parse_hostdev_info(element):
hostdev_info['slot'] = address.get('slot')
hostdev_info['function'] = address.get('function')
alias = element.find('./alias')
if alias is not None:
hostdev_info['alias'] = alias.get('name')
return hostdev_info
@@ -1222,8 +1234,16 @@ class Domain(object):
if device_nodes is not None:
disks_info = []
disks = device_nodes.findall('./disk')
for disk in disks:
disks_info += [_parse_disk_info(disk)]
for idx, disk in enumerate(disks):
disk_info = _parse_disk_info(disk)
alias = disk.find('./alias')
if alias is not None:
disk_info['alias'] = alias.get('name')
else:
disk_info['alias'] = f'disk{idx}'
disks_info.append(disk_info)
devices['disks'] = disks_info
# Manage shares
@@ -1235,7 +1255,7 @@ class Domain(object):
nics_info = []
nics = device_nodes.findall('./interface')
for nic in nics:
for idx, nic in enumerate(nics):
nic_info = {}
nic_info['type'] = nic.get('type')
@@ -1243,6 +1263,12 @@ class Domain(object):
if mac is not None:
nic_info['mac'] = mac.get('address')
alias = nic.find('./alias')
if alias is not None:
nic_info['alias'] = alias.get('name')
else:
nic_info['alias'] = f'net{idx}'
source = nic.find('./source')
if source is not None:
if nic_info['type'] == 'network':
@@ -1409,16 +1435,25 @@ class Domain(object):
if xml.startswith("<disk"):
disk_info = _parse_disk_info(etree.fromstring(xml))
disk_info['_attached'] = True
if 'alias' not in disk_info:
idx = len(self._def['devices']['disks'])
disk_info['alias'] = f'disk{idx}'
self._def['devices']['disks'] += [disk_info]
result = True
elif xml.startswith("<interface"):
nic_info = _parse_nic_info(etree.fromstring(xml))
nic_info['_attached'] = True
if 'alias' not in nic_info:
idx = len(self._def['devices']['nics'])
nic_info['alias'] = f'net{idx}'
self._def['devices']['nics'] += [nic_info]
result = True
elif xml.startswith("<hostdev"):
hostdev_info = _parse_hostdev_info(etree.fromstring(xml))
hostdev_info['_attached'] = True
if 'alias' not in hostdev_info:
idx = len(self._def['devices']['hostdevs'])
hostdev_info['alias'] = f'hostdev{idx}'
self._def['devices']['hostdevs'] += [hostdev_info]
result = True
else:
@@ -1518,7 +1553,9 @@ class Domain(object):
strformat = """
<disk type='%(type)s' device='%(device)s'>
<driver name='%(driver_name)s' type='%(driver_type)s'/>
<alias name='%(alias)s'/>
<source %(source_attr)s='%(source)s'"""
if 'encryption_format' not in disk:
strformat += '/>'
else:
@@ -1555,6 +1592,7 @@ class Domain(object):
# this branch covers kernel ovs interfaces
nics += '''<interface type='%(type)s'>
<mac address='%(mac)s'/>
<alias name='%(alias)s'/>
<target dev='tap274487d1-6%(func)s'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03'
function='0x%(func)s'/>
@@ -1563,6 +1601,7 @@ class Domain(object):
# this branch covers hardware offloaded ovs with vdpa
nics += '''<interface type='%(type)s'>
<mac address='%(mac)s'/>
<alias name='%(alias)s'/>
<source dev='%(source)s'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03'
function='0x%(func)s'/>
@@ -1572,6 +1611,7 @@ class Domain(object):
elif 'source' in nic:
nics += '''<interface type='%(type)s'>
<mac address='%(mac)s'/>
<alias name='%(alias)s'/>
<source %(type)s='%(source)s'/>
<target dev='tap274487d1-6%(func)s'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03'
@@ -1584,6 +1624,7 @@ class Domain(object):
# the fixture we hard code it.
nics += '''<interface type='%(type)s'>
<mac address='%(mac)s'/>
<alias name='%(alias)s'/>
<source dev='fake_pf_interface_name' mode='passthrough'>
<address type='pci' domain='0x0000' bus='0x81' slot='0x00'
function='0x%(func)s'/>
nova/tests/unit/virt/libvirt/test_driver.py
+16 -6
View File
@@ -10726,17 +10726,22 @@ class LibvirtConnTestCase(test.NoDBTestCase,
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
instance = objects.Instance(**self.test_instance)
volume_id = uuids.volume
alias_xml = '<alias name="%s"/>' % vconfig.make_libvirt_device_alias(
volume_id)
if use_alias:
alias = vconfig.make_libvirt_device_alias(volume_id)
else:
# if we haven't created our own alias, libvirt will generate one
# for us
alias = 'virtio-disk0'
mock_xml_with_disk = """<domain>
<devices>
<disk type='file'>
%(alias)s
<alias name="%(alias)s"/>
<source file='/path/to/fake-volume'/>
<target dev='vdc' bus='virtio'/>
</disk>
</devices>
</domain>""" % {'alias': use_alias and alias_xml or ''}
</domain>""" % {'alias': alias}
mock_xml_without_disk = """<domain>
<devices>
</devices>
@@ -10781,10 +10786,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock_get_domain.assert_called_with(instance)
xml = """<disk type="file" device="disk">
%(alias)s
<alias name="%(alias)s"/>
<source file="/path/to/fake-volume"/>
<target bus="virtio" dev="vdc"/>
</disk>""" % {'alias': use_alias and alias_xml or ''}
</disk>""" % {'alias': alias}
# we expect two separate detach calls
self.assertEqual(2, mock_dom.detachDeviceFlags.call_count)
# one for the persistent domain
@@ -26094,6 +26099,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
<devices>
<interface type='bridge'>
<mac address='52:54:00:f6:35:8f'/>
<alias name='bridge0'/>
<model type='virtio'/>
<source bridge='br0'/>
<target dev='tap12345678'/>
@@ -26109,6 +26115,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
expected_cfg.parse_str("""
<interface type='bridge'>
<mac address='52:54:00:f6:35:8f'/>
<alias name='bridge0'/>
<model type='virtio'/>
<source bridge='br0'/>
<target dev='tap12345678'/>
@@ -26250,6 +26257,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
<devices>
<interface type='bridge'>
<mac address='52:54:00:f6:35:8f'/>
<alias name='bridge0'/>
<model type='virtio'/>
<source bridge='br0'/>
<target dev='tap12345678'/>
@@ -26258,6 +26266,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
</interface>
<interface type='bridge'>
<mac address='52:54:00:f6:35:8f'/>
<alias name='bridge1'/>
<model type='virtio'/>
<source bridge='br1'/>
<target dev='tap87654321'/>
@@ -26272,6 +26281,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
expected.parse_str("""
<interface type='bridge'>
<mac address='52:54:00:f6:35:8f'/>
<alias name='bridge0'/>
<model type='virtio'/>
<source bridge='br0'/>
<target dev='tap12345678'/>
nova/virt/libvirt/driver.py
+6
View File
@@ -2673,6 +2673,12 @@ class LibvirtDriver(driver.ComputeDriver):
synchronously.
:raises DeviceDetachFailed: if libvirt sent DeviceRemovalFailedEvent
"""
if dev.alias is None:
# our event handler needs a device alias, and we should never get
# here without one
msg = 'Device %s has no alias. This should not happen.'
raise exception.InternalError(msg % dev)
# So we will issue an detach to libvirt and we will wait for an
# event from libvirt about the result. We need to set up the event
# handling before the detach to avoid missing the event if libvirt