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 12, 2016
1 parent a951d23 commit 17c77a5
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 18 deletions.
22 changes: 21 additions & 1 deletion borg/testsuite/xattr.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
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


Expand Down Expand Up @@ -38,3 +38,23 @@ 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_listxattr_buffer_growth(self):
# make it work even with ext4, which imposes rather low limits
get_buffer(size=64, init=True)
# xattr raw key list will be size 9 * (10 + 1), which is > 64
keys = ['user.attr%d' % i for i in range(9)]
for key in keys:
setxattr(self.tmpfile.name, key, b'x')
got_keys = listxattr(self.tmpfile.name)
self.assert_equal_se(got_keys, keys)
self.assert_equal(len(get_buffer()), 128)

def test_getxattr_buffer_growth(self):
# make it work even with ext4, which imposes rather low limits
get_buffer(size=64, init=True)
value = b'x' * 126
setxattr(self.tmpfile.name, 'user.big', value)
got_value = getxattr(self.tmpfile.name, 'user.big')
self.assert_equal(value, got_value)
self.assert_equal(len(get_buffer()), 128)
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 17c77a5

Please sign in to comment.