When a filesystem does't support xattr return a 507

Currently when the object server tries to write an object's metadata
to a filesystem that doesn't support xattr, it errors with a stacktrace
and returns a 500 error back to the user with no information.

This patch catches the resulting IOError when attempting to read or write
the xattr metadata, logs the error nicely and then returns a 507 error
back to the user.

Seeing as this change is sending back a 507, it also catches and logs
the out of disk space errors (ENOSPC and EDQUOT).

Change-Id: I31932b57582817a0b3b58dd315a996bd0bcbc99b
Closes-Bug: #966671

swift/common/exceptions.py

@@ -75,6 +75,10 @@ class DiskFileDeviceUnavailable(DiskFileError):
     pass
 
 
+class DiskFileXattrNotSupported(DiskFileError):
+    pass
+
+
 class DeviceUnavailable(SwiftException):
     pass
 

swift/obj/diskfile.py

@@ -38,13 +38,13 @@ import uuid
 import hashlib
 import logging
 import traceback
+import xattr
 from os.path import basename, dirname, exists, getmtime, join
 from random import shuffle
 from tempfile import mkstemp
 from contextlib import contextmanager
 from collections import defaultdict
 
-from xattr import getxattr, setxattr
 from eventlet import Timeout
 
 from swift import gettext_ as _
@@ -56,7 +56,7 @@ from swift.common.utils import mkdirs, Timestamp, \
 from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
     DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \
     DiskFileDeleted, DiskFileError, DiskFileNotOpen, PathNotDir, \
-    ReplicationLockTimeout, DiskFileExpired
+    ReplicationLockTimeout, DiskFileExpired, DiskFileXattrNotSupported
 from swift.common.swob import multi_range_iterator
 from swift.common.storage_policy import get_policy_string, POLICIES
 from functools import partial
@@ -76,6 +76,22 @@ get_async_dir = partial(get_policy_string, ASYNCDIR_BASE)
 get_tmp_dir = partial(get_policy_string, TMP_BASE)
 
 
