Hello Pádraig,
thank you for review.
On Tuesday 20 January 2009 11:36:20 Pádraig Brady wrote:
> If SELinux contexts and ACLs are implemented using xattrs,
> does --preserve=xattr copy these also, or does it just copy
> particular namespaces? Worth clarifying in any case.
>
> I'm a bit surprised that -a doesn't copy xattrs actually:
> -a does copy ACLs (but doesn't copy [selinux] context),
> but I guess if xattrs are a superset of SELinux context,
> then that's consistent.
I will find out more information and try to document this behavior. Note
acl/selinux/xattr support can be compiled or not individually in coreutils.
> Also probably not for this patch, but I would also add
> indexes for ACL to mentions of "access control".
> Also there is no description for --preserve=context?
Not yet, maybe subject for another patch.
> Don't we need this?
> #ifndef USE_XATTR
> # define USE_XATTR 0
> #endif
I don't think so. USE_XATTR is defined in both cases by gl_FUNC_XATTR.
> Do we need a require_preserve_xattr?
> I.E. should all these be tristate instead of booleans?
So you want cp to fail on attr_copy_{file,fd} fail with
option --preserve=xattr, but not with option --preserve=all? No problem I
think.
Tristate means enum?
Note the current description of require_preserve_context in copy.h is actually
wrong as the context is not preserved by "cp -a".
> > diff --git a/tests/misc/xattr b/tests/misc/xattr
> > new file mode 100755
> > index 0000000..2f50134
> > --- /dev/null
> > +++ b/tests/misc/xattr
> > @@ -0,0 +1,80 @@
> > +#!/bin/sh
> > +# Ensure that cp --preserve=xattr and mv preserve extended attributes.
>
> we should add a test for `install` to ensure/make obvious
> it doesn't preserve xattrs.
>
> > +# Skip this test if cp was built without xattr support:
> > +grep '^#define USE_XATTR 1' $CONFIG_HEADER > /dev/null ||
> > + skip_test_ "coreutils built without xattr support"
>
> I'd rather test a binary as it's little less coupled I think:
>
> cp --preserve=xattr --help >/dev/null 2>&1 ||
> skip_test_ "coreutils built without xattr support"
Not possible now because --preserve=xattr is not treated as error even if
coreutils is compiled without xattr support. Do you it should be treated as
error?
Kamil
From be354c0cb291f91849e3af4d12737349ecc430d0 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdu...@redhat.com>
Date: Tue, 20 Jan 2009 17:00:26 +0100
Subject: [PATCH] cp/mv: add xattr support
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
This patch was originally written by Andreas Grünbacher, nowadays
available at
http://www.suse.de/~agruen/coreutils/5.91/coreutils-xattr.diff
* bootstrap.conf: Add gnulib module verror.
* m4/xattr.m4: Check for libattr availability, new configure option
--disable-xattr.
* m4/prereq.m4: Require gl_FUNC_XATTR.
* src/Makefile.am: Link cp, mv and ginstall with libattr.
* src/copy.h: Add preserve_xattr to cp_options.
* src/copy.c (copy_attr_error): New function to handle errors during
xattr copying.
(copy_attr_quote): New function to quote file name in error messages
printed by libattr.
(copy_attr_free): Empty function requested by libattr to free quoted
string.
(copy_attr_by_fd): New fd-oriented function to copy xattr.
(copy_attr_by_name): New name-oriented function to copy xattr.
(copy_reg, copy_internal): Call copy_extended_attributes function.
* src/cp.c (usage): Mention new --preserve=xattr option.
(decode_preserve_arg): Handle new --preserve=xattr option.
* src/mv.c: Always attempt to preserve xattr.
* src/install.c: Never attempt to preserve xattr.
* tests/misc/xattr: New test for xattr support in cp, mv and install.
* tests/Makefile.am: Add the new test to list.
* doc/coreutils.texi: Mention xattr support, new --preserve=xattr
option.
* NEWS: Mention the change.
---
NEWS | 6 +++
bootstrap.conf | 2 +-
doc/coreutils.texi | 11 +++++-
m4/prereq.m4 | 1 +
m4/xattr.m4 | 36 +++++++++++++++++
src/Makefile.am | 6 +-
src/copy.c | 78 +++++++++++++++++++++++++++++++++++++
src/copy.h | 4 ++
src/cp.c | 15 ++++++-
src/install.c | 1 +
src/mv.c | 1 +
tests/Makefile.am | 1 +
tests/misc/xattr | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++
13 files changed, 263 insertions(+), 8 deletions(-)
create mode 100644 m4/xattr.m4
create mode 100755 tests/misc/xattr
diff --git a/NEWS b/NEWS
index f1b383e..c9ead42 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,12 @@ GNU coreutils NEWS -*- outline -*-
** New features
+ Add extended attribute support available on certain filesystems like ext2
+ and XFS.
+ cp: Tries to copy xattrs when --preserve=xattr specified
+ mv: Always tries to copy xattrs
+ install: Never copies xattrs
+
cp and mv accept a new option, --no-clobber (-n): silently refrain
from overwriting any existing destination file
diff --git a/bootstrap.conf b/bootstrap.conf
index 1388782..89564fe 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -102,7 +102,7 @@ gnulib_modules="
userspec utimecmp utimens
vasprintf-posix
vc-list-files
- verify version-etc-fsf
+ verify version-etc-fsf verror
warnings
wcwidth winsz-ioctl winsz-termios write-any-file
xalloc
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index d8df107..308b5e6 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7365,7 +7365,7 @@ symbolic links in the destination are always followed if possible.
@itemx @w...@kbd{--preserve}[=@var{attribute_list}]}
@opindex -p
@opindex --preserve
-...@cindex file information, preserving
+...@cindex file information, preserving, extended attributes, xattr
Preserve the specified attributes of the original files.
If specified, the @var{attribute_list} must be a comma-separated list
of one or more of the following strings:
@@ -7392,6 +7392,9 @@ Preserve in the destination files
any links between corresponding source files.
@c Give examples illustrating how hard links are preserved.
@c Also, show how soft links map to hard links with -L and -H.
+...@itemx xattr
+Preserve extended attributes if @command{cp} is built with xattr support,
+and xattrs are supported and enabled on your file system.
@itemx all
Preserve all file attributes.
Equivalent to specifying all of the above.
@@ -7935,6 +7938,9 @@ attributes of destination files. It is typically used in Makefiles to
copy programs into their destination directories. It refuses to copy
files onto themselves.
+...@cindex extended attributes, xattr
+...@command{install} never preserves extended attributes (xattr).
+
The program accepts the following options. Also see @ref{Common options}.
@table @samp
@@ -8083,6 +8089,9 @@ directory succeeded, but the second didn't, the first would be left on
the destination partition and the second and third would be left on the
original partition.
+...@cindex extended attributes, xattr
+...@command{mv} always tries to copy extended attributes (xattr).
+
@cindex prompting, and @command{mv}
If a destination file exists but is normally unwritable, standard input
is a terminal, and the @option{-f} or @option{--force} option is not given,
diff --git a/m4/prereq.m4 b/m4/prereq.m4
index e65682f..536070e 100644
--- a/m4/prereq.m4
+++ b/m4/prereq.m4
@@ -38,6 +38,7 @@ AC_DEFUN([gl_PREREQ],
# handles that; see ../bootstrap.conf.
AC_REQUIRE([gl_EUIDACCESS_STAT])
AC_REQUIRE([gl_FD_REOPEN])
+ AC_REQUIRE([gl_FUNC_XATTR])
AC_REQUIRE([gl_FUNC_XFTS])
AC_REQUIRE([gl_MEMXFRM])
AC_REQUIRE([gl_STRINTCMP])
diff --git a/m4/xattr.m4 b/m4/xattr.m4
new file mode 100644
index 0000000..b8e4c35
--- /dev/null
+++ b/m4/xattr.m4
@@ -0,0 +1,36 @@
+# xattr.m4 - check for Extended Attributes (Linux)
+
+# Copyright (C) 2003, 2008 Free Software Foundation, Inc.
+# This file is free software; the Free Software Foundation
+# gives unlimited permission to copy and/or distribute it,
+# with or without modifications, as long as this notice is preserved.
+
+# Originally written by Andreas Gruenbacher.
+# http://www.suse.de/~agruen/coreutils/5.91/coreutils-xattr.diff
+
+AC_DEFUN([gl_FUNC_XATTR],
+[
+ AC_ARG_ENABLE([xattr],
+ AC_HELP_STRING([--disable-xattr],
+ [turn off support for extended attributes]),
+ [use_xattr=$enableval], [use_xattr=yes])
+
+ if test "$use_xattr" = "yes"; then
+ AC_CHECK_HEADERS([attr/error_context.h attr/libattr.h])
+ if test $ac_cv_header_attr_libattr_h = yes \
+ && test $ac_cv_header_attr_error_context_h = yes; then
+ use_xattr=1
+ else
+ use_xattr=0
+ fi
+ AC_DEFINE_UNQUOTED([USE_XATTR], [$use_xattr],
+ [Define if you want extended attribute support.])
+ xattr_saved_LIBS=$LIBS
+ AC_SEARCH_LIBS([attr_copy_file], [attr],
+ [test "$ac_cv_search_attr_copy_file" = "none required" ||
+ LIB_XATTR=$ac_cv_search_attr_copy_file])
+ AC_CHECK_FUNCS([attr_copy_file])
+ LIBS=$xattr_saved_LIBS
+ AC_SUBST([LIB_XATTR])
+ fi
+])
diff --git a/src/Makefile.am b/src/Makefile.am
index 555700b..907b9e7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -152,9 +152,9 @@ su_LDADD = $(LDADD) $(LIB_CRYPT)
dir_LDADD += $(LIB_ACL)
ls_LDADD += $(LIB_ACL)
vdir_LDADD += $(LIB_ACL)
-cp_LDADD += $(LIB_ACL)
-mv_LDADD += $(LIB_ACL)
-ginstall_LDADD += $(LIB_ACL)
+cp_LDADD += $(LIB_ACL) $(LIB_XATTR)
+mv_LDADD += $(LIB_ACL) $(LIB_XATTR)
+ginstall_LDADD += $(LIB_ACL) $(LIB_XATTR)
stat_LDADD = $(LDADD) $(LIB_SELINUX)
diff --git a/src/copy.c b/src/copy.c
index c9c79a1..7538b2f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -54,6 +54,13 @@
#include "areadlink.h"
#include "yesno.h"
+#if USE_XATTR
+# include <attr/error_context.h>
+# include <attr/libattr.h>
+# include <stdarg.h>
+# include "verror.h"
+#endif
+
#ifndef HAVE_FCHOWN
# define HAVE_FCHOWN false
# define fchown(fd, uid, gid) (-1)
@@ -123,6 +130,70 @@ is_ancestor (const struct stat *sb, const struct dir_list *ancestors)
return false;
}
+#if USE_XATTR
+static void
+copy_attr_error (struct error_context *ctx, char const *fmt, ...)
+{
+ int err = errno;
+ va_list ap;
+
+ /* use verror module to print error message */
+ va_start (ap, fmt);
+ verror(0, err, fmt, ap);
+ va_end (ap);
+}
+
+static char const *
+copy_attr_quote (struct error_context *ctx, char const *str)
+{
+ return quote(str);
+}
+
+static void
+copy_attr_free (struct error_context *ctx, char const *str)
+{
+}
+
+static bool
+copy_attr_by_fd (char const *src_path, int src_fd,
+ char const *dst_path, int dst_fd)
+{
+ struct error_context ctx =
+ {
+ .error = copy_attr_error,
+ .quote = copy_attr_quote,
+ .quote_free = copy_attr_free
+ };
+ return 0 == attr_copy_fd (src_path, src_fd, dst_path, dst_fd, 0, &ctx);
+}
+
+static bool
+copy_attr_by_name (char const *src_path, char const *dst_path)
+{
+ struct error_context ctx =
+ {
+ .error = copy_attr_error,
+ .quote = copy_attr_quote,
+ .quote_free = copy_attr_free
+ };
+ return 0 == attr_copy_file (src_path, dst_path, 0, &ctx);
+}
+#else /* USE_XATTR */
+
+static bool
+copy_attr_by_fd (char const *src_path, int src_fd,
+ char const *dst_path, int dst_fd)
+{
+ return true;
+}
+
+static bool
+copy_attr_by_name (char const *src_path, char const *dst_path)
+{
+ return true;
+}
+#endif /* USE_XATTR */
+
/* Read the contents of the directory SRC_NAME_IN, and recursively
copy the contents to DST_NAME_IN. NEW_DST is true if
DST_NAME_IN is a directory that was created previously in the
@@ -681,6 +752,10 @@ copy_reg (char const *src_name, char const *dst_name,
set_author (dst_name, dest_desc, src_sb);
+ if (x->preserve_xattr && ! copy_attr_by_fd (src_name, source_desc,
+ dst_name, dest_desc))
+ return_val = false;
+
if (x->preserve_mode || x->move_mode)
{
if (copy_acl (src_name, source_desc, dst_name, dest_desc, src_mode) != 0
@@ -1985,6 +2060,9 @@ copy_internal (char const *src_name, char const *dst_name,
set_author (dst_name, -1, &src_sb);
+ if (x->preserve_xattr && ! copy_attr_by_name (src_name, dst_name))
+ delayed_ok = false;
+
if (x->preserve_mode || x->move_mode)
{
if (copy_acl (src_name, -1, dst_name, -1, src_mode) != 0
diff --git a/src/copy.h b/src/copy.h
index 12b7c2d..bc2ca41 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -173,6 +173,10 @@ struct cp_options
fail if it is unable to do so. */
bool require_preserve_context;
+ /* If true, attempt to preserve extended attributes using libattr.
+ Ignored if coreutils are compiled without xattr support. */
+ bool preserve_xattr;
+
/* If true, copy directories recursively and copy special files
as themselves rather than copying their contents. */
bool recursive;
diff --git a/src/cp.c b/src/cp.c
index 191f73e..c350b80 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -187,7 +187,8 @@ Mandatory arguments to long options are mandatory for short options too.\n\
-p same as --preserve=mode,ownership,timestamps\n\
--preserve[=ATTR_LIST] preserve the specified attributes (default:\n\
mode,ownership,timestamps), if possible\n\
- additional attributes: context, links, all\n\
+ additional attributes: context, links, xattr,\n\
+ all\n\
"), stdout);
fputs (_("\
--no-preserve=ATTR_LIST don't preserve the specified attributes\n\
@@ -764,6 +765,7 @@ cp_option_init (struct cp_options *x)
x->preserve_timestamps = false;
x->preserve_security_context = false;
x->require_preserve_context = false;
+ x->preserve_xattr = false;
x->require_preserve = false;
x->recursive = false;
@@ -800,18 +802,20 @@ decode_preserve_arg (char const *arg, struct cp_options *x, bool on_off)
PRESERVE_OWNERSHIP,
PRESERVE_LINK,
PRESERVE_CONTEXT,
+ PRESERVE_XATTR,
PRESERVE_ALL
};
static enum File_attribute const preserve_vals[] =
{
PRESERVE_MODE, PRESERVE_TIMESTAMPS,
- PRESERVE_OWNERSHIP, PRESERVE_LINK, PRESERVE_CONTEXT, PRESERVE_ALL
+ PRESERVE_OWNERSHIP, PRESERVE_LINK, PRESERVE_CONTEXT, PRESERVE_XATTR,
+ PRESERVE_ALL
};
/* Valid arguments to the `--preserve' option. */
static char const* const preserve_args[] =
{
"mode", "timestamps",
- "ownership", "links", "context", "all", NULL
+ "ownership", "links", "context", "xattr", "all", NULL
};
ARGMATCH_VERIFY (preserve_args, preserve_vals);
@@ -852,6 +856,10 @@ decode_preserve_arg (char const *arg, struct cp_options *x, bool on_off)
x->require_preserve_context = on_off;
break;
+ case PRESERVE_XATTR:
+ x->preserve_xattr = on_off;
+ break;
+
case PRESERVE_ALL:
x->preserve_mode = on_off;
x->preserve_timestamps = on_off;
@@ -859,6 +867,7 @@ decode_preserve_arg (char const *arg, struct cp_options *x, bool on_off)
x->preserve_links = on_off;
if (selinux_enabled)
x->preserve_security_context = on_off;
+ x->preserve_xattr = on_off;
break;
default:
diff --git a/src/install.c b/src/install.c
index 9dda05a..9bf9eee 100644
--- a/src/install.c
+++ b/src/install.c
@@ -200,6 +200,7 @@ cp_option_init (struct cp_options *x)
x->open_dangling_dest_symlink = false;
x->update = false;
x->preserve_security_context = false;
+ x->preserve_xattr = false;
x->verbose = false;
x->dest_info = NULL;
x->src_info = NULL;
diff --git a/src/mv.c b/src/mv.c
index a5ab95d..db9207b 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -124,6 +124,7 @@ cp_option_init (struct cp_options *x)
x->preserve_security_context = selinux_enabled;
x->require_preserve = false; /* FIXME: maybe make this an option */
x->require_preserve_context = false;
+ x->preserve_xattr = true;
x->recursive = true;
x->sparse_mode = SPARSE_AUTO; /* FIXME: maybe make this an option */
x->symbolic_link = false;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6dce9cd..66b22f9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -232,6 +232,7 @@ TESTS = \
misc/tty-eof \
misc/unexpand \
misc/uniq \
+ misc/xattr \
chmod/c-option \
chmod/equal-x \
chmod/equals \
diff --git a/tests/misc/xattr b/tests/misc/xattr
new file mode 100755
index 0000000..05c613c
--- /dev/null
+++ b/tests/misc/xattr
@@ -0,0 +1,109 @@
+#!/bin/sh
+# Ensure that cp --preserve=xattr and mv preserve extended attributes and
+# install does not preserve extended attributes.
+
+# Copyright (C) 2009 Free Software Foundation, 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
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ cp --version
+ mv --version
+ ginstall --version
+fi
+
+. $srcdir/test-lib.sh
+
+# Skip this test if cp was built without xattr support:
+grep '^#define USE_XATTR 1' $CONFIG_HEADER > /dev/null ||
+ skip_test_ "coreutils built without xattr support"
+
+# this code was taken from test mv/backup-is-src
+cleanup_() { rm -rf "$other_partition_tmpdir"; }
+. "$abs_srcdir/other-fs-tmpdir"
+b_other="$other_partition_tmpdir/b"
+rm -f $b_other || framework_failure
+
+# testing xattr name-value pair
+xattr_name="user.foo"
+xattr_value="bar"
+xattr_pair="$xattr_name=\"$xattr_value\""
+
+# create new file and check its xattrs
+touch a || framework_failure
+getfattr -d a > out_a || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_a > /dev/null && framework_failure
+
+# try to set user xattr on file
+setfattr -n "$xattr_name" -v "$xattr_value" a > out_a \
+ || skip_test_ "failed to set xattr of file"
+getfattr -d a > out_a || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_a > /dev/null \
+ || skip_test_ "failed to set xattr of file"
+
+fail=0
+
+# cp should not preserve xattr by default
+cp a b || fail=1
+getfattr -d b > out_b || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_b > /dev/null && fail=1
+
+# test if --preserve=xattr option works
+cp --preserve=xattr a b || fail=1
+getfattr -d b > out_b || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_b > /dev/null || fail=1
+
+rm b || framework_failure
+
+# install should never preserve xattr
+ginstall a b || fail=1
+getfattr -d b > out_b || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_b > /dev/null && fail=1
+
+# mv should preserve xattr when renaming within a filesystem.
+# This is implicitly done by rename() and doesn't need explicit
+# xattr support in mv.
+mv a b || fail=1
+getfattr -d b > out_b || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_b > /dev/null || cat >&2 <<EOF
+=================================================================
+$0: WARNING!!!
+rename() does not preserve extended attributes
+=================================================================
+EOF
+
+# try to set user xattr on file on other partition
+test_mv=1
+touch $b_other || framework_failure
+setfattr -n "$xattr_name" -v "$xattr_value" $b_other > out_a || test_mv=0
+getfattr -d $b_other > out_b || test_mv=0
+grep -F "$xattr_pair" out_b > /dev/null || test_mv=0
+rm -f $b_other || framework_failure
+
+if test $test_mv -eq 1; then
+ # mv should preserve xattr when copying content from one partition to another
+ mv b $b_other || fail=1
+ getfattr -d $b_other > out_b || skip_test_ "failed to get xattr of file"
+ grep -F "$xattr_pair" out_b > /dev/null || fail=1
+else
+ cat >&2 <<EOF
+=================================================================
+$0: WARNING!!!
+failed to set xattr of file $b_other
+=================================================================
+EOF
+fi
+
+Exit $fail
--
1.5.4.3
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
