svn commit: r267977 - head/bin/mv

[Xin Li]

[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 at delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-------------- next part --------------
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;