Make Obnam handle value-less extended attributes

What happens:

* llistxattr returns a list of names
* lgetxattr for one or more names returns ENODATA (ENOATTR)

In other words, the name exists, but has no value. This is
different from it having an empty value. This happens at least
sometimes on a btrfs filesystem, but in my opinion should not
be happening at all. However, since it does, I have changed
Obnam to deal with this by pretending the name does not exist.
It logs a warning, but otherwise it is as if the name had not
been returned by llistxattr.

It would not be acceptable to pretend the value is empty, I
think. An empty value may have some significance that is
different from a value-less name. (I'm happy to change this
if it turns out to be the wrong decision.)

I was unable to reproduce this until Vladimir made me a btrfs
disk image, after which this was easy to debug.

While debugging this, I also realised that lgetxattr in LocalFS
creates an OSError wrongly. Fixed that as well.

Reported-by: Vladimir Elisseev

2 files changed, 30 insertions, 2 deletions

diff --git a/obnamlib/metadata.py b/obnamlib/metadata.py
index be3a46c9..eaca40fb 100644
--- a/obnamlib/metadata.py
+++ b/obnamlib/metadata.py
@@ -14,11 +14,14 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
+import errno
 import grp
+import logging
 import os
 import pwd
 import stat
 import struct
+import tracing
 
 import obnamlib
 
@@ -127,10 +130,35 @@ def _cached_getgrgid(gid): # pragma: no cover
 
 
 def get_xattrs_as_blob(fs, filename): # pragma: no cover
+    tracing.trace('filename=%s' % filename)
     names = fs.llistxattr(filename)
+    tracing.trace('names=%s' % repr(names))
     if not names:
         return None
-    values = [fs.lgetxattr(filename, name) for name in names]
+
+    values = []
+    for name in names[:]:
+        tracing.trace('trying name %s' % repr(name))
+        try:
+            value = fs.lgetxattr(filename, name)
+        except OSError, e:
+            # On btrfs, at least, this can happen: the filesystem returns
+            # a list of attribute names, but then fails when looking up
+            # the value for one or more of the names. We pretend that the
+            # name was never returned in that case.
+            #
+            # Obviously this can happen due to race conditions as well.
+            if e.errno == errno.ENODATA:
+                names.remove(name)
+                logging.warning(
+                    '%s has extended attribute named %s without value, '
+                    'ignoring attribute' % (filename, name))
+            else:
+                raise
+        else:
+            tracing.trace('lgetxattr(%s)=%s' % (name, value))
+            values.append(value)
+    assert len(names) == len(values)
 
     name_blob = ''.join('%s\0' % name for name in names)
 
diff --git a/obnamlib/vfs_local.py b/obnamlib/vfs_local.py
index e4b6b274..9b456f9f 100644
--- a/obnamlib/vfs_local.py
+++ b/obnamlib/vfs_local.py
@@ -168,7 +168,7 @@ class LocalFS(obnamlib.VirtualFileSystem):
     def lgetxattr(self, filename, attrname): # pragma: no cover
         ret = obnamlib._obnam.lgetxattr(self.join(filename), attrname)
         if type(ret) is int:
-            raise OSError((ret, os.strerror(ret), filename))
+            raise OSError(ret, os.strerror(ret), filename)
         return ret
 
     def lsetxattr(self, filename, attrname, attrvalue): # pragma: no cover