openstack
/
os-brick
Code Issues Proposed changes

Fix NFS "already mounted" detection 

Previous fix

2f32c98a RemoteFS: don't fail in do_mount if already mounted

was made with the assumption that "already mounted" appearing
in stderr from an NFS mount means that the NFS share was already
mounted.

However, the NFS client can fail with "busy or already mounted"
in cases where it fails to mount the share as well.

This results in the Cinder NFS backup driver believing that the
NFS mount has succeeded when it didn't, and as a result, data
can be written to the local disk instead of the NFS target.

Fail if the share isn't actually mounted rather than succeeding.

Related-Bug: #1780813
Closes-Bug: #1797233
Change-Id: Iebd1afb3340fcaeb1969784966c4f9be35a28417
(cherry picked from commit 360a20c7c7)
(cherry picked from commit 4c94477cb5)
(cherry picked from commit 30d7bc124e)
This commit is contained in:
Eric Harney 2018-10-10 16:46:01 -04:00 committed by Jay S. Bryant
parent e571cc603e
commit f64ad529bf
2 changed files with 31 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
os_brick/remotefs/remotefs.py
View File

@ -123,10 +123,15 @@ class RemoteFsClient(executor.Executor):
except processutils.ProcessExecutionError as exc:
if 'already mounted' in exc.stderr:
LOG.info("Already mounted: %s", share)
else:
LOG.error("Failed to mount %(share)s, reason: %(reason)s",
{'share': share, 'reason': exc.stderr})
raise
# The error message can say "busy or already mounted" when the
# share didn't actually mount, so look for it.
if share in self._read_mounts():
return
LOG.error("Failed to mount %(share)s, reason: %(reason)s",
{'share': share, 'reason': exc.stderr})
raise
def _mount_nfs(self, nfs_share, mount_path, flags=None):
"""Mount nfs share using present mount types."""

22
os_brick/tests/remotefs/test_remotefs.py
View File

@ -82,6 +82,28 @@ class RemoteFsClientTestCase(base.TestCase):
self.assertEqual(mock_do_mount.call_count, 0)
self.assertEqual(mock_execute.call_count, 0)
@mock.patch.object(priv_rootwrap, 'execute')
def test_mount_race(self, mock_execute):
err_msg = 'mount.nfs: /var/asdf is already mounted'
mock_execute.side_effect = putils.ProcessExecutionError(stderr=err_msg)
mounts = {'192.0.2.20:/share': '/var/asdf/'}
client = remotefs.RemoteFsClient("nfs", root_helper='true',
nfs_mount_point_base='/var/asdf')
with mock.patch.object(client, '_read_mounts',
return_value=mounts):
client._do_mount('nfs', '192.0.2.20:/share', '/var/asdf')
@mock.patch.object(priv_rootwrap, 'execute')
def test_mount_failure(self, mock_execute):
err_msg = 'mount.nfs: nfs broke'
mock_execute.side_effect = putils.ProcessExecutionError(stderr=err_msg)
client = remotefs.RemoteFsClient("nfs", root_helper='true',
nfs_mount_point_base='/var/asdf')
self.assertRaises(putils.ProcessExecutionError,
client._do_mount,
'nfs', '192.0.2.20:/share', '/var/asdf')
def _test_no_mount_point(self, fs_type):
self.assertRaises(exception.InvalidParameterValue,
remotefs.RemoteFsClient,