From 404fcd815c4abe00742531bf19a5224c0fe45b36 Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Tue, 25 Mar 2014 10:03:05 -0400 Subject: Updating code to call fs_utils instead of python's os module This change is being done to prepare the code to always call fs_utils for all filesystem calls on the data being stored. By creating this interface (i.e., fs_utils), we can then make a seamless change to use the current method of 'os.' calls over FUSE _or_ libgfapi. This new method will be introduced in a new separate patch. Change-Id: Ic768fa7352d7672b1f060230bb4486f0ec228152 Signed-off-by: Thiago da Silva Reviewed-on: http://review.gluster.org/7333 Reviewed-by: Prashanth Pai Reviewed-by: Luis Pabon Tested-by: Luis Pabon --- gluster/swift/common/DiskDir.py | 13 ++++--- gluster/swift/common/fs_utils.py | 42 ++++++++++++++++++++- gluster/swift/common/utils.py | 70 +++++++++++++++++------------------ gluster/swift/obj/diskfile.py | 77 +++++++++++++++++++++------------------ test/unit/common/test_fs_utils.py | 55 +++++++++++----------------- 5 files changed, 145 insertions(+), 112 deletions(-) diff --git a/gluster/swift/common/DiskDir.py b/gluster/swift/common/DiskDir.py index eb0b292..0a91009 100644 --- a/gluster/swift/common/DiskDir.py +++ b/gluster/swift/common/DiskDir.py @@ -16,7 +16,8 @@ import os import errno -from gluster.swift.common.fs_utils import dir_empty, mkdirs, os_path, do_chown +from gluster.swift.common.fs_utils import dir_empty, mkdirs, do_chown, \ + do_exists, do_touch from gluster.swift.common.utils import validate_account, validate_container, \ get_container_details, get_account_details, create_container_metadata, \ create_account_metadata, DEFAULT_GID, get_container_metadata, \ @@ -158,8 +159,8 @@ class DiskCommon(object): global _db_file if not _db_file: _db_file = os.path.join(Glusterfs.RUN_DIR, 'db_file.db') - if not os.path.exists(_db_file): - file(_db_file, 'w+') + if not do_exists(_db_file): + do_touch(_db_file) self.db_file = _db_file self.metadata = {} self.pending_timeout = pending_timeout or 10 @@ -173,7 +174,7 @@ class DiskCommon(object): self._dir_exists = None def _dir_exists_read_metadata(self): - self._dir_exists = os_path.exists(self.datadir) + self._dir_exists = do_exists(self.datadir) if self._dir_exists: self.metadata = _read_metadata(self.datadir) return self._dir_exists @@ -181,7 +182,7 @@ class DiskCommon(object): def is_deleted(self): # The intention of this method is to check the file system to see if # the directory actually exists. - return not os_path.exists(self.datadir) + return not do_exists(self.datadir) def empty(self): # If it does not exist, then it is empty. A value of True is @@ -464,7 +465,7 @@ class DiskDir(DiskCommon): If the container does exist, update the PUT timestamp only if it is later than the existing value. """ - if not os_path.exists(self.datadir): + if not do_exists(self.datadir): self.initialize(timestamp) else: if timestamp > self.metadata[X_PUT_TIMESTAMP]: diff --git a/gluster/swift/common/fs_utils.py b/gluster/swift/common/fs_utils.py index a93fa6b..9f698df 100644 --- a/gluster/swift/common/fs_utils.py +++ b/gluster/swift/common/fs_utils.py @@ -19,10 +19,10 @@ import errno import stat import random import time +import xattr from collections import defaultdict from itertools import repeat import ctypes -import os.path as _os_path from eventlet import sleep from swift.common.utils import load_libc_function from gluster.swift.common.exceptions import FileOrDirNotFoundError, \ @@ -30,7 +30,41 @@ from gluster.swift.common.exceptions import FileOrDirNotFoundError, \ from swift.common.exceptions import DiskFileNoSpace -os_path = _os_path +def do_exists(path): + return os.path.exists(path) + + +def do_touch(path): + with open(path, 'a'): + os.utime(path, None) + + +def do_getctime(path): + return os.path.getctime(path) + + +def do_getmtime(path): + return os.path.getmtime(path) + + +def do_isdir(path): + return os.path.isdir(path) + + +def do_getsize(path): + return os.path.getsize(path) + + +def do_getxattr(path, key): + return xattr.getxattr(path, key) + + +def do_setxattr(path, key, value): + xattr.setxattr(path, key, value) + + +def do_removexattr(path, key): + xattr.removexattr(path, key) def do_walk(*args, **kwargs): @@ -212,6 +246,10 @@ def do_open(path, flags, **kwargs): return fd +def do_dup(fd): + return os.dup(fd) + + def do_close(fd): try: os.close(fd) diff --git a/gluster/swift/common/utils.py b/gluster/swift/common/utils.py index f2e112a..145add3 100644 --- a/gluster/swift/common/utils.py +++ b/gluster/swift/common/utils.py @@ -16,15 +16,16 @@ import os import stat import errno -import xattr import logging from hashlib import md5 from eventlet import sleep import cPickle as pickle from gluster.swift.common.exceptions import GlusterFileSystemIOError from swift.common.exceptions import DiskFileNoSpace -from gluster.swift.common.fs_utils import os_path, do_stat, do_listdir, \ - do_walk, do_rmdir, do_fstat, do_log_rl, get_filename_from_fd +from gluster.swift.common.fs_utils import do_getctime, do_getmtime, do_stat, \ + do_listdir, do_walk, do_rmdir, do_log_rl, get_filename_from_fd, do_open, \ + do_isdir, do_getsize, do_getxattr, do_setxattr, do_removexattr, do_read, \ + do_close, do_dup, do_lseek, do_fstat from gluster.swift.common import Glusterfs X_CONTENT_TYPE = 'Content-Type' @@ -83,8 +84,8 @@ def read_metadata(path_or_fd): key = 0 while metadata is None: try: - metadata_s += xattr.getxattr(path_or_fd, - '%s%s' % (METADATA_KEY, (key or ''))) + metadata_s += do_getxattr(path_or_fd, + '%s%s' % (METADATA_KEY, (key or ''))) except IOError as err: if err.errno == errno.ENODATA: if key > 0: @@ -103,7 +104,7 @@ def read_metadata(path_or_fd): # Note that we don't touch the keys on errors fetching the # data since it could be a transient state. raise GlusterFileSystemIOError( - err.errno, 'xattr.getxattr("%s", %s)' % (path_or_fd, key)) + err.errno, 'getxattr("%s", %s)' % (path_or_fd, key)) else: try: # If this key provides all or the remaining part of the pickle @@ -134,9 +135,9 @@ def write_metadata(path_or_fd, metadata): key = 0 while metastr: try: - xattr.setxattr(path_or_fd, - '%s%s' % (METADATA_KEY, key or ''), - metastr[:MAX_XATTR_SIZE]) + do_setxattr(path_or_fd, + '%s%s' % (METADATA_KEY, key or ''), + metastr[:MAX_XATTR_SIZE]) except IOError as err: if err.errno in (errno.ENOSPC, errno.EDQUOT): if isinstance(path_or_fd, int): @@ -150,7 +151,7 @@ def write_metadata(path_or_fd, metadata): else: raise GlusterFileSystemIOError( err.errno, - 'xattr.setxattr("%s", %s, metastr)' % (path_or_fd, key)) + 'setxattr("%s", %s, metastr)' % (path_or_fd, key)) metastr = metastr[MAX_XATTR_SIZE:] key += 1 @@ -159,12 +160,12 @@ def clean_metadata(path_or_fd): key = 0 while True: try: - xattr.removexattr(path_or_fd, '%s%s' % (METADATA_KEY, (key or ''))) + do_removexattr(path_or_fd, '%s%s' % (METADATA_KEY, (key or ''))) except IOError as err: if err.errno == errno.ENODATA: break raise GlusterFileSystemIOError( - err.errno, 'xattr.removexattr("%s", %s)' % (path_or_fd, key)) + err.errno, 'removexattr("%s", %s)' % (path_or_fd, key)) key += 1 @@ -254,7 +255,7 @@ def _update_list(path, cont_path, src_list, reg_file=True, object_count=0, object_count += 1 if reg_file and Glusterfs._do_getsize: - bytes_used += os_path.getsize(os.path.join(path, obj_name)) + bytes_used += do_getsize(os.path.join(path, obj_name)) sleep() return object_count, bytes_used @@ -281,7 +282,7 @@ def get_container_details(cont_path): object_count = 0 obj_list = [] - if os_path.isdir(cont_path): + if do_isdir(cont_path): for (path, dirs, files) in do_walk(cont_path): object_count, bytes_used = update_list(path, cont_path, dirs, files, object_count, @@ -299,17 +300,14 @@ def get_account_details(acc_path): container_list = [] container_count = 0 - acc_stats = do_stat(acc_path) - if acc_stats: - is_dir = stat.S_ISDIR(acc_stats.st_mode) - if is_dir: - for name in do_listdir(acc_path): - if name.lower() == TEMP_DIR \ - or name.lower() == ASYNCDIR \ - or not os_path.isdir(os.path.join(acc_path, name)): - continue - container_count += 1 - container_list.append(name) + if do_isdir(acc_path): + for name in do_listdir(acc_path): + if name.lower() == TEMP_DIR \ + or name.lower() == ASYNCDIR \ + or not do_isdir(os.path.join(acc_path, name)): + continue + container_count += 1 + container_list.append(name) return container_list, container_count @@ -317,7 +315,7 @@ def get_account_details(acc_path): def _read_for_etag(fp): etag = md5() while True: - chunk = fp.read(CHUNK_SIZE) + chunk = do_read(fp, CHUNK_SIZE) if chunk: etag.update(chunk) if len(chunk) >= CHUNK_SIZE: @@ -343,15 +341,16 @@ def _get_etag(path_or_fd): # We are given a file descriptor, so this is an invocation from the # DiskFile.open() method. fd = path_or_fd - with os.fdopen(os.dup(fd), 'rb') as fp: - etag = _read_for_etag(fp) - os.lseek(fd, 0, os.SEEK_SET) + etag = _read_for_etag(do_dup(fd)) + do_lseek(fd, 0, os.SEEK_SET) else: # We are given a path to the object when the DiskDir.list_objects_iter # method invokes us. path = path_or_fd - with open(path, 'rb') as fp: - etag = _read_for_etag(fp) + fd = do_open(path, os.O_RDONLY) + etag = _read_for_etag(fd) + do_close(fd) + return etag @@ -367,6 +366,7 @@ def get_object_metadata(obj_path_or_fd): # We are given a path to the object when the DiskDir.list_objects_iter # method invokes us. stats = do_stat(obj_path_or_fd) + if not stats: metadata = {} else: @@ -401,9 +401,9 @@ def get_container_metadata(cont_path): objects, object_count, bytes_used = get_container_details(cont_path) metadata = {X_TYPE: CONTAINER, X_TIMESTAMP: normalize_timestamp( - os_path.getctime(cont_path)), + do_getctime(cont_path)), X_PUT_TIMESTAMP: normalize_timestamp( - os_path.getmtime(cont_path)), + do_getmtime(cont_path)), X_OBJECTS_COUNT: object_count, X_BYTES_USED: bytes_used} return _add_timestamp(metadata) @@ -415,9 +415,9 @@ def get_account_metadata(acc_path): containers, container_count = get_account_details(acc_path) metadata = {X_TYPE: ACCOUNT, X_TIMESTAMP: normalize_timestamp( - os_path.getctime(acc_path)), + do_getctime(acc_path)), X_PUT_TIMESTAMP: normalize_timestamp( - os_path.getmtime(acc_path)), + do_getmtime(acc_path)), X_OBJECTS_COUNT: 0, X_BYTES_USED: 0, X_CONTAINER_COUNT: container_count} diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index ad12c2d..47cde89 100644 --- a/gluster/swift/obj/diskfile.py +++ b/gluster/swift/obj/diskfile.py @@ -41,7 +41,7 @@ from gluster.swift.common.exceptions import GlusterFileSystemOSError from gluster.swift.common.Glusterfs import mount from gluster.swift.common.fs_utils import do_fstat, do_open, do_close, \ do_unlink, do_chown, do_fsync, do_fchown, do_stat, do_write, do_read, \ - do_fadvise64, do_rename, do_fdatasync, do_lseek + do_fadvise64, do_rename, do_fdatasync, do_lseek, do_mkdir from gluster.swift.common.utils import read_metadata, write_metadata, \ validate_object, create_object_metadata, rmobjdir, dir_is_object, \ get_object_metadata @@ -78,7 +78,7 @@ def make_directory(full_path, uid, gid, metadata=None): creating the object metadata if requested. """ try: - os.mkdir(full_path) + do_mkdir(full_path) except OSError as err: if err.errno == errno.ENOENT: # Tell the caller some directory of the parent path does not @@ -91,13 +91,13 @@ def make_directory(full_path, uid, gid, metadata=None): # FIXME: When we are confident, remove this stat() call as it is # not necessary. try: - stats = os.stat(full_path) - except OSError as serr: + stats = do_stat(full_path) + except GlusterFileSystemOSError as serr: # FIXME: Ideally we'd want to return an appropriate error # message and code in the PUT Object REST API response. - raise DiskFileError("_make_directory_unlocked: os.mkdir failed" + raise DiskFileError("make_directory: mkdir failed" " because path %s already exists, and" - " a subsequent os.stat on that same" + " a subsequent stat on that same" " path failed (%s)" % (full_path, str(serr))) else: @@ -105,8 +105,8 @@ def make_directory(full_path, uid, gid, metadata=None): if not is_dir: # FIXME: Ideally we'd want to return an appropriate error # message and code in the PUT Object REST API response. - raise AlreadyExistsAsFile("_make_directory_unlocked:" - " os.mkdir failed on path %s" + raise AlreadyExistsAsFile("make_directory:" + " mkdir failed on path %s" " because it already exists" " but not as a directory" % (full_path)) @@ -114,8 +114,8 @@ def make_directory(full_path, uid, gid, metadata=None): elif err.errno == errno.ENOTDIR: # FIXME: Ideally we'd want to return an appropriate error # message and code in the PUT Object REST API response. - raise AlreadyExistsAsFile("_make_directory_unlocked:" - " os.mkdir failed because some " + raise AlreadyExistsAsFile("make_directory:" + " mkdir failed because some " "part of path %s is not in fact" " a directory" % (full_path)) elif err.errno == errno.EIO: @@ -125,42 +125,47 @@ def make_directory(full_path, uid, gid, metadata=None): # short period of time. _random_sleep() try: - stats = os.stat(full_path) - except OSError as serr: + stats = do_stat(full_path) + except GlusterFileSystemOSError as serr: if serr.errno == errno.ENOENT: - errmsg = "_make_directory_unlocked: os.mkdir failed on" \ - " path %s (EIO), and a subsequent os.stat on" \ + errmsg = "make_directory: mkdir failed on" \ + " path %s (EIO), and a subsequent stat on" \ " that same path did not find the file." % ( full_path,) else: - errmsg = "_make_directory_unlocked: os.mkdir failed on" \ - " path %s (%s), and a subsequent os.stat on" \ + errmsg = "make_directory: mkdir failed on" \ + " path %s (%s), and a subsequent stat on" \ " that same path failed as well (%s)" % ( full_path, str(err), str(serr)) raise DiskFileError(errmsg) else: - # The directory at least exists now - is_dir = stat.S_ISDIR(stats.st_mode) - if is_dir: - # Dump the stats to the log with the original exception. - logging.warn("_make_directory_unlocked: os.mkdir initially" - " failed on path %s (%s) but a stat()" - " following that succeeded: %r" % (full_path, - str(err), - stats)) - # Assume another entity took care of the proper setup. - return True, metadata + if not stats: + errmsg = "make_directory: mkdir failed on" \ + " path %s (EIO), and a subsequent stat on" \ + " that same path did not find the file." % ( + full_path,) + raise DiskFileError(errmsg) else: - raise DiskFileError("_make_directory_unlocked: os.mkdir" - " initially failed on path %s (%s) but" - " now we see that it exists but is not" - " a directory (%r)" % (full_path, - str(err), - stats)) + # The directory at least exists now + is_dir = stat.S_ISDIR(stats.st_mode) + if is_dir: + # Dump the stats to the log with the original exception + logging.warn("make_directory: mkdir initially" + " failed on path %s (%s) but a stat()" + " following that succeeded: %r" % + (full_path, str(err), stats)) + # Assume another entity took care of the proper setup. + return True, metadata + else: + raise DiskFileError("make_directory: mkdir" + " initially failed on path %s (%s)" + " but now we see that it exists" + " but is not a directory (%r)" % + (full_path, str(err), stats)) else: # Some other potentially rare exception occurred that does not # currently warrant a special log entry to help diagnose. - raise DiskFileError("_make_directory_unlocked: os.mkdir failed on" + raise DiskFileError("make_directory: mkdir failed on" " path %s (%s)" % (full_path, str(err))) else: if metadata: @@ -393,7 +398,7 @@ class DiskFileWriter(object): else: # Let's retry since everything looks okay logging.warn( - "DiskFile.put(): os.rename('%s','%s')" + "DiskFile.put(): rename('%s','%s')" " initially failed (%s) but a" " stat('%s') following that succeeded:" " %r" % ( @@ -403,7 +408,7 @@ class DiskFileWriter(object): continue else: raise GlusterFileSystemOSError( - err.errno, "%s, os.rename('%s', '%s')" % ( + err.errno, "%s, rename('%s', '%s')" % ( err.strerror, self._tmppath, df._data_file)) else: # Success! diff --git a/test/unit/common/test_fs_utils.py b/test/unit/common/test_fs_utils.py index 46e7ffc..9f3ad3e 100644 --- a/test/unit/common/test_fs_utils.py +++ b/test/unit/common/test_fs_utils.py @@ -18,26 +18,28 @@ import shutil import random import errno import unittest -import eventlet from nose import SkipTest from mock import patch, Mock from time import sleep from tempfile import mkdtemp, mkstemp from gluster.swift.common import fs_utils as fs from gluster.swift.common.exceptions import NotDirectoryError, \ - FileOrDirNotFoundError, GlusterFileSystemOSError, \ - GlusterFileSystemIOError + FileOrDirNotFoundError, GlusterFileSystemOSError from swift.common.exceptions import DiskFileNoSpace + def mock_os_fsync(fd): return True + def mock_os_fdatasync(fd): return True + def mock_os_mkdir_makedirs_enospc(path): raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC)) + def mock_os_mkdir_makedirs_edquot(path): raise OSError(errno.EDQUOT, os.strerror(errno.EDQUOT)) @@ -52,9 +54,10 @@ class TestFsUtils(unittest.TestCase): tmpdirs = [] tmpfiles = [] for i in range(5): - tmpdirs.append(mkdtemp(dir=tmpparent).rsplit(os.path.sep, 1)[1]) - tmpfiles.append(mkstemp(dir=tmpparent)[1].rsplit(os.path.sep, \ - 1)[1]) + tmpdirs.append( + mkdtemp(dir=tmpparent).rsplit(os.path.sep, 1)[1]) + tmpfiles.append( + mkstemp(dir=tmpparent)[1].rsplit(os.path.sep, 1)[1]) for path, dirnames, filenames in fs.do_walk(tmpparent): assert path == tmpparent @@ -88,7 +91,7 @@ class TestFsUtils(unittest.TestCase): with patch("os.lstat", _mock_os_lstat): try: fs.do_ismount(tmpdir) - except GlusterFileSystemOSError as err: + except GlusterFileSystemOSError: pass else: self.fail("Expected GlusterFileSystemOSError") @@ -122,7 +125,7 @@ class TestFsUtils(unittest.TestCase): with patch("os.lstat", _mock_os_lstat): try: fs.do_ismount(tmpdir) - except GlusterFileSystemOSError as err: + except GlusterFileSystemOSError: pass else: self.fail("Expected GlusterFileSystemOSError") @@ -152,7 +155,7 @@ class TestFsUtils(unittest.TestCase): with patch("os.lstat", _mock_os_lstat): try: fs.do_ismount(tmpdir) - except GlusterFileSystemOSError as err: + except GlusterFileSystemOSError: self.fail("Unexpected exception") else: pass @@ -184,7 +187,7 @@ class TestFsUtils(unittest.TestCase): with patch("os.lstat", _mock_os_lstat): try: fs.do_ismount(tmpdir) - except GlusterFileSystemOSError as err: + except GlusterFileSystemOSError: self.fail("Unexpected exception") else: pass @@ -197,7 +200,7 @@ class TestFsUtils(unittest.TestCase): fd = fs.do_open(tmpfile, os.O_RDONLY) try: os.write(fd, 'test') - except OSError as err: + except OSError: pass else: self.fail("OSError expected") @@ -238,7 +241,7 @@ class TestFsUtils(unittest.TestCase): finally: os.close(fd1) except GlusterFileSystemOSError as ose: - self.fail("Open failed with %s" %ose.strerror) + self.fail("Open failed with %s" % ose.strerror) finally: os.close(fd) os.remove(tmpfile) @@ -273,7 +276,6 @@ class TestFsUtils(unittest.TestCase): path = os.path.join(subdir, str(random.random())) fs.mkdirs(path) assert os.path.exists(path) - assert fs.mkdirs(path) finally: shutil.rmtree(subdir) @@ -288,17 +290,6 @@ class TestFsUtils(unittest.TestCase): finally: shutil.rmtree(tmpdir) - def test_mkdirs(self): - tmpdir = mkdtemp() - try: - fs.mkdirs(os.path.join(tmpdir, "a", "b", "c")) - except OSError: - self.fail("Unexpected exception") - else: - pass - finally: - shutil.rmtree(tmpdir) - def test_mkdirs_existing_file(self): tmpdir = mkdtemp() fd, tmpfile = mkstemp(dir=tmpdir) @@ -343,7 +334,6 @@ class TestFsUtils(unittest.TestCase): else: self.fail("Expected DiskFileNoSpace exception") - def test_do_mkdir(self): try: path = os.path.join('/tmp', str(random.random())) @@ -420,7 +410,6 @@ class TestFsUtils(unittest.TestCase): else: self.fail("Expected GlusterFileSystemOSError") - def test_do_stat(self): tmpdir = mkdtemp() try: @@ -733,13 +722,13 @@ class TestFsUtils(unittest.TestCase): with patch('os.fsync', mock_os_fsync): assert fs.do_fsync(fd) is None except GlusterFileSystemOSError as ose: - self.fail('Opening a temporary file failed with %s' %ose.strerror) + self.fail('Opening a temporary file failed with %s' % + ose.strerror) else: os.close(fd) finally: shutil.rmtree(tmpdir) - def test_do_fsync_err(self): tmpdir = mkdtemp() try: @@ -766,13 +755,13 @@ class TestFsUtils(unittest.TestCase): with patch('os.fdatasync', mock_os_fdatasync): assert fs.do_fdatasync(fd) is None except GlusterFileSystemOSError as ose: - self.fail('Opening a temporary file failed with %s' %ose.strerror) + self.fail('Opening a temporary file failed with %s' % + ose.strerror) else: os.close(fd) finally: shutil.rmtree(tmpdir) - def test_do_fdatasync_err(self): tmpdir = mkdtemp() try: @@ -803,11 +792,11 @@ class TestFsUtils(unittest.TestCase): def test_get_filename_from_fd_err(self): result = fs.get_filename_from_fd("blah") - self.assertEqual(result,None) + self.assertEqual(result, None) result = fs.get_filename_from_fd(-1000) - self.assertEqual(result,None) + self.assertEqual(result, None) result = fs.get_filename_from_fd("blah", True) - self.assertEqual(result,None) + self.assertEqual(result, None) def test_static_var(self): @fs.static_var("counter", 0) -- cgit