openstack
/
os-brick
Code Issues Proposed changes

Merge "Use file locks in connectors"

This commit is contained in:
Zuul 2021-11-12 05:29:52 +00:00 committed by Gerrit Code Review
parent 9c5d5e5cbd 6a43669edc
commit ca721994a2
9 changed files with 40 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
os_brick/initiator/connectors/fibre_channel.py
View File

@ -168,7 +168,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
return volume_paths
@utils.trace
@synchronized('extend_volume')
@synchronized('extend_volume', external=True)
def extend_volume(self, connection_properties):
"""Update the local kernel's size information.
@ -189,7 +189,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
raise exception.VolumePathsNotFound()
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def connect_volume(self, connection_properties):
"""Attach the volume to instance_name.
@ -314,7 +314,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
return raw_devices
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def disconnect_volume(self, connection_properties, device_info,
force=False, ignore_errors=False):
"""Detach the volume from instance_name.

4
os_brick/initiator/connectors/huawei.py
View File

@ -85,7 +85,7 @@ class HuaweiStorHyperConnector(base.BaseLinuxConnector):
return out['dev_addr']
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def connect_volume(self, connection_properties):
"""Connect to a volume.
@ -116,7 +116,7 @@ class HuaweiStorHyperConnector(base.BaseLinuxConnector):
return device_info
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def disconnect_volume(self, connection_properties, device_info,
force=False, ignore_errors=False):
"""Disconnect a volume from the local host.

17
os_brick/initiator/connectors/iscsi.py
View File

@ -474,7 +474,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
root_helper=self._root_helper)
@utils.trace
@synchronized('extend_volume')
@synchronized('extend_volume', external=True)
def extend_volume(self, connection_properties: dict):
"""Update the local kernel's size information.
@ -497,18 +497,10 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
raise exception.VolumePathsNotFound()
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def connect_volume(self, connection_properties: dict):
"""Attach the volume to instance_name.
NOTE: Will retry up to three times to handle the case where c-vol
and n-cpu are both using os-brick to manage iSCSI sessions but they
are on the same node and using different locking directories. In this
case, even though this call is synchronized, they will be separate
locks and can still overlap with connect and disconnect. Since a
disconnect during an initial attach can't cause IO failure (the device
has not been made available yet), we just try the connection again.
:param connection_properties: The valid dictionary that describes all
of the target volume attributes.
:type connection_properties: dict
@ -870,7 +862,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
return device_map
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def disconnect_volume(self, connection_properties, device_info,
force=False, ignore_errors=False):
"""Detach the volume from instance_name.
@ -1056,7 +1048,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
target_iqn = connection_properties['target_iqn']
lock_name = f'connect_to_iscsi_portal-{portal}-{target_iqn}'
method = synchronized(lock_name)(self._connect_to_iscsi_portal_unsafe)
method = synchronized(
lock_name, external=True)(self._connect_to_iscsi_portal_unsafe)
return method(connection_properties)
@utils.retry((exception.BrickException))

6
os_brick/initiator/connectors/nvmeof.py
View File

@ -317,7 +317,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
return self._is_nvme_available(nvme_name)
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def connect_volume(self, connection_properties):
"""Discover and attach the volume.
@ -368,7 +368,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
return device_info
@utils.trace
@synchronized('connect_volume')
@synchronized('connect_volume', external=True)
def disconnect_volume(self, connection_properties, device_info,
force=False, ignore_errors=False):
"""Flush the volume.
@ -412,7 +412,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
raise
@utils.trace
@synchronized('extend_volume')
@synchronized('extend_volume', external=True)
def extend_volume(self, connection_properties):
"""Update the local kernel's size information.

4
os_brick/initiator/connectors/scaleio.py
View File

@ -336,7 +336,7 @@ class ScaleIOConnector(base.BaseLinuxConnector):
return device_info
@utils.trace
@lockutils.synchronized('scaleio', 'scaleio-')
@lockutils.synchronized('scaleio', 'scaleio-', external=True)
def connect_volume(self, connection_properties):
"""Connect the volume.
@ -450,7 +450,7 @@ class ScaleIOConnector(base.BaseLinuxConnector):
return device_info
@utils.trace
@lockutils.synchronized('scaleio', 'scaleio-')
@lockutils.synchronized('scaleio', 'scaleio-', external=True)
def disconnect_volume(self, connection_properties, device_info,
force=False, ignore_errors=False):
"""Disconnect the ScaleIO volume.

3
os_brick/initiator/utils.py
View File

@ -42,5 +42,6 @@ def guard_connection(device):
else:
# Cinder passes an OVO, but Nova passes a dictionary, so we use dict
# key access that works with both.
with lockutils.lock(device['service_uuid'], 'os-brick-'):
with lockutils.lock(device['service_uuid'], 'os-brick-',
external=True):
yield

9
os_brick/tests/base.py
View File

@ -18,6 +18,8 @@ import os
from unittest import mock
import fixtures
from oslo_concurrency import lockutils
from oslo_config import fixture as config_fixture
from oslo_utils import strutils
import testtools
@ -65,6 +67,13 @@ class TestCase(testtools.TestCase):
patcher.start()
self.addCleanup(patcher.stop)
# At runtime this would be set by the library user: Cinder, Nova, etc.
self.useFixture(fixtures.NestedTempfile())
lock_path = self.useFixture(fixtures.TempDir()).path
self.fixture = self.useFixture(config_fixture.Config(lockutils.CONF))
self.fixture.config(lock_path=lock_path, group='oslo_concurrency')
lockutils.set_defaults(lock_path)
def _common_cleanup(self):
"""Runs after each test method to tear down test environment."""

3
os_brick/tests/initiator/test_utils.py
View File

@ -59,4 +59,5 @@ class InitiatorUtilsTestCase(base.TestCase):
utils.ISCSI_SUPPORTS_MANUAL_SCAN = False
with utils.guard_connection({'service_uuid': mock.sentinel.uuid,
'shared_targets': True}):
mock_lock.assert_called_once_with(mock.sentinel.uuid, 'os-brick-')
mock_lock.assert_called_once_with(mock.sentinel.uuid, 'os-brick-',
external=True)

12
releasenotes/notes/external-locks-9f015988ebdc37d6.yaml Normal file
View File

@ -0,0 +1,12 @@
---
upgrade:
- |
Services using os-brick need to set the ``lock_path`` configuration option
in their ``[oslo_concurrency]`` section since it doesn't have a valid
default (related `bug #1947370
<https://bugs.launchpad.net/cinder/+bug/1947370>`_).
fixes:
- |
`Bug #1947370 <https://bugs.launchpad.net/cinder/+bug/1947370>`_: Fixed
race conditions on iSCSI with shared targets and NVMe ``connect_volume``
and ``disconnect_volume`` calls.