From b8c55d1d3c3b9dfd3b962e092e4fd80f2b0dfac3 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Tue, 2 Jul 2019 20:14:21 +0100 Subject: [PATCH] libvirt: Remove unreachable native QEMU iSCSI initiator config code Ieb9a03d308495be4e8c54b5c6c0ff781ea7f0559 introduced support for using QEMU's native iSCSI initiator support way back in Kilo. However this was only enabled when the LibvirtNetVolumeDriver class was configured as the ``iscsi`` volume driver via the ``[libvirt]/volume_drivers`` configurable. Unfortunately this configurable was removed in Liberty by I832820499ec3304132379ad9b9d1ee92c5a75b61 essentially rendering this ``iscsi`` based code path dead ever since unless operators manually hacked the now static ``libvirt_volume_drivers`` list within driver.py. As a result of this and a complete lack of any test coverage in the gate we can now remove this unreachable code from Nova. It might be desirable to reintroduce this support later but this should take the form of an extracted volume driver and a new configurable within nova.conf to switch between the two available drivers. Closes-Bug: #1501447 Change-Id: I1043287fe8063c4b2af07c997a931a7097518ca9 --- .../unit/virt/libvirt/volume/test_net.py | 23 -------- nova/virt/libvirt/volume/net.py | 54 ------------------- 2 files changed, 77 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/volume/test_net.py b/nova/tests/unit/virt/libvirt/volume/test_net.py index b6c70055f524..0332afc3c258 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_net.py +++ b/nova/tests/unit/virt/libvirt/volume/test_net.py @@ -14,7 +14,6 @@ import mock import nova.conf from nova.tests.unit.virt.libvirt.volume import test_volume -from nova.virt.libvirt import host from nova.virt.libvirt.volume import net CONF = nova.conf.CONF @@ -220,28 +219,6 @@ class LibvirtNetVolumeDriverTestCase( libvirt_driver.disconnect_volume(connection_info, mock.sentinel.instance) - @mock.patch.object(host.Host, 'find_secret') - @mock.patch.object(host.Host, 'create_secret') - @mock.patch.object(host.Host, 'delete_secret') - def test_libvirt_iscsi_net_driver(self, mock_delete, mock_create, - mock_find): - mock_find.return_value = test_volume.FakeSecret() - mock_create.return_value = test_volume.FakeSecret() - libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_host) - connection_info = self.iscsi_connection(self.vol, self.location, - self.iqn, auth=True) - secret_type = 'iscsi' - flags_user = connection_info['data']['auth_username'] - conf = libvirt_driver.get_config(connection_info, self.disk_info) - tree = conf.format_dom() - self._assertISCSINetworkAndProtocolEquals(tree) - self.assertEqual(flags_user, tree.find('./auth').get('username')) - self.assertEqual(secret_type, tree.find('./auth/secret').get('type')) - self.assertEqual(test_volume.SECRET_UUID, - tree.find('./auth/secret').get('uuid')) - libvirt_driver.disconnect_volume(connection_info, - mock.sentinel.instance) - def test_extend_volume(self): device_path = '/dev/fake-dev' connection_info = {'data': {'device_path': device_path}} diff --git a/nova/virt/libvirt/volume/net.py b/nova/virt/libvirt/volume/net.py index 15cbe20f3689..1164395a4bcc 100644 --- a/nova/virt/libvirt/volume/net.py +++ b/nova/virt/libvirt/volume/net.py @@ -13,9 +13,6 @@ from oslo_log import log as logging import nova.conf -from nova import exception -from nova.i18n import _ -from nova import utils from nova.virt.libvirt.volume import volume as libvirt_volume @@ -29,30 +26,6 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver): super(LibvirtNetVolumeDriver, self).__init__(host, is_block_dev=False) - def _get_secret_uuid(self, conf, password=None): - # TODO(mriedem): Add delegation methods to connection (LibvirtDriver) - # to call through for these secret CRUD operations so the volume driver - # doesn't need to know the internal attributes of the connection - # object. - secret = self.host.find_secret(conf.source_protocol, - conf.source_name) - if secret is None: - secret = self.host.create_secret(conf.source_protocol, - conf.source_name, - password) - return secret.UUIDString() - - def _delete_secret_by_name(self, connection_info): - source_protocol = connection_info['driver_volume_type'] - netdisk_properties = connection_info['data'] - if source_protocol == 'rbd': - return - elif source_protocol == 'iscsi': - usage_type = 'iscsi' - usage_name = ("%(target_iqn)s/%(target_lun)s" % - netdisk_properties) - self.host.delete_secret(usage_type, usage_name) - def _set_auth_config_rbd(self, conf, netdisk_properties): # The rbd volume driver in cinder sets auth_enabled if the rbd_user is # set in cinder. The rbd auth values from the cinder connection take @@ -94,13 +67,6 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver): # secret_type is always hard-coded to 'ceph' in cinder conf.auth_secret_type = netdisk_properties['secret_type'] - def _set_auth_config_iscsi(self, conf, netdisk_properties): - if netdisk_properties.get('auth_method') == 'CHAP': - conf.auth_secret_type = 'iscsi' - password = netdisk_properties.get('auth_password') - conf.auth_secret_uuid = self._get_secret_uuid(conf, password) - conf.auth_username = netdisk_properties['auth_username'] - def get_config(self, connection_info, disk_info): """Returns xml for libvirt.""" conf = super(LibvirtNetVolumeDriver, @@ -114,28 +80,8 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver): conf.source_ports = netdisk_properties.get('ports', []) if conf.source_protocol == 'rbd': self._set_auth_config_rbd(conf, netdisk_properties) - elif conf.source_protocol == 'iscsi': - try: - conf.source_name = ("%(target_iqn)s/%(target_lun)s" % - netdisk_properties) - target_portal = netdisk_properties['target_portal'] - except KeyError: - raise exception.InternalError(_("Invalid volume source data")) - - ip, port = utils.parse_server_string(target_portal) - if ip == '' or port == '': - raise exception.InternalError(_("Invalid target_lun")) - conf.source_hosts = [ip] - conf.source_ports = [port] - self._set_auth_config_iscsi(conf, netdisk_properties) return conf - def disconnect_volume(self, connection_info, instance): - """Detach the volume from instance_name.""" - super(LibvirtNetVolumeDriver, - self).disconnect_volume(connection_info, instance) - self._delete_secret_by_name(connection_info) - def extend_volume(self, connection_info, instance, requested_size): # There is nothing to do for network volumes. Cinder already extended # the volume and there is no local block device which needs to be