Skip to content

Commit

Permalink
xattr: dynamically grow result buffer until it fits, fixes borgbackup…
Browse files Browse the repository at this point in the history
…#1462

this also fixes the race condition seen in borgbackup#1462 because there is only 1 call now.
either it succeeds, then we get the correct length as result and truncate the result value to that length.
or it fails with ERANGE, then we grow the buffer to double size and repeat.
or it fails with some other error, then we throw OSError.
  • Loading branch information
ThomasWaldmann committed Aug 11, 2016
1 parent a951d23 commit a6de9fc
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 18 deletions.
30 changes: 29 additions & 1 deletion borg/testsuite/xattr.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
import tempfile
import unittest

from ..xattr import is_enabled, getxattr, setxattr, listxattr
from ..xattr import is_enabled, getxattr, setxattr, listxattr, get_buffer
from . import BaseTestCase


@unittest.skipUnless(is_enabled(), 'xattr not enabled on filesystem')
class XattrTestCase(BaseTestCase):

def setUp(self):
# we need to start each test from 2048 to test buffer growing as ext4 is limited to 4096
get_buffer(size=2048, init=True)
self.tmpfile = tempfile.NamedTemporaryFile()
self.symlink = os.path.join(os.path.dirname(self.tmpfile.name), 'symlink')
os.symlink(self.tmpfile.name, self.symlink)
Expand Down Expand Up @@ -38,3 +40,29 @@ def test(self):
self.assert_equal(getxattr(self.tmpfile.fileno(), 'user.foo'), b'bar')
self.assert_equal(getxattr(self.symlink, 'user.foo'), b'bar')
self.assert_equal(getxattr(self.tmpfile.name, 'user.empty'), None)

def test_many_keys(self):
# many xattr keys, test buffer growing
# but: make all fit into 4kiB so test won't fail on ext4
self.assert_equal(len(get_buffer()), 2048)
# xattr raw key list will be size 100 * (20 + 1), which is > 2048
keys = ['user.attr%011d' % i for i in range(100)]
for key in keys:
setxattr(self.tmpfile.name, key, b'x')
got_keys = listxattr(self.tmpfile.name)
# buffer did grow:
self.assert_equal(len(get_buffer()), 4096)
# did we get all the keys?
self.assert_equal_se(got_keys, keys)

def test_big_value(self):
# big xattr value, test buffer growing
# but: make all fit into 4kiB so test won't fail on ext4
self.assert_equal(len(get_buffer()), 2048)
value = b'x' * 2049
setxattr(self.tmpfile.name, 'user.big', value)
got_value = getxattr(self.tmpfile.name, 'user.big')
# buffer needed to grow:
self.assert_equal(len(get_buffer()), 4096)
# did we get correct value?
self.assert_equal(value, got_value)
68 changes: 51 additions & 17 deletions borg/xattr.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import subprocess
import sys
import tempfile
import threading
from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_int, c_uint32, get_errno
from ctypes.util import find_library
from distutils.version import LooseVersion
Expand All @@ -14,6 +15,18 @@
logger = create_logger()


def get_buffer(size=None, init=False):
if size is not None:
size = int(size)
if init or len(thread_local.buffer) < size:
thread_local.buffer = create_string_buffer(size)
return thread_local.buffer


thread_local = threading.local()
get_buffer(size=4096, init=True)


def is_enabled(path=None):
"""Determine if xattr is enabled on the filesystem
"""
Expand Down Expand Up @@ -78,9 +91,17 @@ def get_all(path, follow_symlinks=True):
raise Exception(msg)


class BufferTooSmallError(Exception):
"""the buffer given to an xattr function was too small for the result"""


def _check(rv, path=None):
if rv < 0:
raise OSError(get_errno(), path)
e = get_errno()
if e == errno.ERANGE:
raise BufferTooSmallError
else:
raise OSError(e, path)
return rv

if sys.platform.startswith('linux'): # pragma: linux only
Expand All @@ -106,14 +127,20 @@ def listxattr(path, *, follow_symlinks=True):
func = libc.listxattr
else:
func = libc.llistxattr
n = _check(func(path, None, 0), path)
if n == 0:
return []
namebuf = create_string_buffer(n)
n2 = _check(func(path, namebuf, n), path)
if n2 != n:
raise Exception('listxattr failed')
return [os.fsdecode(name) for name in namebuf.raw.split(b'\0')[:-1] if not name.startswith(b'system.posix_acl_')]
size = len(get_buffer())
while True:
buf = get_buffer(size)
try:
n = _check(func(path, buf, size), path)
except BufferTooSmallError:
size *= 2
else:
if n == 0:
return []
names = buf.raw[:n]
return [os.fsdecode(name)
for name in names.split(b'\0')[:-1]
if not name.startswith(b'system.posix_acl_')]

def getxattr(path, name, *, follow_symlinks=True):
name = os.fsencode(name)
Expand All @@ -125,14 +152,17 @@ def getxattr(path, name, *, follow_symlinks=True):
func = libc.getxattr
else:
func = libc.lgetxattr
n = _check(func(path, name, None, 0))
if n == 0:
return
valuebuf = create_string_buffer(n)
n2 = _check(func(path, name, valuebuf, n), path)
if n2 != n:
raise Exception('getxattr failed')
return valuebuf.raw
size = len(get_buffer())
while True:
buf = get_buffer(size)
try:
n = _check(func(path, name, buf, size), path)
except BufferTooSmallError:
size *= 2
else:
if n == 0:
return
return buf.raw[:n]

def setxattr(path, name, value, *, follow_symlinks=True):
name = os.fsencode(name)
Expand Down Expand Up @@ -172,6 +202,7 @@ def listxattr(path, *, follow_symlinks=True):
func = libc.flistxattr
elif not follow_symlinks:
flags = XATTR_NOFOLLOW
# TODO: fix, see linux
n = _check(func(path, None, 0, flags), path)
if n == 0:
return []
Expand All @@ -191,6 +222,7 @@ def getxattr(path, name, *, follow_symlinks=True):
func = libc.fgetxattr
elif not follow_symlinks:
flags = XATTR_NOFOLLOW
# TODO: fix, see linux
n = _check(func(path, name, None, 0, 0, flags))
if n == 0:
return
Expand Down Expand Up @@ -244,6 +276,7 @@ def listxattr(path, *, follow_symlinks=True):
func = libc.extattr_list_file
else:
func = libc.extattr_list_link
# TODO: fix, see linux
n = _check(func(path, ns, None, 0), path)
if n == 0:
return []
Expand All @@ -269,6 +302,7 @@ def getxattr(path, name, *, follow_symlinks=True):
func = libc.extattr_get_file
else:
func = libc.extattr_get_link
# TODO: fix, see linux
n = _check(func(path, EXTATTR_NAMESPACE_USER, name, None, 0))
if n == 0:
return
Expand Down

0 comments on commit a6de9fc

Please sign in to comment.