Skip site navigation (1)Skip section navigation (2)
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>