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

[Bruce Evans]

[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 at freefall:~/zmsun> cp -p $l/msun/Makefile .
> old> % cp: chflags: ./Makefile: Operation not supported
> old> % pts/29:bde at freefall:~/zmsun> echo $?
> old> % 1
> old> % pts/29:bde at 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 at 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