Fix BZ 865858: remove unnecessary system calls around xattr ops

The following is a simple refactoring of the read and write metadata methods
to just make calls to get/set/remove xattr operations instead of trying to
stat/open/close the file. For a single GET operation, this brings down the
number of system calls made from 13 to 3 for a file object with no
significantly sized metadata (meaning, only 1 xattr key/value pair is needed
to store the metadata).

Change-Id: I698f5886d63d5203f130c8eac1536a3ee4b0dbfb
BUG: 865858
Signed-off-by: Peter Portante <peter.portante@redhat.com>
Reviewed-on: http://review.gluster.org/4078
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>

1 files changed, 19 insertions, 73 deletions

diff --git a/swift/1.4.8/plugins/utils.py b/swift/1.4.8/plugins/utils.py
index b282f42be4a..7be857e9513 100644
--- a/swift/1.4.8/plugins/utils.py
+++ b/swift/1.4.8/plugins/utils.py
@@ -174,69 +174,6 @@ def do_rename(old_path, new_path):
         raise
     return True
 
-def do_setxattr(path, key, value):
-    fd = None
-    if not os.path.isdir(path):
-        fd = do_open(path, 'rb')
-    else:
-        fd = path
-    if fd or os.path.isdir(path):
-        try:
-            xattr.set(fd, key, value)
-        except Exception, err:
-            logging.exception("xattr.set failed on %s key %s err: %s", path, key, str(err))
-            raise
-        finally:
-            if fd and not os.path.isdir(path):
-                do_close(fd)
-    else:
-        logging.error("Open failed path %s", path)
-        return False
-    return True
-
-
-
-def do_getxattr(path, key, log = True):
-    fd = None
-    if not os.path.isdir(path):
-        fd = do_open(path, 'rb')
-    else:
-        fd = path
-    if fd or os.path.isdir(path):
-        try:
-            value = xattr.get(fd, key)
-        except Exception, err:
-            if log:
-                logging.exception("xattr.get failed on %s key %s err: %s", path, key, str(err))
-            raise
-        finally:
-            if fd and not os.path.isdir(path):
-                do_close(fd)
-    else:
-        logging.error("Open failed path %s", path)
-        return False
-    return value
-
-def do_removexattr(path, key):
-    fd = None
-    if not os.path.isdir(path):
-        fd = do_open(path, 'rb')
-    else:
-        fd = path
-    if fd or os.path.isdir(path):
-        try:
-            xattr.remove(fd, key)
-        except Exception, err:
-            logging.exception("xattr.remove failed on %s key %s err: %s", path, key, str(err))
-            raise
-        finally:
-            if fd and not os.path.isdir(path):
-                do_close(fd)
-    else:
-        logging.error("Open failed path %s", path)
-        return False
-    return True
-
 def read_metadata(path):
     """
     Helper function to read the pickled metadata from a File/Directory .
@@ -249,9 +186,10 @@ def read_metadata(path):
     key = 0
     while True:
         try:
-            metadata += do_getxattr(path, '%s%s' % (METADATA_KEY, (key or '')),
-                            log = False)
-        except Exception:
+            metadata += xattr.get(path, '%s%s' % (METADATA_KEY, (key or '')))
+        except IOError as err:
+            if err.errno != errno.ENODATA:
+                logging.exception("xattr.get failed on %s key %s err: %s", path, key, str(err))
             break
         key += 1
     if metadata:
@@ -259,7 +197,6 @@ def read_metadata(path):
     else:
         return {}
 
-
 def write_metadata(path, metadata):
     """
     Helper function to write pickled metadata for a File/Directory.
@@ -270,18 +207,24 @@ def write_metadata(path, metadata):
     metastr = pickle.dumps(metadata, PICKLE_PROTOCOL)
     key = 0
     while metastr:
-        do_setxattr(path, '%s%s' % (METADATA_KEY, key or ''), metastr[:254])
+        try:
+            xattr.set(path, '%s%s' % (METADATA_KEY, key or ''), metastr[:254])
+        except IOError as err:
+            logging.exception("xattr.set failed on %s key %s err: %s", path, key, str(err))
+            raise
         metastr = metastr[254:]
         key += 1
 
 def clean_metadata(path):
     key = 0
     while True:
-        value = do_getxattr(path, '%s%s' % (METADATA_KEY, (key or '')))
-        do_removexattr(path, '%s%s' % (METADATA_KEY, (key or '')))
+        try:
+            xattr.remove(path, '%s%s' % (METADATA_KEY, (key or '')))
+        except IOError as err:
+            if err.errno == errno.ENODATA:
+                break
         key += 1
 
-
 def dir_empty(path):
     """
     Return true if directory/container is empty.
@@ -311,7 +254,11 @@ def get_device_from_account(account):
 def check_user_xattr(path):
     if not os.path.exists(path):
         return False
-    do_setxattr(path, 'user.test.key1', 'value1')
+    try:
+        xattr.set(path, 'user.test.key1', 'value1')
+    except IOError as err:
+        logging.exception("check_user_xattr: set failed on %s err: %s", path, str(err))
+        raise
     try:
         xattr.remove(path, 'user.test.key1')
     except Exception, err:
@@ -319,7 +266,6 @@ def check_user_xattr(path):
         #Remove xattr may fail in case of concurrent remove.
     return True
 
-
 def _check_valid_account(account, fs_object):
     mount_path = getattr(fs_object, 'mount_path', MOUNT_PATH)