diff options
author | Prashanth Pai <ppai@redhat.com> | 2016-05-23 14:47:38 +0530 |
---|---|---|
committer | Thiago da Silva <thiago@redhat.com> | 2016-08-10 11:37:28 -0700 |
commit | 88a1bd75a515e348c6cd5f08e98ec9e39b7f66e8 (patch) | |
tree | 556b3175371b87e63da4ce8a6f1c99ed90b03b17 | |
parent | e482e747f6715d56b3cf6b220c5447de95858bee (diff) |
Don't attempt unlink on a non-existing object
Problem:
getxattr() and unlink() were being called on an object path which was
already determined to be non-existent. This resulted in both these
syscalls always failing with ENOENT when client issues DELETE request on
an object that does not exist.
A request to DELETE an object will incur the following DiskFile API
calls in sequence:
disk_file.read_metadata()
disk_file.delete()
The above mentioned problem manifests because Swift code invokes
disk_file.delete() even when disk_file.read_metadata() has raised
DiskFileNotExist.
Fix:
During disk_file.read_metadata(), make a note that the file does not
exist. When disk_file.delete() is called, do not proceed with object
deletion.
Change-Id: Iaf6915197a8fced7564db8fab80e696dea080c25
Signed-off-by: Prashanth Pai <ppai@redhat.com>
Reviewed-on: http://review.gluster.org/14501
Reviewed-by: Thiago da Silva <thiago@redhat.com>
Tested-by: Thiago da Silva <thiago@redhat.com>
-rw-r--r-- | gluster/swift/obj/diskfile.py | 14 | ||||
-rw-r--r-- | test/unit/obj/test_diskfile.py | 28 |
2 files changed, 41 insertions, 1 deletions
diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index f140a62..c7240ae 100644 --- a/gluster/swift/obj/diskfile.py +++ b/gluster/swift/obj/diskfile.py @@ -571,6 +571,8 @@ class DiskFile(object): self._stat = None # Don't store a value for data_file until we know it exists. self._data_file = None + # This is used to avoid unlink() on a file that does not exist. + self._disk_file_does_not_exist = False self._account = account # Unused, account = volume self._container = container @@ -752,6 +754,7 @@ class DiskFile(object): self._metadata = read_metadata(self._data_file) except (OSError, IOError) as err: if err.errno in (errno.ENOENT, errno.ESTALE): + self._disk_file_does_not_exist = True raise DiskFileNotExist raise err @@ -762,6 +765,7 @@ class DiskFile(object): self._stat = do_stat(self._data_file) except (OSError, IOError) as err: if err.errno in (errno.ENOENT, errno.ESTALE): + self._disk_file_does_not_exist = True raise DiskFileNotExist raise err @@ -1087,10 +1091,18 @@ class DiskFile(object): :raises DiskFileError: this implementation will raise the same errors as the `create()` method. """ + if self._disk_file_does_not_exist: + # It was determined during disk_file.read_metadata() call that + # the file does not exist. Don't proceed with deletion on a + # non-existing path. + return + try: metadata = self._metadata or read_metadata(self._data_file) except (IOError, OSError) as err: - if err.errno not in (errno.ESTALE, errno.ENOENT): + if err.errno in (errno.ESTALE, errno.ENOENT): + return + else: raise else: if metadata[X_TIMESTAMP] >= timestamp: diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index eab8ebf..72db849 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -1142,3 +1142,31 @@ class TestDiskFile(unittest.TestCase): self.assertFalse(_m_do_fchown.called) assert os.path.exists(gdf._data_file) assert not os.path.exists(tmppath) + + def test_unlink_not_called_on_non_existent_object(self): + # Create container dir + cpath = os.path.join(self.td, "vol0", "container") + os.makedirs(cpath) + self.assertTrue(os.path.exists(cpath)) + + # This file does not exist + obj_path = os.path.join(cpath, "object") + self.assertFalse(os.path.exists(obj_path)) + + # Create diskfile instance and check attribute initialization + gdf = self._get_diskfile("vol0", "p57", "ufo47", "container", "object") + assert gdf._obj == "object" + assert gdf._data_file == obj_path + self.assertFalse(gdf._disk_file_does_not_exist) + + # Simulate disk file call sequence issued during DELETE request. + # And confirm that read_metadata() and unlink() is not called. + self.assertRaises(DiskFileNotExist, gdf.read_metadata) + self.assertTrue(gdf._disk_file_does_not_exist) + _m_rmd = Mock() + _m_do_unlink = Mock() + with patch("gluster.swift.obj.diskfile.read_metadata", _m_rmd): + with patch("gluster.swift.obj.diskfile.do_unlink", _m_do_unlink): + gdf.delete(0) + self.assertFalse(_m_rmd.called) + self.assertFalse(_m_do_unlink.called) |