From owner-svn-src-head@FreeBSD.ORG Thu Aug 21 04:31:23 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 467085B2; Thu, 21 Aug 2014 04:31:23 +0000 (UTC) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 8CCA932BF; Thu, 21 Aug 2014 04:31:21 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id C6433D4081D; Thu, 21 Aug 2014 14:31:20 +1000 (EST) Date: Thu, 21 Aug 2014 14:31:19 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bruce Evans Subject: Re: svn commit: r254627 - in head: bin/chflags bin/ls lib/libc/gen lib/libc/sys sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/fs/msdosfs sys/fs/smbfs sys/sys sys/ufs/ufs In-Reply-To: <20140628165434.A1406@etaplex.bde.org> Message-ID: <20140821141813.V1394@besplex.bde.org> References: <201308212304.r7LN4mr6058450@svn.freebsd.org> <20140627195201.GA52113@nargothrond.kdm.org> <20140628165434.A1406@etaplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=AOuw8Gd4 c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=mVt93wZpjUsA:10 a=NgjQapKE0a4A:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=PIQYOUdQSiZef2HGeeYA:9 a=CjuIK1q_8ugA:10 Cc: "src-committers@freebsd.org" , Xin LI , "svn-src-all@freebsd.org" , "Kenneth D. Merry" , Craig Rodrigues , "svn-src-head@freebsd.org" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Aug 2014 04:31:23 -0000 [My mail connection wasn't working back in June when I wrote this. This is the first of many replies to try to prevent breakage of mv. I have now checked what happens for simple tests on ref11. Details in later replies.] On Sat, 28 Jun 2014, Bruce Evans wrote: > On Fri, 27 Jun 2014, Kenneth D. Merry wrote: > >> On Fri, Jun 27, 2014 at 12:48:29 -0700, Xin LI wrote: >>> Hi, >>> >>> Craig have hit an interesting issue today, where he tried to 'mv' a file >>> from ZFS dataset to a NFS mount, 'mv' bails out because chflags failed. >>> >>> I think it's probably sensible to have mv ignoring UF_ARCHIVE, and set the >>> flag on the target unconditionally? i.e.: >>> >>> Index: mv.c >>> =================================================================== >>> --- mv.c (revision 267940) >>> +++ mv.c (working copy) >>> @@ -337,8 +337,8 @@ >>> * 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; >> >> Yes, that sounds like a good way to do it. > > No, this is very broken. > > Ignoring the error is bad enough. POSIX requires duplicating all of > the attributes and certain error handling when they cannot be > duplicated. File flags aren't a POSIX attribute, but not duplicating > or handling errors differently for them them breaks the spirit of the > POSIX spec. > > Forcing the archive flag to be set on the copy is worse. It is broken > especially broken if the source and target both support the archive flag, > since it then fails to preserve the flag when it is clear on the source. > > The old code was bad too. I think it usually gives the POSIX behaviour, > but it only applies to the unusual case where only a few regular files > are moved, and its checking if the preservation worked can be done > better by stat()ing the result and comparing with the original. The > usual case (by number of files moved, if not by mv instances), is for > moving whole directory heirarchies. The above code is not used in that > case. cp -pR is used. cp -pR is more buggy than the above in general, > but for the chflags() its error handling is less fancy and thus stricter > than the above, so tends to produce thousands or warnings instead of only > 1. More and different details in another reply. > > I sent the following mail to ken about this (mostly for cp -p instead of > mv) in April, but received no reply: > > old> Copying files on freefall now causes annoying warnings. This is because > old> zfs supports UF_ARCHIVE but nfs doesn't: > old> old> % Script started on Sat Apr 5 05:10:55 2014 > old> % pts/29:bde@freefall:~/zmsun> cp -p $l/msun/Makefile . > old> % cp: chflags: ./Makefile: Operation not supported > old> % pts/29:bde@freefall:~/zmsun> echo $? > old> % 1 > old> % pts/29:bde@freefall:~/zmsun> ls -lo $l/msun/Makefile Makefile > old> % -rw-r--r-- 1 root wheel uarch 8610 Mar 2 11:00 > /usr/src/lib/msun/Makefile > old> % -rw-r--r-- 1 bde devel - 8610 Mar 2 11:00 Makefile > old> % pts/29:bde@freefall:~/zmsun> exit > old> % old> % Script done on Sat Apr 5 05:11:28 2014 > old> old> cp works, but this is hard to determine since the exit status is 1. > Oops, > old> that means that cp doesn't work. It also cannot copy the more important > old> uid and gid, but it doesn't warn about this or change the exit status to > old> 1 for this. Not warning is a historical hack to keep cp usable. Not > old> indicating the error in any other way is not good, but is also > historical. > old> This is only done when chown() returns EPERM. For chflags() on nfs, > we're > old> getting EOPNOTSUPPORT for the whole syscall. > > So cp -pR is completely broken for use by mv for the uid and gid, but works > almost as correctly as possibly for file flags. POSIX has relaxed > requirements for cp relative to mv, since cp without -p is not required > to preserve any attributes, and cp with -p can't be expected to preserve > all the attributes in many cases, unlike the usual case for mv where it > is not across a file system. > > old> The support is useless in practice, at least on freefall, because zfs > old> always sets UF_ARCHIVE and nothing ever clears it. zfs sets it even > old> for directories and symlinks. > old> old> Also, sys/stat.h still has SF_ARCHIVED: > old> old> % #define UF_ARCHIVE 0x00000800 /* file needs to be > archived */ > old> % #define SF_ARCHIVED 0x00010000 /* file is archived */ > old> old> It's not clear what SF_ARCHIVED means, especially since no file > system > old> supports it. The only references to it are: > old> old> % ./fs/tmpfs/tmpfs_subr.c: if ((flags & ~(SF_APPEND | > SF_ARCHIVED | SF_IMMUTABLE | SF_NOUNLINK | > old> % ./ufs/ufs/ufs_vnops.c: if ((vap->va_flags & ~(SF_APPEND | > SF_ARCHIVED | SF_IMMUTABLE | > old> old> So applications can set this flag for ffs and tmpfs, but since the > fs never > old> changes it, it is almost useless. Perhaps we left it for compatibility > old> in ffs. tmpfs doesn't have anything to be compatible with. > old> old> UF_ARCHIVE and UF_NODUMP are fairly bogus for tmpfs too. I think > all they > old> do is prevent the above error when copying files from fs's that support > old> them. > old> old> Bruce > > Attributes like file times cannot be preserved in general, and the > checks for this are both too strict and too weak. Too strict because > it is impossible to preserve file times in general (utimes() cannot > even ask to preserve sub-microsecond resolution. POSIX file systems > are only required to support seconds resolution. FreeBSD supports > some non-POSIX file systems that have worse than seconds resolution). > Too weak because it is impossible to ask for strict preservation. > Syscalls silently change times to something that they can represent > or write. Utilities don't check if the requested settings were > actually made. They should check, but then there are problems > handling errors and complications configuring the utilities to do > the sloppy preservation that they historically did. File times are > only a relatively simple unimportant case. > > Bruce Bruce