+def _get_filename(fd):
+    """
+    Helper function to get to file name from a file descriptor or filename.
+
+    :param fd: file descriptor or filename.
+
+    :returns: the filename.
+    """
+    if hasattr(fd, 'name'):
+        # fd object
+        return fd.name
+
+    # fd is a filename
+    return fd
+
+
 def read_metadata(fd):
     """
     Helper function to read the pickled metadata from an object file.
@@ -88,10 +104,16 @@ def read_metadata(fd):
     key = 0
     try:
         while True:
-            metadata += getxattr(fd, '%s%s' % (METADATA_KEY, (key or '')))
+            metadata += xattr.getxattr(fd, '%s%s' % (METADATA_KEY,
+                                                    (key or '')))
             key += 1
-    except IOError:
-        pass
+    except IOError as e:
+        for err in 'ENOTSUP', 'EOPNOTSUPP':
+            if hasattr(errno, err) and e.errno == getattr(errno, err):
+                msg = "Filesystem at %s does not support xattr" % \
+                      _get_filename(fd)
+                logging.exception(msg)
+                raise DiskFileXattrNotSupported(e)
     return pickle.loads(metadata)
 
 
@@ -105,9 +127,23 @@ def write_metadata(fd, metadata):
     metastr = pickle.dumps(metadata, PICKLE_PROTOCOL)
     key = 0
     while metastr:
-        setxattr(fd, '%s%s' % (METADATA_KEY, key or ''), metastr[:254])
-        metastr = metastr[254:]
-        key += 1
+        try:
+            xattr.setxattr(fd, '%s%s' % (METADATA_KEY, key or ''),
+                           metastr[:254])
+            metastr = metastr[254:]
+            key += 1
+        except IOError as e:
+            for err in 'ENOTSUP', 'EOPNOTSUPP':
+                if hasattr(errno, err) and e.errno == getattr(errno, err):
+                    msg = "Filesystem at %s does not support xattr" % \
+                          _get_filename(fd)
+                    logging.exception(msg)
+                    raise DiskFileXattrNotSupported(e)
+            if e.errno in (errno.ENOSPC, errno.EDQUOT):
+                msg = "No space left on device for %s" % _get_filename(fd)
+                logging.exception(msg)
+                raise DiskFileNoSpace()
+            raise
 
 
 def extract_policy_index(obj_path):

swift/obj/server.py

@@ -35,7 +35,8 @@ from swift.common.constraints import check_object_creation, \
     valid_timestamp, check_utf8
 from swift.common.exceptions import ConnectionTimeout, DiskFileQuarantined, \
     DiskFileNotExist, DiskFileCollision, DiskFileNoSpace, DiskFileDeleted, \
-    DiskFileDeviceUnavailable, DiskFileExpired, ChunkReadTimeout
+    DiskFileDeviceUnavailable, DiskFileExpired, ChunkReadTimeout, \
+    DiskFileXattrNotSupported
 from swift.obj import ssync_receiver
 from swift.common.http import is_success
 from swift.common.request_helpers import get_name_and_placement, is_user_meta
@@ -338,6 +339,8 @@ class ObjectController(object):
             return HTTPInsufficientStorage(drive=device, request=request)
         try:
             orig_metadata = disk_file.read_metadata()
+        except DiskFileXattrNotSupported:
+            return HTTPInsufficientStorage(drive=device, request=request)
         except (DiskFileNotExist, DiskFileQuarantined):
             return HTTPNotFound(request=request)
         orig_timestamp = Timestamp(orig_metadata.get('X-Timestamp', 0))
@@ -359,7 +362,10 @@ class ObjectController(object):
                 self.delete_at_update('DELETE', orig_delete_at, account,
                                       container, obj, request, device,
                                       policy_idx)
-        disk_file.write_metadata(metadata)
+        try:
+            disk_file.write_metadata(metadata)
+        except (DiskFileXattrNotSupported, DiskFileNoSpace):
+            return HTTPInsufficientStorage(drive=device, request=request)
         return HTTPAccepted(request=request)
 
     @public
@@ -389,6 +395,8 @@ class ObjectController(object):
             return HTTPInsufficientStorage(drive=device, request=request)
         try:
             orig_metadata = disk_file.read_metadata()
+        except DiskFileXattrNotSupported:
+            return HTTPInsufficientStorage(drive=device, request=request)
         except (DiskFileNotExist, DiskFileQuarantined):
             orig_metadata = {}
 
@@ -453,7 +461,7 @@ class ObjectController(object):
                         header_caps = header_key.title()
                         metadata[header_caps] = request.headers[header_key]
                 writer.put(metadata)
-        except DiskFileNoSpace:
+        except (DiskFileXattrNotSupported, DiskFileNoSpace):
             return HTTPInsufficientStorage(drive=device, request=request)
         if orig_delete_at != new_delete_at:
             if new_delete_at:
@@ -517,6 +525,8 @@ class ObjectController(object):
                 response.headers['X-Timestamp'] = file_x_ts.normal
                 response.headers['X-Backend-Timestamp'] = file_x_ts.internal
                 resp = request.get_response(response)
+        except DiskFileXattrNotSupported:
+            return HTTPInsufficientStorage(drive=device, request=request)
         except (DiskFileNotExist, DiskFileQuarantined) as e:
             headers = {}
             if hasattr(e, 'timestamp'):
@@ -539,6 +549,8 @@ class ObjectController(object):
             return HTTPInsufficientStorage(drive=device, request=request)
         try:
             metadata = disk_file.read_metadata()
+        except DiskFileXattrNotSupported:
+            return HTTPInsufficientStorage(drive=device, request=request)
         except (DiskFileNotExist, DiskFileQuarantined) as e:
             headers = {}
             if hasattr(e, 'timestamp'):
@@ -580,6 +592,8 @@ class ObjectController(object):
             return HTTPInsufficientStorage(drive=device, request=request)
         try:
             orig_metadata = disk_file.read_metadata()
+        except DiskFileXattrNotSupported:
+            return HTTPInsufficientStorage(drive=device, request=request)
         except DiskFileExpired as e:
             orig_timestamp = e.timestamp
             orig_metadata = e.metadata

test/unit/obj/test_diskfile.py

@@ -43,7 +43,7 @@ from swift.common import ring
 from swift.common.exceptions import DiskFileNotExist, DiskFileQuarantined, \
     DiskFileDeviceUnavailable, DiskFileDeleted, DiskFileNotOpen, \
     DiskFileError, ReplicationLockTimeout, PathNotDir, DiskFileCollision, \
-    DiskFileExpired, SwiftException, DiskFileNoSpace
+    DiskFileExpired, SwiftException, DiskFileNoSpace, DiskFileXattrNotSupported
 from swift.common.storage_policy import POLICIES, get_policy_string
 from functools import partial
 
@@ -1046,6 +1046,17 @@ class TestDiskFile(unittest.TestCase):
         md = df.read_metadata()
         self.assertEqual(md['X-Timestamp'], Timestamp(42).internal)
 
+    def test_read_metadata_no_xattr(self):
+        def mock_getxattr(*args, **kargs):
+            error_num = errno.ENOTSUP if hasattr(errno, 'ENOTSUP') else \
+                errno.EOPNOTSUPP
+            raise IOError(error_num, "Operation not supported")
+
+        with mock.patch('xattr.getxattr', mock_getxattr):
+            self.assertRaises(
+                DiskFileXattrNotSupported,
+                diskfile.read_metadata, 'n/a')
+
     def test_get_metadata_not_opened(self):
         df = self._simple_get_diskfile()
         self.assertRaises(DiskFileNotOpen, df.get_metadata)
@@ -1586,6 +1597,40 @@ class TestDiskFile(unittest.TestCase):
         exp_name = '%s.meta' % timestamp
         self.assertTrue(exp_name in set(dl))
 
+    def test_write_metadata_no_xattr(self):
+        timestamp = Timestamp(time()).internal
+        metadata = {'X-Timestamp': timestamp, 'X-Object-Meta-test': 'data'}
+
+        def mock_setxattr(*args, **kargs):
+            error_num = errno.ENOTSUP if hasattr(errno, 'ENOTSUP') else \
+                errno.EOPNOTSUPP
+            raise IOError(error_num, "Operation not supported")
+
+        with mock.patch('xattr.setxattr', mock_setxattr):
+            self.assertRaises(
+                DiskFileXattrNotSupported,
+                diskfile.write_metadata, 'n/a', metadata)
+
+    def test_write_metadata_disk_full(self):
+        timestamp = Timestamp(time()).internal
+        metadata = {'X-Timestamp': timestamp, 'X-Object-Meta-test': 'data'}
+
+        def mock_setxattr_ENOSPC(*args, **kargs):
+            raise IOError(errno.ENOSPC, "No space left on device")
+
+        def mock_setxattr_EDQUOT(*args, **kargs):
+            raise IOError(errno.EDQUOT, "Exceeded quota")
+
+        with mock.patch('xattr.setxattr', mock_setxattr_ENOSPC):
+            self.assertRaises(
+                DiskFileNoSpace,
+                diskfile.write_metadata, 'n/a', metadata)
+
+        with mock.patch('xattr.setxattr', mock_setxattr_EDQUOT):
+            self.assertRaises(
+                DiskFileNoSpace,
+                diskfile.write_metadata, 'n/a', metadata)
+
     def test_delete(self):
         df = self._get_open_disk_file()
         ts = time()

test/unit/obj/test_server.py

@@ -18,6 +18,7 @@
 
 import cPickle as pickle
 import datetime
+import errno
 import operator
 import os
 import mock
@@ -708,6 +709,27 @@ class TestObjectController(unittest.TestCase):
                            'X-Object-Meta-1': 'One',
                            'X-Object-Meta-Two': 'Two'})
 
+    def test_PUT_user_metadata_no_xattr(self):
+        timestamp = normalize_timestamp(time())
+        req = Request.blank(
+            '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
+            headers={'X-Timestamp': timestamp,
+                     'Content-Type': 'text/plain',
+                     'ETag': 'b114ab7b90d9ccac4bd5d99cc7ebb568',
+                     'X-Object-Meta-1': 'One',
+                     'X-Object-Meta-Two': 'Two'})
+        req.body = 'VERIFY THREE'
+
+        def mock_get_and_setxattr(*args, **kargs):
+            error_num = errno.ENOTSUP if hasattr(errno, 'ENOTSUP') else \
+                errno.EOPNOTSUPP
+            raise IOError(error_num, 'Operation not supported')
+
+        with mock.patch('xattr.getxattr', mock_get_and_setxattr):
+            with mock.patch('xattr.setxattr', mock_get_and_setxattr):
+                resp = req.get_response(self.object_controller)
+                self.assertEquals(resp.status_int, 507)
+
     def test_PUT_client_timeout(self):
         class FakeTimeout(BaseException):
             def __enter__(self):