Hello,

Ondřej Vašík wrote: 
> Jim Meyering píše v Pá 17. 04. 2009 v 13:54 +0200:
> > Ondřej Vašík wrote:
> > > as reported in rhbz #496142 by Eric Sandeen, mv produces a lot of
> > > unwanted messages when moving files to filesystem without xattr support.
> > > I guess mv selinux/xattr diagnostic error messages should be reduced as
> > > is done for cp -a, attached patch should do that.
> > ...
> > > +  mv: do not produce diagnostic errors for preserving xattr's to
> > > +  prevent message spam on file systems without xattr support.
> > 
> > Thanks for the patch, but mv's charter is stricter than that of cp -a.
> > I.e., people expect mv to preserve all attributes when the source
> > and destination are on the same file system, so IMHO, failure to
> > preserve any attribute (e.g., for an inter-device mv) requires
> > _some_ diagnosis.

> > However, when there are many files, and mv is failing to preserve some
> > attribute repeatedly, it would make sense to print only the first N
> > such diagnostics, and then for the N+1st, print one more saying that
> > there have been additional attribute-preservation failures, but mv will
> > not display any more.
> 
> Changing proposed patch to not display diagnostics for ENODATA and
> ENOTSUP errno's - as is done for mv and SELinux contexts.

Actually I found that it was not exactly the same - as for
require-preserve=xattr error was hidden even for the failure. This
should be addressed by the first patch.

It reminds me another issue - due to reduce_diagnostic, if you use cp -a
--preserve=xattr,context , failure diagnostic is reduced, but the
command itself fails. Additionally for cp and --require-preserve=context
and ENODATA/ENOTSUP errors on destination filesystem cp just passes
without error - with no context preserved. All those things are
addressed by the second patch.

Greetings,
         Ondřej Vašík
From 3bf0e97d6564ef1aee5a4bf97931884df1b9501a Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email protected]>
Date: Fri, 17 Apr 2009 11:00:35 +0200
Subject: [PATCH] mv: do not produce xattr preserving failures when not supported by filesystem

