Browse Source

Merge pull request #7008 from KN4CK3R/forwardport-6990

xattrs / extended stat: improve exception handling (master)
TW 2 years ago
parent
commit
c258eb45f4
3 changed files with 27 additions and 37 deletions
  1. 12 8
      src/borg/archive.py
  2. 3 3
      src/borg/testsuite/archiver.py
  3. 12 26
      src/borg/xattr.py

+ 12 - 8
src/borg/archive.py

@@ -1141,15 +1141,19 @@ class MetadataCollector:
 
     def stat_ext_attrs(self, st, path, fd=None):
         attrs = {}
-        with backup_io("extended stat"):
-            flags = 0 if self.noflags else get_flags(path, st, fd=fd)
-            xattrs = {} if self.noxattrs else xattr.get_all(fd or path, follow_symlinks=False)
-            if not self.noacls:
+        if not self.noflags:
+            with backup_io("extended stat (flags)"):
+                flags = get_flags(path, st, fd=fd)
+            if flags:
+                attrs["bsdflags"] = flags
+        if not self.noxattrs:
+            with backup_io("extended stat (xattrs)"):
+                xattrs = xattr.get_all(fd or path, follow_symlinks=False)
+            if xattrs:
+                attrs["xattrs"] = StableDict(xattrs)
+        if not self.noacls:
+            with backup_io("extended stat (ACLs)"):
                 acl_get(path, attrs, st, self.numeric_ids, fd=fd)
-        if xattrs:
-            attrs["xattrs"] = StableDict(xattrs)
-        if flags:
-            attrs["bsdflags"] = flags
         return attrs
 
     def stat_attrs(self, st, path, fd=None):

+ 3 - 3
src/borg/testsuite/archiver.py

@@ -1570,7 +1570,7 @@ class ArchiverTestCase(ArchiverTestCaseBase):
             assert xattr.getxattr(b"input/file", b"security.capability") == capabilities
 
     @pytest.mark.skipif(
-        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of" "fakeroot"
+        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of fakeroot"
     )
     def test_extract_xattrs_errors(self):
         def patched_setxattr_E2BIG(*args, **kwargs):
@@ -3702,7 +3702,7 @@ id: 2 / e29442 3506da 4e1ea7 / 25f62a 5a3d41 - 02
 
     # derived from test_extract_xattrs_errors()
     @pytest.mark.skipif(
-        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of" "fakeroot"
+        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of fakeroot"
     )
     def test_do_not_fail_when_percent_is_in_xattr_name(self):
         """https://github.com/borgbackup/borg/issues/6063"""
@@ -3720,7 +3720,7 @@ id: 2 / e29442 3506da 4e1ea7 / 25f62a 5a3d41 - 02
 
     # derived from test_extract_xattrs_errors()
     @pytest.mark.skipif(
-        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of" "fakeroot"
+        not xattr.XATTR_FAKEROOT, reason="xattr not supported on this system or on this version of fakeroot"
     )
     def test_do_not_fail_when_percent_is_in_file_name(self):
         """https://github.com/borgbackup/borg/issues/6063"""

+ 12 - 26
src/borg/xattr.py

@@ -77,22 +77,14 @@ def get_all(path, follow_symlinks=False):
                 # xattr name is a bytes object, we directly use it.
                 result[name] = getxattr(path, name, follow_symlinks=follow_symlinks)
             except OSError as e:
-                name_str = name.decode()
-                if isinstance(path, int):
-                    path_str = "<FD %d>" % path
-                else:
-                    path_str = os.fsdecode(path)
-                if e.errno == ENOATTR:
-                    # if we get ENOATTR, a race has happened: xattr names were deleted after list.
-                    # we just ignore the now missing ones. if you want consistency, do snapshots.
+                # note: platform.xattr._check has already made a nice exception e with errno, msg, path/fd
+                if e.errno in (ENOATTR,):  # errors we just ignore silently
+                    # ENOATTR: a race has happened: xattr names were deleted after list.
                     pass
-                elif e.errno == errno.EPERM:
-                    # we were not permitted to read this attribute, still can continue trying to read others
-                    logger.warning(
-                        "{}: Operation not permitted when reading extended attribute {}".format(path_str, name_str)
-                    )
-                else:
-                    raise
+                else:  # all others: warn, skip this single xattr name, continue processing other xattrs
+                    # EPERM: we were not permitted to read this attribute
+                    # EINVAL: maybe xattr name is invalid or other issue, #6988
+                    logger.warning("when getting extended attribute %s: %s", name.decode(errors="replace"), str(e))
     except OSError as e:
         if e.errno in (errno.ENOTSUP, errno.EPERM):
             # if xattrs are not supported on the filesystem, we give up.
@@ -122,24 +114,18 @@ def set_all(path, xattrs, follow_symlinks=False):
         try:
             setxattr(path, k, v, follow_symlinks=follow_symlinks)
         except OSError as e:
+            # note: platform.xattr._check has already made a nice exception e with errno, msg, path/fd
             warning = True
-            k_str = k.decode()
-            if isinstance(path, int):
-                path_str = "<FD %d>" % path
-            else:
-                path_str = os.fsdecode(path)
             if e.errno == errno.E2BIG:
-                err_str = "too big for this filesystem"
-            elif e.errno == errno.ENOTSUP:
-                err_str = "xattrs not supported on this filesystem"
+                err_str = "too big for this filesystem (%s)" % str(e)
             elif e.errno == errno.ENOSPC:
                 # ext4 reports ENOSPC when trying to set an xattr with >4kiB while ext4 can only support 4kiB xattrs
                 # (in this case, this is NOT a "disk full" error, just a ext4 limitation).
-                err_str = "no space left on device [xattr len = %d]" % (len(v),)
+                err_str = "fs full or xattr too big? [xattr len = %d] (%s)" % (len(v), str(e))
             else:
                 # generic handler
                 # EACCES: permission denied to set this specific xattr (this may happen related to security.* keys)
                 # EPERM: operation not permitted
-                err_str = os.strerror(e.errno)
-            logger.warning("%s: when setting extended attribute %s: %s", path_str, k_str, err_str)
+                err_str = str(e)
+            logger.warning("when setting extended attribute %s: %s", k.decode(errors="replace"), err_str)
     return warning