[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 <delp...@delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
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;
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to