From b11e3f1d0da238543cb8a3b6c3e00c6b98e99910 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 11 Aug 2021 18:36:27 +0100 Subject: [PATCH] fup: Remove unused legacy block_device_info format As announced on the ML [1] this change removes the now unused legacy format from the codebase and updates the reference docs. [1] http://lists.openstack.org/pipermail/openstack-discuss/2021-August/024116.html Change-Id: I3895b61b436b9bb882477d2d1b3f4907f03b3b1c --- doc/source/reference/block-device-structs.rst | 27 +++++---- nova/block_device.py | 24 -------- nova/compute/manager.py | 22 ------- nova/tests/unit/compute/test_compute.py | 1 - nova/tests/unit/test_block_device.py | 39 ------------ nova/tests/unit/virt/libvirt/test_driver.py | 4 -- nova/tests/unit/virt/test_block_device.py | 59 ------------------- .../unit/virt/vmwareapi/test_driver_api.py | 3 - nova/virt/block_device.py | 53 ++++------------- nova/virt/driver.py | 10 ---- nova/virt/fake.py | 4 -- nova/virt/hyperv/driver.py | 4 -- nova/virt/ironic/driver.py | 4 -- nova/virt/libvirt/driver.py | 4 -- nova/virt/vmwareapi/driver.py | 4 -- 15 files changed, 28 insertions(+), 234 deletions(-) diff --git a/doc/source/reference/block-device-structs.rst b/doc/source/reference/block-device-structs.rst index 77781612144..1b8636c5378 100644 --- a/doc/source/reference/block-device-structs.rst +++ b/doc/source/reference/block-device-structs.rst @@ -59,6 +59,10 @@ or volumes, and some associated data. ``block_device_info`` --------------------- +.. versionchanged:: 24.0.0 (Xena) + + The legacy block_device_info format is no longer supported. + Drivers do not directly use BDM objects. Instead, they are transformed into a different driver-specific representation. This representation is normally called ``block_device_info``, and is generated by @@ -74,24 +78,23 @@ called ``block_device_info``, and is generated by ``swap`` A swap disk, or None if there is no swap disk -The disks are represented in one of two ways, depending on the specific -driver currently in use. There's the 'new' representation, used by the libvirt -and vmwareAPI drivers, and the 'legacy' representation used by all other -drivers. The legacy representation is a plain dict. It does not contain the -same information as the new representation. +.. note:: + + The disks were previously represented in one of two ways, depending on the + specific driver in use. A legacy plain dict format or the currently used + DriverBlockDevice format discussed below. Support for the legacy format + was removed in Xena. -The new representation involves subclasses of -``nova.block_device.DriverBlockDevice``. As well as containing different -fields, the new representation significantly also retains a reference to the -underlying BDM object. This means that by manipulating the -``DriverBlockDevice`` object, the driver is able to persist data to the BDM -object in the DB. +Disks are represented by subclasses of ``nova.block_device.DriverBlockDevice``. +These subclasses retain a reference to the underlying BDM object. This means +that by manipulating the ``DriverBlockDevice`` object, the driver is able to +persist data to the BDM object in the DB. .. note:: Common usage is to pull ``block_device_mapping`` out of this dict into a variable called ``block_device_mapping``. This is not a - ``BlockDeviceMapping`` object, or list of them. + ``BlockDeviceMapping`` object, or a list of them. .. note:: diff --git a/nova/block_device.py b/nova/block_device.py index 340d4b9285a..a026078317d 100644 --- a/nova/block_device.py +++ b/nova/block_device.py @@ -311,30 +311,6 @@ def snapshot_from_bdm(snapshot_id, template): return BlockDeviceDict(snapshot_dict) -def legacy_mapping(block_device_mapping): - """Transform a list of block devices of an instance back to the - legacy data format. - """ - - legacy_block_device_mapping = [] - - for bdm in block_device_mapping: - try: - legacy_block_device = BlockDeviceDict(bdm).legacy() - except exception.InvalidBDMForLegacy: - continue - - legacy_block_device_mapping.append(legacy_block_device) - - # Re-enumerate the ephemeral devices - for i, dev in enumerate(dev for dev in legacy_block_device_mapping - if dev['virtual_name'] and - is_ephemeral(dev['virtual_name'])): - dev['virtual_name'] = dev['virtual_name'][:-1] + str(i) - - return legacy_block_device_mapping - - def from_legacy_mapping(legacy_block_device_mapping, image_uuid='', root_device_name=None, no_root=False): """Transform a legacy list of block devices to the new data format.""" diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6c5d6274a05..4bea89bc828 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -578,8 +578,6 @@ def __init__(self, compute_driver=None, *args, **kwargs): # compute manager via the virtapi, so we want it to be fully # initialized before that happens. self.driver = driver.load_compute_driver(self.virtapi, compute_driver) - self.use_legacy_block_device_info = \ - self.driver.need_legacy_block_device_info self.rt = resource_tracker.ResourceTracker( self.host, self.driver, reportclient=self.reportclient) @@ -1939,23 +1937,6 @@ def _default_block_device_names(self, instance, image_meta, block_devices): swap, block_device_mapping) - def _block_device_info_to_legacy(self, block_device_info): - """Convert BDI to the old format for drivers that need it.""" - - if self.use_legacy_block_device_info: - ephemerals = driver_block_device.legacy_block_devices( - driver.block_device_info_get_ephemerals(block_device_info)) - mapping = driver_block_device.legacy_block_devices( - driver.block_device_info_get_mapping(block_device_info)) - swap = block_device_info['swap'] - if swap: - swap = swap.legacy() - - block_device_info.update({ - 'ephemerals': ephemerals, - 'swap': swap, - 'block_device_mapping': mapping}) - def _add_missing_dev_names(self, bdms, instance): for bdm in bdms: if bdm.device_name is not None: @@ -1977,7 +1958,6 @@ def _prep_block_device(self, context, instance, bdms): mapping, context, instance, self.volume_api, self.driver, wait_func=self._await_block_device_map_created) - self._block_device_info_to_legacy(block_device_info) return block_device_info except exception.OverQuota as e: @@ -2093,8 +2073,6 @@ def _get_instance_block_device_info(self, context, instance, driver.block_device_info_get_mapping(block_device_info), context, instance, self.volume_api, self.driver) - self._block_device_info_to_legacy(block_device_info) - return block_device_info def _build_failed(self, node): diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 943712085df..cbb699ec4a8 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -1424,7 +1424,6 @@ def fake_attach_block_devices(bdm, *args, **kwargs): } manager = compute_manager.ComputeManager() - manager.use_legacy_block_device_info = False mock_bdm_saves = [mock.patch.object(bdm, 'save') for bdm in bdms] with test.nested(*mock_bdm_saves): block_device_info = manager._prep_block_device(self.context, diff --git a/nova/tests/unit/test_block_device.py b/nova/tests/unit/test_block_device.py index 2e9d768d58d..f5a4fc56947 100644 --- a/nova/tests/unit/test_block_device.py +++ b/nova/tests/unit/test_block_device.py @@ -656,45 +656,6 @@ def test_from_api_invalid_specify_volume_type_with_source_volume_mapping( self.assertIn('Specifying volume type to existing volume is ' 'not supported', str(ex)) - def test_legacy(self): - for legacy, new in zip(self.legacy_mapping, self.new_mapping): - self.assertThat( - legacy, - matchers.IsSubDictOf(new.legacy())) - - def test_legacy_mapping(self): - got_legacy = block_device.legacy_mapping(self.new_mapping) - - for legacy, expected in zip(got_legacy, self.legacy_mapping): - self.assertThat(expected, matchers.IsSubDictOf(legacy)) - - def test_legacy_source_image(self): - for legacy, new in zip(self.legacy_mapping_source_image, - self.new_mapping_source_image): - if new['destination_type'] == 'volume': - self.assertThat(legacy, matchers.IsSubDictOf(new.legacy())) - else: - self.assertRaises(exception.InvalidBDMForLegacy, new.legacy) - - def test_legacy_mapping_source_image(self): - got_legacy = block_device.legacy_mapping(self.new_mapping) - - for legacy, expected in zip(got_legacy, self.legacy_mapping): - self.assertThat(expected, matchers.IsSubDictOf(legacy)) - - def test_legacy_mapping_from_object_list(self): - bdm1 = objects.BlockDeviceMapping() - bdm1 = objects.BlockDeviceMapping._from_db_object( - None, bdm1, fake_block_device.FakeDbBlockDeviceDict( - self.new_mapping[0])) - bdm2 = objects.BlockDeviceMapping() - bdm2 = objects.BlockDeviceMapping._from_db_object( - None, bdm2, fake_block_device.FakeDbBlockDeviceDict( - self.new_mapping[1])) - bdmlist = objects.BlockDeviceMappingList() - bdmlist.objects = [bdm1, bdm2] - block_device.legacy_mapping(bdmlist) - def test_image_mapping(self): removed_fields = ['id', 'instance_uuid', 'connection_info', 'created_at', 'updated_at', 'deleted_at', 'deleted'] diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 03eea0555fd..a30c5d55c0c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -938,10 +938,6 @@ def test_public_api_signatures(self): inst = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) self.assertPublicAPISignatures(baseinst, inst) - def test_legacy_block_device_info(self): - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) - self.assertFalse(drvr.need_legacy_block_device_info) - @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_cpu_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_storage_bus_traits') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_video_model_traits') diff --git a/nova/tests/unit/virt/test_block_device.py b/nova/tests/unit/virt/test_block_device.py index bed9052fa1e..9fc97c0ff85 100644 --- a/nova/tests/unit/virt/test_block_device.py +++ b/nova/tests/unit/virt/test_block_device.py @@ -64,10 +64,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'swap_size': 2, 'disk_bus': 'scsi'} - swap_legacy_driver_bdm = { - 'device_name': '/dev/sdb1', - 'swap_size': 2} - ephemeral_bdm_dict = block_device.BlockDeviceDict( {'id': 2, 'instance_uuid': uuids.instance, 'device_name': '/dev/sdc1', @@ -87,12 +83,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'guest_format': 'ext4', 'disk_bus': 'scsi'} - ephemeral_legacy_driver_bdm = { - 'device_name': '/dev/sdc1', - 'size': 4, - 'virtual_name': 'ephemeral0', - 'num': 0} - volume_bdm_dict = block_device.BlockDeviceDict( {'id': 3, 'instance_uuid': uuids.instance, 'device_name': '/dev/sda1', @@ -117,11 +107,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'boot_index': 0, 'volume_type': None} - volume_legacy_driver_bdm = { - 'mount_device': '/dev/sda1', - 'connection_info': {"fake": "connection_info"}, - 'delete_on_termination': False} - volume_bdm_dict_without_conn_info = block_device.BlockDeviceDict( {'id': 3, 'instance_uuid': uuids.instance, 'device_name': '/dev/sda1', @@ -172,11 +157,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'boot_index': -1, 'volume_type': None} - volsnapshot_legacy_driver_bdm = { - 'mount_device': '/dev/sda2', - 'connection_info': {"fake": "connection_info"}, - 'delete_on_termination': True} - volimage_bdm_dict = block_device.BlockDeviceDict( {'id': 5, 'instance_uuid': uuids.instance, 'device_name': '/dev/sda2', @@ -202,11 +182,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'boot_index': -1, 'volume_type': None} - volimage_legacy_driver_bdm = { - 'mount_device': '/dev/sda2', - 'connection_info': {"fake": "connection_info"}, - 'delete_on_termination': True} - volblank_bdm_dict = block_device.BlockDeviceDict( {'id': 6, 'instance_uuid': uuids.instance, 'device_name': '/dev/sda2', @@ -231,11 +206,6 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'boot_index': -1, 'volume_type': None} - volblank_legacy_driver_bdm = { - 'mount_device': '/dev/sda2', - 'connection_info': {"fake": "connection_info"}, - 'delete_on_termination': True} - def setUp(self): super(TestDriverBlockDevice, self).setUp() self.volume_api = mock.MagicMock(autospec=cinder.API) @@ -339,9 +309,6 @@ def _test_driver_device(self, name): # Reset the value test_bdm[field] = value - expected = getattr(self, "%s_legacy_driver_bdm" % name) - self.assertThat(expected, matchers.DictMatches(test_bdm.legacy())) - # Test passthru attributes for passthru in test_bdm._proxy_as_attr: self.assertEqual(getattr(test_bdm, passthru), @@ -1289,37 +1256,11 @@ def test_convert_volume_without_connection_info(self): driver_block_device.convert_volume( self.volume_bdm_without_conn_info)) - def test_legacy_block_devices(self): - test_snapshot = self.driver_classes['volsnapshot']( - self.volsnapshot_bdm) - - block_device_mapping = [test_snapshot, test_snapshot] - legacy_bdm = driver_block_device.legacy_block_devices( - block_device_mapping) - self.assertEqual(legacy_bdm, [self.volsnapshot_legacy_driver_bdm, - self.volsnapshot_legacy_driver_bdm]) - - # Test that the ephemerals work as expected - test_ephemerals = [self.driver_classes['ephemeral']( - self.ephemeral_bdm) for _ in range(2)] - expected = [self.ephemeral_legacy_driver_bdm.copy() - for _ in range(2)] - expected[0]['virtual_name'] = 'ephemeral0' - expected[0]['num'] = 0 - expected[1]['virtual_name'] = 'ephemeral1' - expected[1]['num'] = 1 - legacy_ephemerals = driver_block_device.legacy_block_devices( - test_ephemerals) - self.assertEqual(expected, legacy_ephemerals) - def test_get_swap(self): swap = [self.swap_driver_bdm] - legacy_swap = [self.swap_legacy_driver_bdm] no_swap = [self.volume_driver_bdm] self.assertEqual(swap[0], driver_block_device.get_swap(swap)) - self.assertEqual(legacy_swap[0], - driver_block_device.get_swap(legacy_swap)) self.assertIsNone(driver_block_device.get_swap(no_swap)) self.assertIsNone(driver_block_device.get_swap([])) diff --git a/nova/tests/unit/virt/vmwareapi/test_driver_api.py b/nova/tests/unit/virt/vmwareapi/test_driver_api.py index 4086a7d7cc7..8d3589c7bba 100644 --- a/nova/tests/unit/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/unit/virt/vmwareapi/test_driver_api.py @@ -266,9 +266,6 @@ def tearDown(self): super(VMwareAPIVMTestCase, self).tearDown() vmwareapi_fake.cleanup() - def test_legacy_block_device_info(self): - self.assertFalse(self.conn.need_legacy_block_device_info) - def test_get_host_ip_addr(self): self.assertEqual(HOST, self.conn.get_host_ip_addr()) diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py index 7dcc5ff1614..8adffdaf9a8 100644 --- a/nova/virt/block_device.py +++ b/nova/virt/block_device.py @@ -83,9 +83,8 @@ class DriverBlockDevice(dict): Uses block device objects internally to do the database access. - _fields and _legacy_fields class attributes present a set of fields that - are expected on a certain DriverBlockDevice type. We may have more legacy - versions in the future. + The _fields class attribute present a set of fields that + are expected on a certain DriverBlockDevice type. If an attribute access is attempted for a name that is found in the _proxy_as_attr set, it will be proxied to the underlying object. This @@ -103,7 +102,6 @@ class DriverBlockDevice(dict): """ _fields = set() - _legacy_fields = set() _proxy_as_attr_inherited = set(['uuid', 'is_volume']) _update_on_save = {'disk_bus': None, @@ -179,14 +177,6 @@ def get(self, name, default=None): else: return super(DriverBlockDevice, self).get(name, default) - def legacy(self): - """Basic legacy transformation. - - Basic method will just drop the fields that are not in - _legacy_fields set. Override this in subclass if needed. - """ - return {key: self.get(key) for key in self._legacy_fields} - def attach(self, **kwargs): """Make the device available to be used by VMs. @@ -223,7 +213,6 @@ def save(self): class DriverSwapBlockDevice(DriverBlockDevice): _fields = set(['device_name', 'swap_size', 'disk_bus']) - _legacy_fields = _fields - set(['disk_bus']) _update_on_save = {'disk_bus': None, 'device_name': None} @@ -241,8 +230,6 @@ def _transform(self): class DriverEphemeralBlockDevice(DriverBlockDevice): _new_only_fields = set(['disk_bus', 'device_type', 'guest_format']) _fields = set(['device_name', 'size']) | _new_only_fields - _legacy_fields = (_fields - _new_only_fields | - set(['num', 'virtual_name'])) def _transform(self): if not block_device.new_format_is_ephemeral(self._bdm_obj): @@ -255,20 +242,19 @@ def _transform(self): 'guest_format': self._bdm_obj.guest_format }) - def legacy(self, num=0): - legacy_bdm = super(DriverEphemeralBlockDevice, self).legacy() - legacy_bdm['num'] = num - legacy_bdm['virtual_name'] = 'ephemeral' + str(num) - return legacy_bdm - class DriverVolumeBlockDevice(DriverBlockDevice): - _legacy_fields = set(['connection_info', 'mount_device', - 'delete_on_termination']) - _new_fields = set(['guest_format', 'device_type', - 'disk_bus', 'boot_index', - 'attachment_id']) - _fields = _legacy_fields | _new_fields + _new_fields = set([ + 'guest_format', + 'device_type', + 'disk_bus', + 'boot_index', + 'attachment_id']) + _fields = set([ + 'connection_info', + 'mount_device', + 'delete_on_termination' + ]) | _new_fields _valid_source = 'volume' _valid_destination = 'volume' @@ -884,19 +870,6 @@ def refresh_conn_infos(block_device_mapping, *refresh_args, **refresh_kwargs): return block_device_mapping -def legacy_block_devices(block_device_mapping): - bdms = [bdm.legacy() for bdm in block_device_mapping] - - # Re-enumerate ephemeral devices - if all(isinstance(bdm, DriverEphemeralBlockDevice) - for bdm in block_device_mapping): - for i, dev in enumerate(bdms): - dev['virtual_name'] = dev['virtual_name'][:-1] + str(i) - dev['num'] = i - - return bdms - - def get_swap(transformed_list): """Get the swap device out of the list context. diff --git a/nova/virt/driver.py b/nova/virt/driver.py index df3b1e5dda7..f5f622d72d9 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -1617,16 +1617,6 @@ def delete_instance_files(self, instance): """ return True - # TODO(lyarwood): This is no longer used and should be removed. - @property - def need_legacy_block_device_info(self): - """Tell the caller if the driver requires legacy block device info. - - Tell the caller whether we expect the legacy format of block - device info to be passed in to methods that expect it. - """ - return True - def volume_snapshot_create(self, context, instance, volume_id, create_info): """Snapshots volumes attached to a specified instance. diff --git a/nova/virt/fake.py b/nova/virt/fake.py index d94e339cd88..008aa944867 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -654,10 +654,6 @@ def quiesce(self, context, instance, image_meta): def unquiesce(self, context, instance, image_meta): pass - @property - def need_legacy_block_device_info(self): - return False - class FakeVirtAPI(virtapi.VirtAPI): @contextlib.contextmanager diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py index 16cb9db8e90..350e59e295c 100644 --- a/nova/virt/hyperv/driver.py +++ b/nova/virt/hyperv/driver.py @@ -144,10 +144,6 @@ def _check_minimum_windows_version(self): 'has been deprecated In Queens, and will be removed ' 'in Rocky.') - @property - def need_legacy_block_device_info(self): - return False - def init_host(self, host): self._serialconsoleops.start_console_handlers() event_handler = eventhandler.InstanceEventHandler( diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index aabc05776ed..5c34e5f2c59 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -1893,10 +1893,6 @@ def get_serial_console(self, context, instance): instance=instance) raise exception.ConsoleTypeUnavailable(console_type='serial') - @property - def need_legacy_block_device_info(self): - return False - def prepare_networks_before_block_device_mapping(self, instance, network_info): """Prepare networks before the block devices are mapped to instance. diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index cb7db5901f8..de410b4e42d 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -11537,10 +11537,6 @@ def delete_instance_files(self, instance): LOG.info('Deletion of %s complete', target_del, instance=instance) return True - @property - def need_legacy_block_device_info(self): - return False - def default_root_device_name(self, instance, image_meta, root_bdm): disk_bus = blockinfo.get_disk_bus_for_device_type( instance, CONF.libvirt.virt_type, image_meta, "disk") diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py index 520d710cba5..4eeae8a3501 100644 --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -172,10 +172,6 @@ def _check_min_version(self): '%(version)s in the 16.0.0 release.', {'version': constants.NEXT_MIN_VC_VERSION}) - @property - def need_legacy_block_device_info(self): - return False - def _update_pbm_location(self): if CONF.vmware.pbm_wsdl_location: pbm_wsdl_loc = CONF.vmware.pbm_wsdl_location