only increment disk address unit for scsi devices
We were erroneously incrementing the disk address unit attribute for non-scsi devices, which resulted in inconsistent disk device naming and addresses when SCSI devices were used along with non-SCSI devices (like configdrive devices). Also, we ensure that we assign unit number 0 for the boot volume of a boot-from-volume instance. Co-authored-by: Mehdi Abaakouk <sileht@sileht.net> Closes-bug: #1729584 Closes-bug: #1753394 Change-Id: Ia91e2f9c316e25394a0f41dc341d903dfcff6921 (cherry picked from commit2616b384e6
) (cherry picked from commitf9c66434ee
) (cherry picked from commitb255e16bd9
)
This commit is contained in:
parent
c6490e957c
commit
1150d4a2af
@ -3547,6 +3547,158 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||
self.assertEqual(cfg.devices[4].model, 'virtio-scsi')
|
||||
mock_save.assert_called_with()
|
||||
|
||||
def test_get_guest_config_one_scsi_volume_with_configdrive(self):
|
||||
"""Tests that the unit attribute is only incremented for block devices
|
||||
that have a scsi bus. Unit numbering should begin at 0 since we are not
|
||||
booting from volume.
|
||||
"""
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
|
||||
|
||||
image_meta = objects.ImageMeta.from_dict({
|
||||
"disk_format": "raw",
|
||||
"properties": {"hw_scsi_model": "virtio-scsi",
|
||||
"hw_disk_bus": "scsi"}})
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
instance_ref.config_drive = 'True'
|
||||
conn_info = {'driver_volume_type': 'fake'}
|
||||
bdms = block_device_obj.block_device_make_list_from_dicts(
|
||||
self.context, [
|
||||
fake_block_device.FakeDbBlockDeviceDict(
|
||||
{'id': 1,
|
||||
'source_type': 'volume', 'destination_type': 'volume',
|
||||
'device_name': '/dev/sdc', 'disk_bus': 'scsi'}),
|
||||
]
|
||||
)
|
||||
bd_info = {
|
||||
'block_device_mapping': driver_block_device.convert_volumes(bdms)}
|
||||
bd_info['block_device_mapping'][0]['connection_info'] = conn_info
|
||||
|
||||
disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type,
|
||||
instance_ref,
|
||||
image_meta,
|
||||
bd_info)
|
||||
with mock.patch.object(
|
||||
driver_block_device.DriverVolumeBlockDevice, 'save'):
|
||||
cfg = drvr._get_guest_config(instance_ref, [], image_meta,
|
||||
disk_info, [], bd_info)
|
||||
|
||||
# The device order is determined by the order that devices are
|
||||
# appended in _get_guest_storage_config in the driver.
|
||||
|
||||
# The first device will be the instance's local disk (since we're
|
||||
# not booting from volume). It should begin unit numbering at 0.
|
||||
self.assertIsInstance(cfg.devices[0],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertIn('disk', cfg.devices[0].source_path)
|
||||
self.assertEqual('sda', cfg.devices[0].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[0].target_bus)
|
||||
self.assertEqual(0, cfg.devices[0].device_addr.unit)
|
||||
|
||||
# The second device will be the ephemeral disk
|
||||
# (the flavor in self.test_instance has ephemeral_gb > 0).
|
||||
# It should have the next unit number of 1.
|
||||
self.assertIsInstance(cfg.devices[1],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertIn('disk.local', cfg.devices[1].source_path)
|
||||
self.assertEqual('sdb', cfg.devices[1].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[1].target_bus)
|
||||
self.assertEqual(1, cfg.devices[1].device_addr.unit)
|
||||
|
||||
# This is the config drive. It should not have unit number set.
|
||||
self.assertIsInstance(cfg.devices[2],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertIn('disk.config', cfg.devices[2].source_path)
|
||||
self.assertEqual('hda', cfg.devices[2].target_dev)
|
||||
self.assertEqual('ide', cfg.devices[2].target_bus)
|
||||
self.assertIsNone(cfg.devices[2].device_addr)
|
||||
|
||||
# And this is the attached volume.
|
||||
self.assertIsInstance(cfg.devices[3],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertEqual('sdc', cfg.devices[3].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[3].target_bus)
|
||||
self.assertEqual(2, cfg.devices[3].device_addr.unit)
|
||||
|
||||
def test_get_guest_config_boot_from_volume_with_configdrive(self):
|
||||
"""Tests that the unit attribute is only incremented for block devices
|
||||
that have a scsi bus and that the bootable volume in a boot-from-volume
|
||||
scenario always has the unit set to 0.
|
||||
"""
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
|
||||
|
||||
image_meta = objects.ImageMeta.from_dict({
|
||||
"disk_format": "raw",
|
||||
"properties": {"hw_scsi_model": "virtio-scsi",
|
||||
"hw_disk_bus": "scsi"}})
|
||||
instance_ref = objects.Instance(**self.test_instance)
|
||||
instance_ref.config_drive = 'True'
|
||||
conn_info = {'driver_volume_type': 'fake'}
|
||||
bdms = block_device_obj.block_device_make_list_from_dicts(
|
||||
self.context, [
|
||||
# This is the boot volume (boot_index = 0).
|
||||
fake_block_device.FakeDbBlockDeviceDict(
|
||||
{'id': 1,
|
||||
'source_type': 'volume', 'destination_type': 'volume',
|
||||
'device_name': '/dev/sda', 'boot_index': 0}),
|
||||
# This is just another attached volume.
|
||||
fake_block_device.FakeDbBlockDeviceDict(
|
||||
{'id': 2,
|
||||
'source_type': 'volume', 'destination_type': 'volume',
|
||||
'device_name': '/dev/sdc', 'disk_bus': 'scsi'}),
|
||||
]
|
||||
)
|
||||
bd_info = {
|
||||
'block_device_mapping': driver_block_device.convert_volumes(bdms)}
|
||||
bd_info['block_device_mapping'][0]['connection_info'] = conn_info
|
||||
bd_info['block_device_mapping'][1]['connection_info'] = conn_info
|
||||
|
||||
disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type,
|
||||
instance_ref,
|
||||
image_meta,
|
||||
bd_info)
|
||||
with mock.patch.object(
|
||||
driver_block_device.DriverVolumeBlockDevice, 'save'):
|
||||
cfg = drvr._get_guest_config(instance_ref, [], image_meta,
|
||||
disk_info, [], bd_info)
|
||||
|
||||
# The device order is determined by the order that devices are
|
||||
# appended in _get_guest_storage_config in the driver.
|
||||
|
||||
# The first device will be the ephemeral disk
|
||||
# (the flavor in self.test_instance has ephemeral_gb > 0).
|
||||
# It should begin unit numbering at 1 because 0 is reserved for the
|
||||
# boot volume for boot-from-volume.
|
||||
self.assertIsInstance(cfg.devices[0],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertIn('disk.local', cfg.devices[0].source_path)
|
||||
self.assertEqual('sdb', cfg.devices[0].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[0].target_bus)
|
||||
self.assertEqual(1, cfg.devices[0].device_addr.unit)
|
||||
|
||||
# The second device will be the config drive. It should not have a
|
||||
# unit number set.
|
||||
self.assertIsInstance(cfg.devices[1],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertIn('disk.config', cfg.devices[1].source_path)
|
||||
self.assertEqual('hda', cfg.devices[1].target_dev)
|
||||
self.assertEqual('ide', cfg.devices[1].target_bus)
|
||||
self.assertIsNone(cfg.devices[1].device_addr)
|
||||
|
||||
# The third device will be the boot volume. It should have a
|
||||
# unit number of 0.
|
||||
self.assertIsInstance(cfg.devices[2],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertEqual('sda', cfg.devices[2].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[2].target_bus)
|
||||
self.assertEqual(0, cfg.devices[2].device_addr.unit)
|
||||
|
||||
# The fourth device will be the other attached volume.
|
||||
self.assertIsInstance(cfg.devices[3],
|
||||
vconfig.LibvirtConfigGuestDisk)
|
||||
self.assertEqual('sdc', cfg.devices[3].target_dev)
|
||||
self.assertEqual('scsi', cfg.devices[3].target_bus)
|
||||
self.assertEqual(2, cfg.devices[3].device_addr.unit)
|
||||
|
||||
def test_get_guest_config_with_vnc(self):
|
||||
self.flags(enabled=True, group='vnc')
|
||||
self.flags(virt_type='kvm', group='libvirt')
|
||||
|
@ -3622,7 +3622,7 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
LOG.debug('Config drive not found in RBD, falling back to the '
|
||||
'instance directory', instance=instance)
|
||||
disk_info = disk_mapping[name]
|
||||
if 'unit' in disk_mapping:
|
||||
if 'unit' in disk_mapping and disk_info['bus'] == 'scsi':
|
||||
disk_unit = disk_mapping['unit']
|
||||
disk_mapping['unit'] += 1 # Increments for the next disk added
|
||||
conf = disk.libvirt_info(disk_info['bus'],
|
||||
@ -3661,7 +3661,13 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
# use disk_mapping as container to keep reference of the
|
||||
# unit added and be able to increment it for each disk
|
||||
# added.
|
||||
#
|
||||
# NOTE(jaypipes,melwitt): If this is a boot-from-volume instance,
|
||||
# we need to start the disk mapping unit at 1 since we set the
|
||||
# bootable volume's unit to 0 for the bootable volume.
|
||||
disk_mapping['unit'] = 0
|
||||
if self._is_booted_from_volume(block_device_info):
|
||||
disk_mapping['unit'] = 1
|
||||
|
||||
def _get_ephemeral_devices():
|
||||
eph_devices = []
|
||||
@ -3751,8 +3757,14 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
info = disk_mapping[vol_dev]
|
||||
self._connect_volume(connection_info, info)
|
||||
if scsi_controller and scsi_controller.model == 'virtio-scsi':
|
||||
info['unit'] = disk_mapping['unit']
|
||||
disk_mapping['unit'] += 1
|
||||
# Check if this is the bootable volume when in a
|
||||
# boot-from-volume instance, and if so, ensure the unit
|
||||
# attribute is 0.
|
||||
if vol.get('boot_index') == 0:
|
||||
info['unit'] = 0
|
||||
else:
|
||||
info['unit'] = disk_mapping['unit']
|
||||
disk_mapping['unit'] += 1
|
||||
cfg = self._get_volume_config(connection_info, info)
|
||||
devices.append(cfg)
|
||||
vol['connection_info'] = connection_info
|
||||
|
Loading…
Reference in New Issue
Block a user