fuse: Fix getxattr, listxattr calls and add a regression test (RHBZ#691389).

The documentation for the getxattr and listxattr calls is not very
clear and as a result we were always returning something different
from that which the Linux kernel would usually return.

This fixes these calls, at least far enough that both the 'getfattr'
and 'getfacl' programs now work fine on FUSE-mounted filesystems.

Note that SELinux attrs are *not* passed through.  This appears to be
a known bug between SELinux and FUSE.  For more information see:

http://www.spinics.net/lists/selinux/msg09460.html

diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index 5840d5d..ebf9458 100644 (file)
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -90,6 +90,7 @@
   xz
 #endif /* ARCHLINUX */
 
   xz
 #endif /* ARCHLINUX */
 

+acl

 attr
 bash
 binutils
 attr
 bash
 binutils

diff --git a/fuse/Makefile.am b/fuse/Makefile.am
index 7d3b463..1cdb993 100644 (file)
--- a/fuse/Makefile.am
+++ b/fuse/Makefile.am
@@ -1,5 +1,5 @@
 # libguestfs
 # libguestfs

-# Copyright (C) 2009 Red Hat Inc.
+# Copyright (C) 2009-2011 Red Hat Inc.

 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index 6adf140..a094490 100644 (file)
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -1,5 +1,5 @@
 /* guestmount - mount guests using libguestfs and FUSE
 /* guestmount - mount guests using libguestfs and FUSE

- * Copyright (C) 2009-2010 Red Hat Inc.
+ * Copyright (C) 2009-2011 Red Hat Inc.

  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by


@@ -745,19 +745,41 @@ fg_getxattr (const char *path, const char *name, char *value,
     free_attrs = 1;
   }
 
     free_attrs = 1;
   }
 

+  /* Find the matching attribute (index in 'i'). */
+  ssize_t r;

   size_t i;
   size_t i;

-  int r = -ENOATTR;

   for (i = 0; i < xattrs->len; ++i) {
   for (i = 0; i < xattrs->len; ++i) {

-    if (STREQ (xattrs->val[i].attrname, name)) {
-      size_t sz = xattrs->val[i].attrval_len;
-      if (sz > size)
-        sz = size;
-      memcpy (value, xattrs->val[i].attrval, sz);
-      r = 0;
+    if (STREQ (xattrs->val[i].attrname, name))

       break;
       break;

-    }

   }
 
   }
 

+  if (i == xattrs->len) {       /* not found */
+    r = -ENOATTR;
+    goto out;
+  }
+
+  /* The getxattr man page is unclear, but if value == NULL then we
+   * return the space required (the caller then makes a second syscall
+   * after allocating the required amount of space).  If value != NULL
+   * then it's not clear what we should do, but it appears we should
+   * copy as much as possible and return -ERANGE if there's not enough
+   * space in the buffer.
+   */
+  size_t sz = xattrs->val[i].attrval_len;
+  if (value == NULL) {
+    r = sz;
+    goto out;
+  }
+
+  if (sz <= size)
+    r = sz;
+  else {
+    r = -ERANGE;
+    sz = size;
+  }
+  memcpy (value, xattrs->val[i].attrval, sz);
+
+out:

   if (free_attrs)
     guestfs_free_xattr_list ((struct guestfs_xattr_list *) xattrs);
 
   if (free_attrs)
     guestfs_free_xattr_list ((struct guestfs_xattr_list *) xattrs);
 


@@ -781,25 +803,47 @@ fg_listxattr (const char *path, char *list, size_t size)
     free_attrs = 1;
   }
 
     free_attrs = 1;
   }
 

+  /* Calculate how much space is required to hold the result. */
+  size_t space = 0;
+  size_t len;

   size_t i;
   size_t i;

-  ssize_t copied = 0;

   for (i = 0; i < xattrs->len; ++i) {
   for (i = 0; i < xattrs->len; ++i) {

-    size_t len = strlen (xattrs->val[i].attrname) + 1;
+    len = strlen (xattrs->val[i].attrname) + 1;
+    space += len;
+  }
+
+  /* The listxattr man page is unclear, but if list == NULL then we
+   * return the space required (the caller then makes a second syscall
+   * after allocating the required amount of space).  If list != NULL
+   * then it's not clear what we should do, but it appears we should
+   * copy as much as possible and return -ERANGE if there's not enough
+   * space in the buffer.
+   */
+  ssize_t r;
+  if (list == NULL) {
+    r = space;
+    goto out;
+  }
+
+  r = 0;
+  for (i = 0; i < xattrs->len; ++i) {
+    len = strlen (xattrs->val[i].attrname) + 1;

     if (size >= len) {
       memcpy (list, xattrs->val[i].attrname, len);
       size -= len;
       list += len;
     if (size >= len) {
       memcpy (list, xattrs->val[i].attrname, len);
       size -= len;
       list += len;

-      copied += len;
+      r += len;

     } else {
     } else {

-      copied = -ERANGE;
+      r = -ERANGE;

       break;
     }
   }
 
       break;
     }
   }
 

