From de89635ab70b36852d638efd6e45d245217613cc Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Tue, 26 Sep 2017 21:01:14 +0200
Subject: [PATCH] Protect against race within os.path.realpath

On iSCSI connections, when we are making sure that there no links remain
like /dev/disk/by-id/scsi-* we make several calls to os.path.realpath
which normal behavior is:

- If file path doesn't exist, returns the same file path
- If file path exists, return the real path

But there is a third option, and that is when the file did exist but it
dissapear right when the  call to os.readlink in posixpath:_joinrealpath

   path, ok = _joinrealpath(path, os.readlink(newpath), seen)

Which ends up raising an exception such as:

   OSError: [Errno 2] No such file or directory:
            '/dev/disk/by-id/scsi-20024f40058540081'

And because of this exception the detach will fail when it shouldn't.

This patch adds includes the call to os.path.realpath within a
try...except clause to prevent this race condition from unexpectely
making the detach operation fail.

Change-Id: Ieb58826b28c62094c941fce10863c0a75fb4e8aa
Closes-Bug: #1719719
---
 os_brick/initiator/linuxscsi.py            | 13 +++++++++++--
 os_brick/tests/initiator/test_linuxscsi.py | 12 ++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py
index 80c4e0192..9c631dc64 100644
--- a/os_brick/initiator/linuxscsi.py
+++ b/os_brick/initiator/linuxscsi.py
@@ -235,8 +235,17 @@ class LinuxSCSI(executor.Executor):
     def _remove_scsi_symlinks(self, devices_names):
         devices = ['/dev/' + dev for dev in devices_names]
         links = glob.glob('/dev/disk/by-id/scsi-*')
-        unlink = [link for link in links
-                  if os.path.realpath(link) in devices]
+        unlink = []
+        for link in links:
+            try:
+                if os.path.realpath(link) in devices:
+                    unlink.append(link)
+            except OSError:
+                # A race condition in Python's posixpath:realpath just occurred
+                # so we can ignore it because the file was just removed between
+                # a check if file exists and a call to os.readlink
+                continue
+
         if unlink:
             priv_rootwrap.unlink_root(no_errors=True, *unlink)
 
diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py
index f876dadd5..77b320834 100644
--- a/os_brick/tests/initiator/test_linuxscsi.py
+++ b/os_brick/tests/initiator/test_linuxscsi.py
@@ -867,6 +867,18 @@ loop0                                     0"""
         realpath_mock.assert_has_calls([mock.call(g) for g in paths])
         unlink_mock.assert_not_called()
 
+    @mock.patch.object(linuxscsi.priv_rootwrap, 'unlink_root')
+    @mock.patch('glob.glob')
+    @mock.patch('os.path.realpath', side_effect=[OSError, '/dev/sda'])
+    def test_remove_scsi_symlinks_race_condition(self, realpath_mock,
+                                                 glob_mock, unlink_mock):
+        paths = ['/dev/disk/by-id/scsi-wwid1', '/dev/disk/by-id/scsi-wwid2']
+        glob_mock.return_value = paths
+        self.linuxscsi._remove_scsi_symlinks(['sda'])
+        glob_mock.assert_called_once_with('/dev/disk/by-id/scsi-*')
+        realpath_mock.assert_has_calls([mock.call(g) for g in paths])
+        unlink_mock.assert_called_once_with(paths[1], no_errors=True)
+
     @mock.patch('glob.glob')
     def test_get_hctl_with_target(self, glob_mock):
         glob_mock.return_value = [