From 360a20c7c7b4c385255200a574edc0ca1ac8d87c Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Wed, 10 Oct 2018 16:46:01 -0400 Subject: [PATCH] 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 --- os_brick/remotefs/remotefs.py | 13 +++++++++---- os_brick/tests/remotefs/test_remotefs.py | 6 +++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/os_brick/remotefs/remotefs.py b/os_brick/remotefs/remotefs.py index 1c1d41943..0fa6538d9 100644 --- a/os_brick/remotefs/remotefs.py +++ b/os_brick/remotefs/remotefs.py @@ -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.""" diff --git a/os_brick/tests/remotefs/test_remotefs.py b/os_brick/tests/remotefs/test_remotefs.py index 92ba0f7f5..27762153f 100644 --- a/os_brick/tests/remotefs/test_remotefs.py +++ b/os_brick/tests/remotefs/test_remotefs.py @@ -86,9 +86,13 @@ class RemoteFsClientTestCase(base.TestCase): 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') - client._do_mount('nfs', '192.0.2.20:/share', '/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):