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