From: John Spray <john.spray@redhat.com>
Date: Thu, 3 Mar 2016 13:20:49 +0000 (+0000)
Subject: pybind: fix libcephfs getxattr
X-Git-Tag: v10.1.1~124^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7043b594aec7c88e5aeb91bfb0aa463691065aa2;p=ceph.git

pybind: fix libcephfs getxattr

The python bindings were expecting a different behaviour
than libcephfs actually has.  On overflow, it doesn't return
the data size, it just gives you -ERANGE.  The python bindings
need to cope with that, and also let you pass in a size param
so that you can handle it.

Signed-off-by: John Spray <john.spray@redhat.com>
---

diff --git a/src/pybind/cephfs/cephfs.pyx b/src/pybind/cephfs/cephfs.pyx
index 712163671b98..4f2334b1adb2 100644
--- a/src/pybind/cephfs/cephfs.pyx
+++ b/src/pybind/cephfs/cephfs.pyx
@@ -183,6 +183,9 @@ class LibCephFSStateError(Error):
     pass
 
 
+class OutOfRange(Error):
+    pass
+
 cdef errno_to_exception =  {
     errno.EPERM     : PermissionError,
     errno.ENOENT    : ObjectNotFound,
@@ -192,6 +195,7 @@ cdef errno_to_exception =  {
     errno.ENODATA   : NoData,
     errno.EINVAL    : InvalidValue,
     errno.EOPNOTSUPP: OperationNotSupported,
+    errno.ERANGE    : OutOfRange,
 }
 
 
@@ -689,7 +693,7 @@ cdef class LibCephFS(object):
             raise make_ex(ret, "error in write")
         return ret
 
-    def getxattr(self, path, name):
+    def getxattr(self, path, name, size=255):
         self.require_state("mounted")
 
         path = cstr(path, 'path')
@@ -699,7 +703,7 @@ cdef class LibCephFS(object):
             char* _path = path
             char* _name = name
 
-            size_t ret_length = 255
+            size_t ret_length = size
             char *ret_buf = NULL
 
         try:
@@ -711,14 +715,6 @@ cdef class LibCephFS(object):
             if ret < 0:
                 raise make_ex(ret, "error in getxattr")
 
-            if ret > ret_length:
-                ret_buf = <char *>realloc_chk(ret_buf, ret)
-                with nogil:
-                    ret = ceph_getxattr(self.cluster, _path, _name, ret_buf,
-                                        ret)
-                if ret < 0:
-                    raise make_ex(ret, "error in getxattr")
-
             return ret_buf[:ret]
         finally:
             free(ret_buf)
diff --git a/src/test/pybind/test_cephfs.py b/src/test/pybind/test_cephfs.py
index 77cd31bf9946..6ed18108b61f 100644
--- a/src/test/pybind/test_cephfs.py
+++ b/src/test/pybind/test_cephfs.py
@@ -86,13 +86,11 @@ def test_xattr():
 
     cephfs.setxattr("/", "user.big", "x" * 300, 0)
 
-    # FIXME: Python bindings expect ceph_getxattr to return the attr size,
-    # even if it's longer than the requested size.  It actually returns
-    # -ERANGE.
-    # What *should* happen is that the cephfs pythno bindings should
-    # either accept a size field, or they should increase the buffer size
-    # until they can accomodate the true size of the buffer.
-    # assert_equal(300, len(cephfs.getxattr("/", "user.big")))
+    # Default size is 255, get ERANGE
+    assert_raises(libcephfs.OutOfRange, cephfs.getxattr, "/", "user.big")
+
+    # Pass explicit size, and we'll get the value
+    assert_equal(300, len(cephfs.getxattr("/", "user.big", 300)))
 
 
 @with_setup(setup_test)