FC: Avoid calling `lsscsi` during disconnect

When we disconnect a fibre channel volume, we get the
/dev/sdX path for each device and remove them by executing
the ``tee -a /sys/block/sdX/device/delete`` command.

Currently, in the disconnect workflow, we call ``lsscsi``
command to fetch the HCTL values but never use them.
This execution of ``lsscsi`` for each device can cause
performance issues if we have many devices connected to
the system.

Following is the workflow:
1. Get the volume path symlink in /dev/disk/by-path[1]
/dev/disk/by-path/pci-0000:00:07.0-fc-0x5000d310047d1842-lun-1

2. Translate it to /dev/SdX[2]

3. Call lsscsi to fetch the HCTL values[3]

4. Remove individual devices using the /dev/sdX path[4]

As we can clearly see that step 3 is unnecessary in this workflow.

Also the get_name_from_path method translates the /dev/disk/by-path
symlink to real path /dev/sdX so we don't need to call get_device_info
and it's functionality is redundant here.

Mar 11 20:07:50 ubuntu2204.localdomain cinder-volume[252928]: DEBUG os_brick.initiator.connectors.fibre_channel [None req-9033642e-43ba-4076-a460-63e562b69b3c admin None] Real path /dev/sda for symlink /dev/disk/by-path/pci-0000:00:07.0-f
c-0x5000d310047d1842-lun-1 {{(pid=252928) disconnect_volume /opt/stack/data/venv/lib/python3.10/site-packages/os_brick/initiator/connectors/fibre_channel.py:385}}

[1] 6e83ac6eee/os_brick/initiator/connectors/fibre_channel.py (L381)
[2] 6e83ac6eee/os_brick/initiator/connectors/fibre_channel.py (L384)
[3] 6e83ac6eee/os_brick/initiator/connectors/fibre_channel.py (L393)
[4] https://opendev.org/openstack/os-brick/src/branch/master/os_brick/initiator/connectors/fibre_channel.py#L438

Closes-Bug: #2102053

Change-Id: Idd0577cf31f186a8cd12dd3d4054b0a1a91ec3d4
This commit is contained in:
Rajat Dhasmana
2025-03-03 04:55:41 +05:30
committed by Eric Harney
parent 6e83ac6eee
commit d9392320d7
3 changed files with 37 additions and 9 deletions

View File

@@ -390,7 +390,12 @@ class FibreChannelConnector(base.BaseLinuxConnector):
with exc.context(force, 'Flushing %s failed', mpath_path):
self._linuxscsi.flush_multipath_device(mpath_path)
real_path = typing.cast(str, real_path)
dev_info = self._linuxscsi.get_device_info(real_path)
# real_path is a /dev/sdX path (translated from the
# /dev/disk/by-path symlink) which is sufficient to
# remove the device.
# NOTE: We don't require the HCTL values of the device
# which was fetched in traditional workflow.
dev_info = {'device': real_path}
devices.append(dev_info)
# If flush failed, then remove it forcefully since force=True

View File

@@ -292,6 +292,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
self.connector.connect_volume,
connection_info['data'])
@mock.patch.object(fibre_channel.FibreChannelConnector, 'get_volume_paths')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_name_from_path')
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
@@ -310,6 +312,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
sysfs_multipath_dm_mock,
wait_mpath_device_mock,
find_mp_device_path_mock,
get_name_from_path_mock,
get_volume_paths_mock,
sysfs_dm_name=None):
self.connector.use_multipath = True
get_fc_hbas_mock.side_effect = self.fake_get_fc_hbas
@@ -328,7 +332,12 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
'address': '1:0:0:2',
'host': 1, 'channel': 0,
'id': 0, 'lun': 1}]}
get_device_info_mock.side_effect = devices['devices']
dev_list = [
devices['devices'][0]['device'],
devices['devices'][1]['device'],
]
get_volume_paths_mock.return_value = dev_list
get_name_from_path_mock.side_effect = dev_list
get_scsi_wwn_mock.return_value = wwn
location = '10.0.2.15:3260'
@@ -846,12 +855,14 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas')
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
@mock.patch.object(fibre_channel.FibreChannelConnector, 'get_volume_paths')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_name_from_path')
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
@mock.patch.object(base.BaseLinuxConnector, 'check_valid_device')
def test_disconnect_volume(self, check_valid_device_mock,
find_mp_device_path_mock,
get_device_info_mock,
get_name_from_path_mock,
get_volume_paths_mock,
get_scsi_wwn_mock,
get_fc_hbas_info_mock,
get_fc_hbas_mock,
@@ -877,7 +888,12 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
'address': '1:0:0:2',
'host': 1, 'channel': 0,
'id': 0, 'lun': 1}]}
get_device_info_mock.side_effect = devices['devices']
dev_list = [
devices['devices'][0]['device'],
devices['devices'][1]['device'],
]
get_volume_paths_mock.return_value = dev_list
get_name_from_path_mock.side_effect = dev_list
get_scsi_wwn_mock.return_value = wwn
location = '10.0.2.15:3260'
@@ -912,13 +928,11 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas')
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
@mock.patch.object(base.BaseLinuxConnector, 'check_valid_device')
def test_disconnect_volume_fails(self, ignore_exc, side_effect,
check_valid_device_mock,
find_mp_device_path_mock,
get_device_info_mock,
get_scsi_wwn_mock,
get_fc_hbas_info_mock,
get_fc_hbas_mock,
@@ -950,7 +964,6 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
'address': '1:0:0:2',
'host': 1, 'channel': 0,
'id': 0, 'lun': 1}]}
get_device_info_mock.side_effect = devices['devices']
get_scsi_wwn_mock.return_value = wwn
location = '10.0.2.15:3260'
@@ -985,8 +998,12 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
else:
del_map_mock.assert_not_called()
expected_dev_list = [
{'device': '/dev/dm-1'},
{'device': '/dev/dm-1'},
]
remove_mock.assert_called_once_with(connection_info['data'],
devices['devices'],
expected_dev_list,
devices['devices'][0],
True, mock.ANY)
self.assertEqual(len(expected), realpath_mock.call_count)

View File

@@ -0,0 +1,6 @@
---
fixes:
- |
`Bug #2102053 <https://bugs.launchpad.net/os-brick/+bug/2102053>`_:
Fibre Channel: Improved performance during disconnect by avoiding
execution of ``lsscsi`` command.