Merge "When a filesystem does't support xattr return a 507"

swift/common/exceptions.py

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

swift/obj/diskfile.py

@@ -39,13 +39,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 eventlet.hubs import trampoline
 
@@ -61,7 +61,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
@@ -84,6 +84,22 @@ get_tmp_dir = partial(get_policy_string, TMP_BASE)
 MD5_OF_EMPTY_STRING = 'd41d8cd98f00b204e9800998ecf8427e'
 
 
+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.
@@ -96,10 +112,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:
+    except IOError as e:
-        pass
+        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)
 
 
@@ -113,9 +135,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])
+        try:
-        metastr = metastr[254:]
+            xattr.setxattr(fd, '%s%s' % (METADATA_KEY, key or ''),
-        key += 1
+                           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

@@ -36,7 +36,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, \
@@ -353,6 +354,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))
@@ -376,7 +379,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
@@ -406,6 +412,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 = {}
 
@@ -474,7 +482,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:
@@ -538,6 +546,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'):
@@ -560,6 +570,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'):
@@ -601,6 +613,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

@@ -44,7 +44,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
 
@@ -1059,6 +1059,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)
@@ -1618,6 +1629,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
@@ -726,6 +727,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):