Date: Fri, 8 Mar 2013 00:37:15 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Kenneth D. Merry" <ken@FreeBSD.org> Cc: arch@FreeBSD.org, fs@FreeBSD.org Subject: Re: patches to add new stat(2) file flags Message-ID: <20130307222553.P981@besplex.bde.org> In-Reply-To: <20130307000533.GA38950@nargothrond.kdm.org> References: <20130307000533.GA38950@nargothrond.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 6 Mar 2013, Kenneth D. Merry wrote:
> I have attached diffs against head for some additional stat(2) file flags.
>
> The primary purpose of these flags is to improve compatibility with CIFS,
> both from the client and the server side.
> ...
I missed looking at the diffs in my previous reply.
% --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 2013-03-06 14:47:25.987128763 -0700
% @@ -117,6 +117,16 @@
% set the user immutable flag (owner or super-user only)
% .It Cm uunlnk , uunlink
% set the user undeletable flag (owner or super-user only)
% +.It Cm system , usystem
% +set the Windows system flag (owner or super-user only)
This begins unsorting of the list.
It's not just a Windows flag, since it also works in DOS.
"Owner or" is too strict for msdosfs, since files can only have a
single owner so it is controlling access using groups is needed. I
use owner root and group msdosfs for msdosfs mounts. This works for
normal operations like open/read/write, but fails for most attributes
including file flags. msdosfs doesn't support many attributes but
this change is supposed to add support for 3 new file flags so it would
be good if it didn't restrict the support to root.
% +.It Cm sparse , usparse
% +set the sparse file attribute (owner or super-user only)
% +.It Cm offline , uoffline
% +set the offline file attribute (owner or super-user only)
% +.It Cm reparse , ureparse
% +set the Windows reparse point file attribute (owner or super-user only)
% +.It Cm hidden , uhidden
% +set the hidden file attribute (owner or super-user only)
The additions at the end are also internally unsorted.
Previously only "opaque" and "nodump" were unsorted. They are UF flags
sorted with the SF flags, and "no" in "nodump" is not ignored for the
purposes of sorting.
Not having "u" in the old and new UF flag names messes up the sort order
(unless you add futher confusion by ignoring "u" for the purposes of
sorting) and makes it harder to add SF variants of the flags.
% .El
% .Pp
% Putting the letters
% --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c 2013-03-06 15:09:32.842437917 -0700
% @@ -68,7 +68,17 @@
% { "nodump", 1, UF_NODUMP },
% { "noopaque", 0, UF_OPAQUE },
% { "nouunlnk", 0, UF_NOUNLINK },
% - { "nouunlink", 0, UF_NOUNLINK }
% + { "nouunlink", 0, UF_NOUNLINK },
% + { "nosystem", 0, UF_SYSTEM, },
% + { "nousystem", 0, UF_SYSTEM, },
% + { "nosparse", 0, UF_SPARSE, },
% + { "nousparse", 0, UF_SPARSE, },
% + { "nooffline", 0, UF_OFFLINE, },
% + { "nouoffline", 0, UF_OFFLINE, },
% + { "noreparse", 0, UF_REPARSE, },
% + { "noureparse", 0, UF_REPARSE, },
% + { "nohidden", 0, UF_HIDDEN, },
% + { "nouhidden", 0, UF_HIDDEN, }
This is totally disordered too.
The old table was sorted except for "nosnapshot". Another bug is that
"nosnapshot" is supported here (so chflags(1) can show it), but is not
documented in chflags(1).
% };
% #define nmappings (sizeof(mapping) / sizeof(mapping[0]))
%
% --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 2013-03-06 14:49:39.357125717 -0700
% @@ -75,16 +75,49 @@
% Do not dump the file.
% .It Dv UF_IMMUTABLE
% The file may not be changed.
% +Filesystems may use this flag to maintain compatibility with the Windows and
% +CIFS FILE_ATTRIBUTE_READONLY attribute.
% .It Dv UF_APPEND
% The file may only be appended to.
This was already totally disordered. It even has UF's before SF's, although
SF's sort before UF's both lexically and logically, though not in sys/stat.h
or in bits.
The disorder here was apparently copied from the implementation
(sys/stat.h, which is in the order of the bits, which is historical
for binary compatibility) and not cleaned up like chflags(1) and
strttoflags(3).
% .It Dv UF_NOUNLINK
% The file may not be renamed or deleted.
% .It Dv UF_OPAQUE
% The directory is opaque when viewed through a union stack.
% +.It Dv UF_SYSTEM
% +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM attribute.
% +Filesystems in FreeBSD may store and display this flag, but do not provide
% +any special handling when it is set.
More disordered than before...
% +.It Dv UF_HIDDEN
% +The file may be hidden from directory listings at the application's
% +discretion.
% +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute.
Not just Windows...
% .It Dv SF_ARCHIVED
% -The file may be archived.
% +The file has been archived.
% +This flag means the opposite of the Windows and CIFS FILE_ATTRIBUTE_ARCHIVE
% +attribute.
% +That attribute means that the file should be archived, whereas
% +.Dv SF_ARCHIVED
% +means that the file has been archived.
WinXP uses the poor wording "avialable for archiving". This is better.
% +Filesystems in FreeBSD may or may not have special handling for this flag.
% +For instance, ZFS tracks changes to files and will clear this bit when a
% +file is updated.
% +UFS only stores the flag, and relies on the application to change it when
% +needed.
I think that is useless, since changing it is needed whenever the file
changes, and applications can do that (short of running as daemons and
watching for changes).
WinXP seems to not set the flag when attributes change. I think that is
a bug, and FreeBSD msdosfs does set it when attributes are changed (except
for the SF_ARCHIVED attribute itself, and atimes when the atimes are set
automatically). The FreeBSD behaviour is like setting the ctime for any
change and is bug for bug compatible in doing this even for null changes.
FreeBSD doesn't have many attributes to set, but you just added some more.
I think this should be controlled by a mount option so that bug for bug
compatibility with WinDOS is possible.
% .It Dv SF_IMMUTABLE
% The file may not be changed.
% +This flag also indicates that the Windows ans CIFS FILE_ATTRIBUTE_READONLY
% +attribute is set.
s/ans/and/
You only mentioned UF_IMMUTABLE in your general description. Mapping
READONLY to both gives even more confusing semantics, and mapping it to
SF_IMMUTABLE seems too strict since for example it prevents using FreeBSD
to manage the flag at high securelevels.
% --- //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c 2013-03-06 14:49:47.179130318 -0700
% @@ -345,8 +345,17 @@
% vap->va_birthtime.tv_nsec = 0;
% }
% vap->va_flags = 0;
% + /*
% + * The DOS Archive attribute means that a file needs to be
% + * archived. The BSD SF_ARCHIVED attribute means that a file has
% + * been archived. Thus the inversion here.
% + */
No need to document it again. It goes without saying that ARCHIVE
!= ARCHIVED.
% if ((dep->de_Attributes & ATTR_ARCHIVE) == 0)
% vap->va_flags |= SF_ARCHIVED;
% + if (dep->de_Attributes & ATTR_HIDDEN)
% + vap->va_flags |= UF_HIDDEN;
% + if (dep->de_Attributes & ATTR_SYSTEM)
% + vap->va_flags |= UF_SYSTEM;
% vap->va_gen = 0;
% vap->va_blocksize = pmp->pm_bpcluster;
% vap->va_bytes =
% @@ -420,12 +429,21 @@
% if (error)
% return (error);
% }
The permissions check before this is delicate and was broken and is
more broken now. It is still short-circuit to handle setting the
single flag that used to be supported, and is slightly broken for that:
- unprivileged user asks to set ARCHIVE by passing !SF_ARCHIVED. We
allow that, although this may toggle the flag and normal semantics
for SF flags is to not allow toggling.
- unprivileged user asks to clear ARCHIVE by passing SF_ARCHIVED. We
don't allow that. But we should allow preserving ARCHIVE if it is
already clear.
The bug wasn't very important when only 1 flag was supported. Now it
prevents unprivileged users managing the new UF flags if ARCHIVE is
clear. Fortunately, this is the unusual case. Anyway, unprivileged
users can set ARCHIVE by doing some other operation. Even the chflags()
operation should set ARCHIVE and thus allow further chflags()'s that now
keep ARCHIVE set. Except it is very confusing if a chflags() asks for
ARCHIVE to be clear. This request might be just to try to preserve
the current setting and not want it if other things are changed, or
it might be to purposely clear it. Changing it from set to clear should
still be privileged.
See the more complicated permissions check in ffs. It would be safest
to duplicate most of it, to get different permissions checking for the
SF and UF flags. Then decide if we want to keep allowing setting
ARCHIVE without privilege.
% - if (vap->va_flags & ~SF_ARCHIVED)
% + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM))
Style bugs (missing spaces around binary operator '|'). These style
bugs are common in atribute handling for other fs's but were not here.
% return EOPNOTSUPP;
% if (vap->va_flags & SF_ARCHIVED)
% dep->de_Attributes &= ~ATTR_ARCHIVE;
% else if (!(dep->de_Attributes & ATTR_DIRECTORY))
% dep->de_Attributes |= ATTR_ARCHIVE;
The comment before this says that we ignore attmps to set ATTR_ARCHIVED
for directories. However, it is out of date. WinXP allows setting it
and all the new flags for directories, and so do we.
% + if (vap->va_flags & UF_HIDDEN)
% + dep->de_Attributes |= ATTR_HIDDEN;
% + else
% + dep->de_Attributes &= ~ATTR_HIDDEN;
% + if (vap->va_flags & UF_SYSTEM)
% + dep->de_Attributes |= ATTR_SYSTEM;
% + else
% + dep->de_Attributes &= ~ATTR_SYSTEM;
I thought you were adding ATTR_READONLY -> UF_IMMUTABLE here.
The WinXP attrib command (at least on a FAT32 fs) doesn't allow setting
or clearing ARCHIVE (even if it is already set or clear) if any of
HIDDEN, READONLY or SYSTEM is already set and remains set after the
command. Thus the HRS attributes act a bit like immutable flags, but
subtly differently. (ffs has the quite different and worse behaviour
of allowing chflags() irrespective of immutable flags being set before
or after, provided there is enough privilege to change the immutable
flags.) Anyway, they should all give some aspects of immutability.
% +
Style bug (extra blank line).
% dep->de_flag |= DE_MODIFIED;
% }
%
% --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-06 15:05:45.936126193 -0700
% @@ -268,6 +268,22 @@
% #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */
% #define UF_NOUNLINK 0x00000010 /* file may not be removed or renamed */
Old style bugs: inconsistent space instead of tab after #define for the 2
newer definitions.
% /*
% + * These two bits are defined in MacOS X. They are not currently used in
% + * FreeBSD.
% + */
% +#if 0
% +#define UF_COMPRESSED 0x00000020 /* file is compressed */
% +#define UF_TRACKED 0x00000040 /* renames and deletes are tracked */
% +#endif
% +
% +#define UF_SYSTEM 0x00000080 /* Windows system file bit */
% +#define UF_SPARSE 0x00000100 /* sparse file */
% +#define UF_OFFLINE 0x00000200 /* file is offline */
% +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit */
% +/* This is the same as the MacOS X definition of UF_HIDDEN */
% +#define UF_HIDDEN 0x00008000 /* file is hidden */
New style bugs: random spaces/tabs after #define. 1 main comment not
punctuated.
% +
% +/*
% * Super-user changeable flags.
% */
% #define SF_SETTABLE 0xffff0000 /* mask of superuser changeable flags */
% --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-04 17:51:12.000000000 -0700
% --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700
% +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c 2013-03-06 15:06:27.004132409 -0700
% @@ -529,8 +529,9 @@
% }
% if (vap->va_flags != VNOVAL) {
% if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND |
% - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE |
% - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0)
% + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM |
% + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED |
% + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0)
% return (EOPNOTSUPP);
% if (vp->v_mount->mnt_flag & MNT_RDONLY)
% return (EROFS);
The random order is harder to read with more flags.
I think unsupported flags should just be unsupported -- most or all of the
new flags and SF_ARCHIVED too. Most need system support to work. Without
system support you can only copy them from other fs's and then lose them
when utilities like tar don't unsderstand them.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130307222553.P981>