Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 27 Jun 2014 16:06:53 -0700
From:      Xin Li <delphij@delphij.net>
To:        Jilles Tjoelker <jilles@stack.nl>, Xin LI <delphij@FreeBSD.org>
Cc:        FreeBSD Current <freebsd-current@freebsd.org>, "Kenneth D. Merry" <ken@freebsd.org>
Subject:   Re: svn commit: r267977 - head/bin/mv
Message-ID:  <53ADF90D.2030707@delphij.net>
In-Reply-To: <20140627222323.GA43131@stack.nl>
References:  <201406271957.s5RJvs6j074326@svn.freebsd.org> <20140627222323.GA43131@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help 
This is a multi-part message in MIME format.
--------------020002050206020306060303
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

[moving discussion to freebsd-current@]

On 06/27/14 15:23, Jilles Tjoelker wrote:
> On Fri, Jun 27, 2014 at 07:57:54PM +0000, Xin LI wrote:
>> Author: delphij
>> Date: Fri Jun 27 19:57:54 2014
>> New Revision: 267977
>> URL: http://svnweb.freebsd.org/changeset/base/267977
> 
>> Log:
>>   Always set UF_ARCHIVE on target (because they are by definition new files
>>   and should be archived) and ignore error when we can't set it (e.g. NFS).
> 
>>   Reviewed by:	ken
>>   MFC after:	2 weeks
> 
>> Modified:
>>   head/bin/mv/mv.c
> 
>> Modified: head/bin/mv/mv.c
>> ==============================================================================
>> --- head/bin/mv/mv.c	Fri Jun 27 19:50:30 2014	(r267976)
>> +++ head/bin/mv/mv.c	Fri Jun 27 19:57:54 2014	(r267977)
>> @@ -337,8 +337,8 @@ err:		if (unlink(to))
>>  	 * on a file that we copied, i.e., that we didn't create.)
>>  	 */
>>  	errno = 0;
>> -	if (fchflags(to_fd, sbp->st_flags))
>> -		if (errno != EOPNOTSUPP || sbp->st_flags != 0)
>> +	if (fchflags(to_fd, sbp->st_flags | UF_ARCHIVE))
>> +		if (errno != EOPNOTSUPP || ((sbp->st_flags & ~UF_ARCHIVE) != 0))
>>  			warn("%s: set flags (was: 0%07o)", to, sbp->st_flags);
>>  
>>  	tval[0].tv_sec = sbp->st_atime;
> 
> The part ignoring failures to set UF_ARCHIVE is OK. However, it seems
> inconsistent to set UF_ARCHIVE on a cross-filesystem mv of a single
> file, but not on a cross-filesystem mv of a directory tree or a file
> newly created via shell output redirection.
> 
> If UF_ARCHIVE is supposed to be set automatically, I think this should
> be done in the kernel, like msdosfs already does. However, I'm not sure
> this is actually a useful feature: backup programs are smarter than an
> archive attribute these days.

The flag is supposed to be set automatically (as my understanding of the
ZFS portion of implementation).

However in order to implement that way, we will have to stat() the
target file (attached).  Personally, I think this is a little bit
wasteful, but it would probably something that we have to do if we
implement a switch to turn off automatic UF_ARCHIVE behavior.

Cheers
-- 
Xin LI <delphij@delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die

--------------020002050206020306060303
Content-Type: text/plain; charset=UTF-8;
 name="mv.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="mv.diff"

Index: bin/mv/mv.c
===================================================================
--- bin/mv/mv.c	(revision 267984)
+++ bin/mv/mv.c	(working copy)
@@ -278,6 +278,7 @@ fastcopy(const char *from, const char *to, struct
 	static char *bp = NULL;
 	mode_t oldmode;
 	int nread, from_fd, to_fd;
+	struct stat to_sb;
 
 	if ((from_fd = open(from, O_RDONLY, 0)) < 0) {
 		warn("fastcopy: open() failed (from): %s", from);
@@ -329,6 +330,7 @@ err:		if (unlink(to))
 	 */
 	preserve_fd_acls(from_fd, to_fd, from, to);
 	(void)close(from_fd);
+
 	/*
 	 * XXX
 	 * NFS doesn't support chflags; ignore errors unless there's reason
@@ -336,10 +338,19 @@ err:		if (unlink(to))
 	 * if the server supports flags and we were trying to *remove* flags
 	 * on a file that we copied, i.e., that we didn't create.)
 	 */
-	errno = 0;
-	if (fchflags(to_fd, sbp->st_flags | UF_ARCHIVE))
-		if (errno != EOPNOTSUPP || ((sbp->st_flags & ~UF_ARCHIVE) != 0))
-			warn("%s: set flags (was: 0%07o)", to, sbp->st_flags);
+	if (fstat(to_fd, &to_sb) == 0) {
+		if ((sbp->st_flags  & ~UF_ARCHIVE) !=
+		    (to_sb.st_flags & ~UF_ARCHIVE)) {
+			errno = 0;
+			if (fchflags(to_fd,
+			    sbp->st_flags | (to_sb.st_flags & UF_ARCHIVE)))
+				if (errno != EOPNOTSUPP ||
+				    ((sbp->st_flags & ~UF_ARCHIVE) != 0))
+					warn("%s: set flags (was: 0%07o)",
+					    to, sbp->st_flags);
+		}
+	} else
+		warn("%s: can not stat", to);
 
 	tval[0].tv_sec = sbp->st_atime;
 	tval[1].tv_sec = sbp->st_mtime;

--------------020002050206020306060303--

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53ADF90D.2030707>