Date: Sat, 28 Jun 2014 17:24:02 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Kenneth D. Merry" <ken@freebsd.org> Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Craig Rodrigues <rodrigc@ixsystems.com>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Xin LI <delphij@gmail.com> 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 Message-ID: <20140628165434.A1406@etaplex.bde.org> In-Reply-To: <20140627195201.GA52113@nargothrond.kdm.org> References: <201308212304.r7LN4mr6058450@svn.freebsd.org> <CAGMYy3tDPqOjQxvZ3S0ER%2B5YMNUOpKXpY6NTOtbSV1WtvyPBXg@mail.gmail.com> <20140627195201.GA52113@nargothrond.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140628165434.A1406>