*src/copy.c: Do not show error diagnostics when xattrs not supported and
   preservation of xattrs is not explicitly required  (reported by
   Eric Sandeen in rhbz #496142)
---
 NEWS       |    3 +++
 src/copy.c |   20 ++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index d8ffde0..063d07f 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   LS_COLORS environment variable. To disable it you can add something like
   this to your profile: eval `dircolors | sed s/hl=[^:]*:/hl=:/`
 
+  mv: do not produce diagnostic errors for preserving xattr's on file
+  systems without xattr support.
+
 
 * Noteworthy changes in release 7.1 (2009-02-21) [stable]
 
diff --git a/src/copy.c b/src/copy.c
index 9b0e139..d9db23c 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -139,6 +139,22 @@ copy_attr_error (struct error_context *ctx ATTRIBUTE_UNUSED,
   int err = errno;
   va_list ap;
 
+  if (errno != ENOTSUP && errno != ENODATA)
+    {
+      /* use verror module to print error message */
+      va_start (ap, fmt);
+      verror (0, err, fmt, ap);
+      va_end (ap);
+    }
+}
+
+static void
+copy_attr_allerror (struct error_context *ctx ATTRIBUTE_UNUSED,
+		 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);
@@ -163,7 +179,7 @@ copy_attr_by_fd (char const *src_path, int src_fd,
 {
   struct error_context ctx =
   {
-    .error = copy_attr_error,
+    .error = x->require_preserve_xattr ? copy_attr_allerror : copy_attr_error,
     .quote = copy_attr_quote,
     .quote_free = copy_attr_free
   };
@@ -177,7 +193,7 @@ copy_attr_by_name (char const *src_path, char const *dst_path,
 {
   struct error_context ctx =
   {
-    .error = copy_attr_error,
+    .error = x->require_preserve_xattr ? copy_attr_allerror : copy_attr_error,
     .quote = copy_attr_quote,
     .quote_free = copy_attr_free
   };
-- 
1.5.6.1.156.ge903b

From 5023cacf5c62c95b1f9c41ef7c7684209a5abc82 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email protected]>
Date: Fri, 24 Apr 2009 14:29:45 +0200
Subject: [PATCH] cp: diagnose failure when preserving xattr/context required even with -a option

* src/copy.c (copy_attr_by_fs): Always show diagnostics when preserving xattrs
is required
             (copy_attr_by_name): Likewise
             (copy_reg): Always show diagnostics when preserving SELinux
context is required
             (copy_internal): Likewise + Do not ignore ENOTSUP and ENODATA
errors when preserving SELinux context is required
* NEWS (Bug fixes): mention it
---
 NEWS       |    5 +++++
 src/copy.c |   16 +++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 063d07f..b3fb475 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   The bug strikes only with both --recursive (-r, -R) and --link (-l).
   [bug introduced in coreutils-7.1]
 
+  cp now correctly diagnoses selinux/xattr failures when option
+  --preserve=context,xattr is specified with combination with option -a.
+  Additionally do not ignore unsupported operation error when preserving
+  SELinux context was explicitly requested.
+
   ls --sort=version (-v) sorted names beginning with "." inconsistently.
   Now, names that start with "." are always listed before those that don't.
 
diff --git a/src/copy.c b/src/copy.c
index d9db23c..d3f3739 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -184,7 +184,8 @@ copy_attr_by_fd (char const *src_path, int src_fd,
     .quote_free = copy_attr_free
   };
   return 0 == attr_copy_fd (src_path, src_fd, dst_path, dst_fd, 0,
-                            x->reduce_diagnostics ? NULL : &ctx);
+                            (x->reduce_diagnostics
+                             && !x->require_preserve_xattr)? NULL : &ctx);
 }
 
 static bool
@@ -198,7 +199,8 @@ copy_attr_by_name (char const *src_path, char const *dst_path,
     .quote_free = copy_attr_free
   };
   return 0 == attr_copy_file (src_path, dst_path, 0,
-                              x-> reduce_diagnostics ? NULL :&ctx);
+                              (x-> reduce_diagnostics
+                               && !x->require_preserve_xattr)? NULL :&ctx);
 }
 #else /* USE_XATTR */
 
@@ -481,7 +483,7 @@ copy_reg (char const *src_name, char const *dst_name,
 	  security_context_t con = NULL;
 	  if (getfscreatecon (&con) < 0)
 	    {
-	      if (!x->reduce_diagnostics)
+	      if (!x->reduce_diagnostics && !x->require_preserve_context)
 	        error (0, errno, _("failed to get file system create context"));
 	      if (x->require_preserve_context)
 		{
@@ -494,7 +496,7 @@ copy_reg (char const *src_name, char const *dst_name,
 	    {
 	      if (fsetfilecon (dest_desc, con) < 0)
 		{
-		  if (!x->reduce_diagnostics)
+		  if (!x->reduce_diagnostics && !x->require_preserve_context)
 		    error (0, errno,
 			   _("failed to set the security context of %s to %s"),
 			   quote_n (0, dst_name), quote_n (1, con));
@@ -1747,7 +1749,7 @@ copy_internal (char const *src_name, char const *dst_name,
 	{
 	  if (setfscreatecon (con) < 0)
 	    {
-	      if (!x->reduce_diagnostics)
+	      if (!x->reduce_diagnostics && !x->require_preserve_context)
 	        error (0, errno,
 		       _("failed to set default file creation context to %s"),
 		       quote (con));
@@ -1761,9 +1763,9 @@ copy_internal (char const *src_name, char const *dst_name,
 	}
       else
 	{
-	  if (errno != ENOTSUP && errno != ENODATA)
+	  if ((errno != ENOTSUP && errno != ENODATA) || x->require_preserve_context)
 	    {
-	      if (!x->reduce_diagnostics)
+	      if (!x->reduce_diagnostics && !x->require_preserve_context)
 	        error (0, errno,
 		       _("failed to get security context of %s"),
 		       quote (src_name));
-- 
1.5.6.1.156.ge903b

Attachment: signature.asc
Description: Toto je digitálně podepsaná část zprávy

_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Reply via email to