Do not update root_device_name during guest config

_get_guest_config() is currently updating instance.root_device_name
and called in many ways like:

_hard_reboot(), rescue(), spawn(), resume(), finish_migration(),
finish_revert_migration()

It is an issue because root_device_name is initally set during instance
build and should remain the same after:

manager.py: _do_build_and_run_instance()
             ..
               _default_block_device_names() <-here
               ..
               driver.spawn()

This may lead to edge case, like in rescue where this value can be mistakenly
updated to reflect disk bus property of rescue image (hw_disk_bus).
Further more, a _get* method should not modify instance object.

Note that test test_get_guest_config_bug_1118829 is removed because no more
relevant with current code.

Change-Id: I1787f9717618d0837208844e8065840d30341cf7
Closes-Bug: #1835926
(cherry picked from commit 5e0ed5e7fe)
(cherry picked from commit 5e858d0cbd)
(cherry picked from commit 9f9f8d330a)
This commit is contained in:
Alexandre Arents 2019-07-09 16:13:01 +00:00
parent de00ef96a9
commit c075e3a76d
2 changed files with 28 additions and 28 deletions

View File

@ -3730,29 +3730,6 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertIsInstance(cfg.devices[9], self.assertIsInstance(cfg.devices[9],
vconfig.LibvirtConfigMemoryBalloon) vconfig.LibvirtConfigMemoryBalloon)
def test_get_guest_config_bug_1118829(self):
self.flags(virt_type='uml', group='libvirt')
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
instance_ref = objects.Instance(**self.test_instance)
disk_info = {'disk_bus': 'virtio',
'cdrom_bus': 'ide',
'mapping': {u'vda': {'bus': 'virtio',
'type': 'disk',
'dev': u'vda'},
'root': {'bus': 'virtio',
'type': 'disk',
'dev': 'vda'}}}
# NOTE(jdg): For this specific test leave this blank
# This will exercise the failed code path still,
# and won't require fakes and stubs of the iscsi discovery
block_device_info = {}
image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
drvr._get_guest_config(instance_ref, [], image_meta, disk_info,
None, block_device_info)
self.assertEqual(instance_ref['root_device_name'], '/dev/vda')
def test_get_guest_config_with_root_device_name(self): def test_get_guest_config_with_root_device_name(self):
self.flags(virt_type='uml', group='libvirt') self.flags(virt_type='uml', group='libvirt')
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
@ -19949,6 +19926,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
mock_build_device_metadata, mock_set_host_enabled, mock_build_device_metadata, mock_set_host_enabled,
mock_write_to_file, mock_write_to_file,
mock_get_mdev, mock_get_mdev,
image_meta_dict=None,
exists=None): exists=None):
self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) self.flags(instances_path=self.useFixture(fixtures.TempDir()).path)
mock_build_device_metadata.return_value = None mock_build_device_metadata.return_value = None
@ -19959,8 +19937,10 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
backend = self.useFixture( backend = self.useFixture(
fake_imagebackend.ImageBackendFixture(exists=exists)) fake_imagebackend.ImageBackendFixture(exists=exists))
image_meta = objects.ImageMeta.from_dict( if not image_meta_dict:
{'id': uuids.image_id, 'name': 'fake'}) image_meta_dict = {'id': uuids.image_id, 'name': 'fake'}
image_meta = objects.ImageMeta.from_dict(image_meta_dict)
network_info = _fake_network_info(self, 1) network_info = _fake_network_info(self, 1)
rescue_password = 'fake_password' rescue_password = 'fake_password'
@ -20024,6 +20004,29 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
[uuids.mdev1], [uuids.mdev1],
doc.xpath("devices/*[@type='mdev']/source/address/@uuid")) doc.xpath("devices/*[@type='mdev']/source/address/@uuid"))
def test_rescue_with_different_hw_disk_bus(self):
params = {'config_drive': None, 'root_device_name': '/dev/vda'}
image_meta_dict = {
'id': uuids.image_id,
'name': 'fake',
'properties': {
'hw_disk_bus': 'scsi',
'hw_scsi_model': 'virtio-scsi'
}
}
instance = self._create_instance(params)
backend, doc = self._test_rescue(instance,
image_meta_dict=image_meta_dict)
domain_disk_device_name = doc.xpath('devices/disk/target/@dev')
# Assert that rescued instance will have scsi device name (sd*)
self.assertEqual(['sda', 'sdb'], domain_disk_device_name)
# Assert that instance object preserve virtio root_device_name
# (aarents): Bug #1835926
self.assertEqual('/dev/vda', instance.get('root_device_name'))
@mock.patch('nova.virt.configdrive.ConfigDriveBuilder._make_iso9660') @mock.patch('nova.virt.configdrive.ConfigDriveBuilder._make_iso9660')
def test_rescue_config_drive(self, mock_mkisofs): def test_rescue_config_drive(self, mock_mkisofs):
instance = self._create_instance({'config_drive': str(True)}) instance = self._create_instance({'config_drive': str(True)})

View File

@ -5250,9 +5250,6 @@ class LibvirtDriver(driver.ComputeDriver):
else: else:
root_device_name = None root_device_name = None
if root_device_name:
instance.root_device_name = root_device_name
guest.os_type = (fields.VMMode.get_from_instance(instance) or guest.os_type = (fields.VMMode.get_from_instance(instance) or
self._get_guest_os_type(virt_type)) self._get_guest_os_type(virt_type))
caps = self._host.get_capabilities() caps = self._host.get_capabilities()