+ out:

   if (free_attrs)
     guestfs_free_xattr_list ((struct guestfs_xattr_list *) xattrs);
 
   if (free_attrs)
     guestfs_free_xattr_list ((struct guestfs_xattr_list *) xattrs);
 

-  return copied;
+  return r;

 }
 
 static int
 }
 
 static int


@@ -881,7 +925,7 @@ usage (int status)
              "  --help               Display help message and exit\n"
              "  --keys-from-stdin    Read passphrases from stdin\n"
              "  --live               Connect to a live virtual machine\n"
              "  --help               Display help message and exit\n"
              "  --keys-from-stdin    Read passphrases from stdin\n"
              "  --live               Connect to a live virtual machine\n"

-             "  -m|--mount dev[:mnt] Mount dev on mnt (if omitted, /)\n"
+             "  -m|--mount dev[:mnt[:opts]] Mount dev on mnt (if omitted, /)\n"

              "  -n|--no-sync         Don't autosync\n"
              "  -o|--option opt      Pass extra option to FUSE\n"
              "  -r|--ro              Mount read-only\n"
              "  -n|--no-sync         Don't autosync\n"
              "  -o|--option opt      Pass extra option to FUSE\n"
              "  -r|--ro              Mount read-only\n"

diff --git a/fuse/test-fuse.sh b/fuse/test-fuse.sh
index 6d2f0fe..0a429f7 100755 (executable)
--- a/fuse/test-fuse.sh
+++ b/fuse/test-fuse.sh
@@ -1,6 +1,6 @@
 #!/bin/bash -
 # libguestfs
 #!/bin/bash -
 # libguestfs

-# Copyright (C) 2009 Red Hat Inc.
+# Copyright (C) 2009-2011 Red Hat Inc.

 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by


@@ -92,15 +92,20 @@ $guestfish <<EOF
   run
   part-disk /dev/sda mbr
   mkfs ext2 /dev/sda1
   run
   part-disk /dev/sda mbr
   mkfs ext2 /dev/sda1

-  mount /dev/sda1 /
+  mount_options acl,user_xattr /dev/sda1 /

   write /hello.txt hello
   write /world.txt "hello world"
   touch /empty
   write /hello.txt hello
   write /world.txt "hello world"
   touch /empty

+  touch /user_xattr
+  setxattr user.test hello123 8 /user_xattr
+  touch /acl
+  # XXX hack until libguestfs gets ACL support
+  debug sh "setfacl -m u:500:r /sysroot/acl" | cat > /dev/null

 EOF
 
 stage Mounting the filesystem
 $guestmount \
 EOF
 
 stage Mounting the filesystem
 $guestmount \

-    -a "$image" -m /dev/sda1 \
+    -a "$image" -m /dev/sda1:/:acl,user_xattr \

     -o uid="$(id -u)" -o gid="$(id -g)" "$mp"
 # To debug guestmount, add this to the end of the preceding command:
 # -v -x & sleep 60
     -o uid="$(id -u)" -o gid="$(id -g)" "$mp"
 # To debug guestmount, add this to the end of the preceding command:
 # -v -x & sleep 60


@@ -223,9 +228,22 @@ world
 bigger
 biggest" ]
 
 bigger
 biggest" ]
 

+stage 'Checking extended attribute (xattr) read operation'
+if getfattr --help > /dev/null 2>&1 ; then
+  [ "$(getfattr -d user_xattr | grep -v ^#)" = 'user.test="hello123"' ]
+fi
+
+stage Checking POSIX ACL read operation
+if getfacl --help > /dev/null 2>&1 ; then
+  [ "$(getfacl -n acl | grep -v ^#)" = "user::rw-
+user:500:r--
+group::r--
+mask::r--
+other::r--" ]
+fi
+

 # These ones are not yet tested by the current script:
 #stage XXX statfs/statvfs
 # These ones are not yet tested by the current script:
 #stage XXX statfs/statvfs

-#stage XXX xattr operations

 
 # These ones cannot easily be tested by the current script, eg because
 # this script doesn't run as root:
 
 # These ones cannot easily be tested by the current script, eg because
 # this script doesn't run as root: