| Message ID | 53AC84C9.2080106@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Jun 26, 2014 at 03:38:33PM -0500, Eric Sandeen wrote: > +FILE ATTRIBUTES > +--------------- > +The btrfs filesystem supports setting the following file > +attributes the `chattr`(1) utility > +append only (a), no atime updates (A), compressed (c), no copy on write (C), > +no dump (d), synchronous directory updates (d), immutable (i), > +synchronous updates (S), and no compression (X). The formatting is not eye-pleasing. I've spotted a few mistakes: * 'd' is listed twice, for sync directory updates it's 'D' * and 'X' does not mean "no compression" and never has, although I'd like to see a chattr bit for that because we have the corresponding inode bit I've checked your patches, the meaning of 'X' hasn't changed. I took the opportunity and reformated the options: @@ -183,9 +183,24 @@ FILE ATTRIBUTES --------------- The btrfs filesystem supports setting the following file attributes the `chattr`(1) utility -append only (a), no atime updates (A), compressed (c), no copy on write (C), -no dump (d), synchronous directory updates (d), immutable (i), -synchronous updates (S), and no compression (X). + +*a* -- append only + +*A* -- no atime updates + +*c* -- compressed + +*C* -- no copy on write + +*d* -- no dump + +*D* -- synchronous directory updates + +*i* -- immutable + +*S* -- synchronous updates For descriptions of these attribute flags, please refer to the `chattr`(1) man page. --- looks almost the same in the manpage and gives IMO a good overview. For initial patch I'm ok with the descriptions, we can enhance it later with btrfs specifics. Are you ok with the proposed changes? (I don't want to bother with resending for simple changes.) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/27/14, 8:42 AM, David Sterba wrote: > On Thu, Jun 26, 2014 at 03:38:33PM -0500, Eric Sandeen wrote: >> +FILE ATTRIBUTES >> +--------------- >> +The btrfs filesystem supports setting the following file >> +attributes the `chattr`(1) utility >> +append only (a), no atime updates (A), compressed (c), no copy on write (C), >> +no dump (d), synchronous directory updates (d), immutable (i), >> +synchronous updates (S), and no compression (X). > > The formatting is not eye-pleasing. > > I've spotted a few mistakes: > > * 'd' is listed twice, for sync directory updates it's 'D' Crud, sorry about that. > * and 'X' does not mean "no compression" and never has, although I'd > like to see a chattr bit for that because we have the corresponding > inode bit Ok, then I'm not sure what it does mean. Supposedly these flags are supported; via check_flags(), called by setflags(), which I was basing these on: if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \ FS_NOATIME_FL | FS_NODUMP_FL | \ FS_SYNC_FL | FS_DIRSYNC_FL | \ FS_NOCOMP_FL | FS_COMPR_FL | FS_NOCOW_FL)) and the kernel header says that's: #define FS_NOCOMP_FL 0x00000400 /* Don't compress */ chattr(1) says: "compression raw access (X)," and also "The ’X’ attribute is used by the experimental compression patches to indicate that a raw contents of a compressed file can be accessed directly. It currently may not be set or reset using chattr(1), although it can be displayed by lsattr(1)." Hum, ok, so we are starting to go off the rails here, aren't we ;) e2fsprogs has this flag translation: { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, for: #define EXT2_NOCOMPR_FL 0x00000400 /* Access raw compressed data */ and btrfs_ioctl_setflags claims to handle it: if (flags & FS_NOCOMP_FL) { ip->flags &= ~BTRFS_INODE_COMPRESS; ip->flags |= BTRFS_INODE_NOCOMPRESS; ... so hopefully you can understand my confusion? ;) The comment says: * The COMPRESS flag can only be changed by users, while the NOCOMPRESS * flag may be changed automatically if compression code won't make * things smaller. (but doesn't says "may *only* be...") But OTOH, chattr won't ever even *pass* "X" to the fs, will it. So I guess I'm lost. It looks like there's code to handle an incoming "X" but I don't think chattr will send it. Do we ever get an outbound "X" for an opportunistically not-compressed file? If so, maybe that still needs to be specified. Otherwise, yeah, the *format* changes look great, thanks. ;) -Eric > I've checked your patches, the meaning of 'X' hasn't changed. > > I took the opportunity and reformated the options: > > @@ -183,9 +183,24 @@ FILE ATTRIBUTES > --------------- > The btrfs filesystem supports setting the following file > attributes the `chattr`(1) utility > -append only (a), no atime updates (A), compressed (c), no copy on write (C), > -no dump (d), synchronous directory updates (d), immutable (i), > -synchronous updates (S), and no compression (X). > + > +*a* -- append only > + > +*A* -- no atime updates > + > +*c* -- compressed > + > +*C* -- no copy on write > + > +*d* -- no dump > + > +*D* -- synchronous directory updates > + > +*i* -- immutable > + > +*S* -- synchronous updates > > For descriptions of these attribute flags, please refer to the > `chattr`(1) man page. > --- > > looks almost the same in the manpage and gives IMO a good > overview. For initial patch I'm ok with the descriptions, we can enhance it > later with btrfs specifics. > > Are you ok with the proposed changes? (I don't want to bother with > resending for simple changes.) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 09:56:10AM -0500, Eric Sandeen wrote: > > * and 'X' does not mean "no compression" and never has, although I'd > > like to see a chattr bit for that because we have the corresponding > > inode bit > > Ok, then I'm not sure what it does mean. Supposedly these flags are supported; > via check_flags(), called by setflags(), which I was basing these on: > > if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \ > FS_NOATIME_FL | FS_NODUMP_FL | \ > FS_SYNC_FL | FS_DIRSYNC_FL | \ > FS_NOCOMP_FL | FS_COMPR_FL | > FS_NOCOW_FL)) > > and the kernel header says that's: > > #define FS_NOCOMP_FL 0x00000400 /* Don't compress */ Passing this bit directly via ioctl works as expected, but to my knowledge there is no chattr letter allocated for it. > chattr(1) says: "compression raw access (X)," and also "The ’X’ attribute > is used by the experimental compression patches to indicate that a raw > contents of a compressed file can be accessed directly. It currently > may not be set or reset using chattr(1), although it can be displayed by lsattr(1)." > > Hum, ok, so we are starting to go off the rails here, aren't we ;) Yeah. And there's no support for accessing raw compressed data. > e2fsprogs has this flag translation: > { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, > for: > #define EXT2_NOCOMPR_FL 0x00000400 /* Access raw compressed data */ > > and btrfs_ioctl_setflags claims to handle it: > > if (flags & FS_NOCOMP_FL) { > ip->flags &= ~BTRFS_INODE_COMPRESS; > ip->flags |= BTRFS_INODE_NOCOMPRESS; > ... > > so hopefully you can understand my confusion? ;) Oh I do now, but it's ext2 fault :) > The comment says: > > * The COMPRESS flag can only be changed by users, while the NOCOMPRESS > * flag may be changed automatically if compression code won't make > * things smaller. > > (but doesn't says "may *only* be...") And thats IMO right (at least I expect it work like that), the user may set or drop the NOCOMPRESS flag. The comment says that it may appear without user interaction. > But OTOH, chattr won't ever even *pass* "X" to the fs, will it. > > So I guess I'm lost. It looks like there's code to handle an incoming > "X" but I don't think chattr will send it. > > Do we ever get an outbound "X" for an opportunistically not-compressed file? > If so, maybe that still needs to be specified. AFAICS 'X' is not listed among the standard chattr options and chattr.c in e2fsprogs has no support for that. There is lib/e2p/pf.c: { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, but this is used only locally by print_flags. I hope this answers your questions, 'X' has no meaning for btrfs now. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/27/14, 10:30 AM, David Sterba wrote: > On Fri, Jun 27, 2014 at 09:56:10AM -0500, Eric Sandeen wrote: >>> * and 'X' does not mean "no compression" and never has, although I'd >>> like to see a chattr bit for that because we have the corresponding >>> inode bit >> >> Ok, then I'm not sure what it does mean. Supposedly these flags are supported; >> via check_flags(), called by setflags(), which I was basing these on: >> >> if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \ >> FS_NOATIME_FL | FS_NODUMP_FL | \ >> FS_SYNC_FL | FS_DIRSYNC_FL | \ >> FS_NOCOMP_FL | FS_COMPR_FL | >> FS_NOCOW_FL)) >> >> and the kernel header says that's: >> >> #define FS_NOCOMP_FL 0x00000400 /* Don't compress */ > > Passing this bit directly via ioctl works as expected, but to my > knowledge there is no chattr letter allocated for it. it's in the manpage, but as a read-only attribute, i.e. lsattr only. >> chattr(1) says: "compression raw access (X)," and also "The ’X’ attribute >> is used by the experimental compression patches to indicate that a raw >> contents of a compressed file can be accessed directly. It currently >> may not be set or reset using chattr(1), although it can be displayed by lsattr(1)." >> >> Hum, ok, so we are starting to go off the rails here, aren't we ;) > > Yeah. And there's no support for accessing raw compressed data. > >> e2fsprogs has this flag translation: >> { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, >> for: >> #define EXT2_NOCOMPR_FL 0x00000400 /* Access raw compressed data */ >> >> and btrfs_ioctl_setflags claims to handle it: >> >> if (flags & FS_NOCOMP_FL) { >> ip->flags &= ~BTRFS_INODE_COMPRESS; >> ip->flags |= BTRFS_INODE_NOCOMPRESS; >> ... >> >> so hopefully you can understand my confusion? ;) > > Oh I do now, but it's ext2 fault :) Ok but btrfs setflags tries to handle "FS_NOCOMP_FL" - how is that ever set? >> The comment says: >> >> * The COMPRESS flag can only be changed by users, while the NOCOMPRESS >> * flag may be changed automatically if compression code won't make >> * things smaller. >> >> (but doesn't says "may *only* be...") > > And thats IMO right (at least I expect it work like that), the user may > set or drop the NOCOMPRESS flag. The comment says that it may appear > without user interaction. > >> But OTOH, chattr won't ever even *pass* "X" to the fs, will it. >> >> So I guess I'm lost. It looks like there's code to handle an incoming >> "X" but I don't think chattr will send it. >> >> Do we ever get an outbound "X" for an opportunistically not-compressed file? >> If so, maybe that still needs to be specified. > > AFAICS 'X' is not listed among the standard chattr options and chattr.c > in e2fsprogs has no support for that. > > There is > > lib/e2p/pf.c: { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, > > but this is used only locally by print_flags. Right, it's a "read-only" flag for lsattr. > I hope this answers your questions, 'X' has no meaning for btrfs now. The only remaining question is, why does the btrfs setflags interface try to parse it, if nothing sends it? Or if something does send it, what? And where is this all documented? ;) btrfs tries to handle a flag value which is identical to the 'X' flag value, which lsattr/chattr says is readonly... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 10:36:54AM -0500, Eric Sandeen wrote: > >> #define FS_NOCOMP_FL 0x00000400 /* Don't compress */ > > > > Passing this bit directly via ioctl works as expected, but to my > > knowledge there is no chattr letter allocated for it. > > it's in the manpage, but as a read-only attribute, i.e. lsattr only. Oh right, sorry. > >> and btrfs_ioctl_setflags claims to handle it: > >> > >> if (flags & FS_NOCOMP_FL) { > >> ip->flags &= ~BTRFS_INODE_COMPRESS; > >> ip->flags |= BTRFS_INODE_NOCOMPRESS; > >> ... > >> > >> so hopefully you can understand my confusion? ;) > > > > Oh I do now, but it's ext2 fault :) > > Ok but btrfs setflags tries to handle "FS_NOCOMP_FL" - how is that ever set? It handles the bit, but there's no friendly userspace tool for that, just the ioctl. > > AFAICS 'X' is not listed among the standard chattr options and chattr.c > > in e2fsprogs has no support for that. > > > > There is > > > > lib/e2p/pf.c: { EXT2_NOCOMPR_FL, "X", "Compression_Raw_Access" }, > > > > but this is used only locally by print_flags. > > Right, it's a "read-only" flag for lsattr. Got it. > > I hope this answers your questions, 'X' has no meaning for btrfs now. > > The only remaining question is, why does the btrfs setflags interface > try to parse it, if nothing sends it? It's allowed operation to manipulate the bit, I don't have anything better to say. > Or if something does send it, what? Lack of tool support, only via ioctl. > And where is this all documented? ;) Eww, sources? :) > btrfs tries to handle a flag value which is identical to the > 'X' flag value, which lsattr/chattr says is readonly... I'm looking at it from the kernel side, ie what's its meaning of the flag. The chattr tool still lives under the hood of e2fsprogs, but the ioctl interface is inherited to other filesystems (stating the obvious). e2fsprogs/chattr can decide to implement other meaning or new bits more or less freely (eg. there's the new 'N' flag for inlined files that I discovered just today while exploring the 'X' flag). There was a discussion at fsdevel about extending the interface or reworking it completely, I don't know if there's an outcome. From the btrfs side, we have the object properties that make a nice interface for accessing the file attributes in parallel with the chattr tool. The interface is currently underused so it's not possible to manipulate the flags yet. I'd rather move the efforts to finalize this interface than adding single bits of the SETFLAGS ioctl and further extensions of the chattr/lsattr tools. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/27/14, 11:10 AM, David Sterba wrote: > On Fri, Jun 27, 2014 at 10:36:54AM -0500, Eric Sandeen wrote: ... >> btrfs tries to handle a flag value which is identical to the >> 'X' flag value, which lsattr/chattr says is readonly... > > I'm looking at it from the kernel side, ie what's its meaning of the > flag. The chattr tool still lives under the hood of e2fsprogs, but > the ioctl interface is inherited to other filesystems (stating the > obvious). e2fsprogs/chattr can decide to implement other meaning or new > bits more or less freely (eg. there's the new 'N' flag for inlined files > that I discovered just today while exploring the 'X' flag). Yes, the interface originated w/ extN, but has clearly spread to other filesystems, and spread like a weed. ;) It's still the de facto interface, but looking through other filesystems, it's a bit of a disaster. (filesystems specifying inheritance of flags they ignore, for example). > There was a discussion at fsdevel about extending the interface or > reworking it completely, I don't know if there's an outcome. > > From the btrfs side, we have the object properties that make a nice > interface for accessing the file attributes in parallel with the chattr > tool. The interface is currently underused so it's not possible to > manipulate the flags yet. or test the code, despite it being merged. \o/ oh well... > I'd rather move the efforts to finalize this interface than adding > single bits of the SETFLAGS ioctl and further extensions of the > chattr/lsattr tools. ok. In any case, back to the original patch: Your changes look fine. 'X' can't be set, so leave it out. Sorry about the 'd' vs 'D' - and I like the new formatting. Feel free to make those changes. (only nitpick: is 'X' ever reported by lsattr on btrfs? If so, it could/should still be included) ((and a side note: I tried to change the text of the manpage to be "btrfs" not "btrfs-mount" but that somehow broke the build, and I didn't dig a lot further)) -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 11:38:10AM -0500, Eric Sandeen wrote: > > From the btrfs side, we have the object properties that make a nice > > interface for accessing the file attributes in parallel with the chattr > > tool. The interface is currently underused so it's not possible to > > manipulate the flags yet. > > or test the code, despite it being merged. \o/ oh well... The core support for the properties was merged and one example added. Implementing all properties was not required nor requested. There are various types of the properties (simple state bits, bits with runtime behaviour change or props affecting other state of the whole filesystem). I want to implement the user-visible interface right from the beginning, there's my proposal in the list, but I'm currently working on other stuff so it's stalled. The file attributes is independent of that. > > I'd rather move the efforts to finalize this interface than adding > > single bits of the SETFLAGS ioctl and further extensions of the > > chattr/lsattr tools. > > ok. In any case, back to the original patch: Your changes look fine. > 'X' can't be set, so leave it out. Sorry about the 'd' vs 'D' - and I > like the new formatting. Feel free to make those changes. Good, thanks. > (only nitpick: is 'X' ever reported by lsattr on btrfs? If so, > it could/should still be included) I'll take a look and will keep 'X' if it's visible somewher. > ((and a side note: I tried to change the text of the manpage to > be "btrfs" not "btrfs-mount" but that somehow broke the build, and > I didn't dig a lot further)) This is AFAIK requirement of the manpage style used, we can either drop it or change the internal man page name after it's generated. No big deal for now. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 01, 2014 at 07:43:46PM +0200, David Sterba wrote: > > (only nitpick: is 'X' ever reported by lsattr on btrfs? If so, > > it could/should still be included) > > I'll take a look and will keep 'X' if it's visible somewher. For the record, with sufficiently new e2fsprogs 'X' appears if the file has the NOCMPRESS bit set. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/btrfs-mount.txt b/Documentation/btrfs-mount.txt index 4433a78..c38e80a 100644 --- a/Documentation/btrfs-mount.txt +++ b/Documentation/btrfs-mount.txt @@ -3,7 +3,7 @@ btrfs-mount(5) NAME ---- -btrfs-mount - mount options for the btrfs filesystem +btrfs-mount - mount options and supported file attributes for the btrfs filesystem DESCRIPTION ----------- @@ -179,8 +179,20 @@ MOUNT OPTIONS *user_subvol_rm_allowed*:: Allow subvolumes to be deleted by a non-root user. Use with caution. +FILE ATTRIBUTES +--------------- +The btrfs filesystem supports setting the following file +attributes the `chattr`(1) utility +append only (a), no atime updates (A), compressed (c), no copy on write (C), +no dump (d), synchronous directory updates (d), immutable (i), +synchronous updates (S), and no compression (X). + +For descriptions of these attribute flags, please refer to the +`chattr`(1) man page. + SEE ALSO -------- +`chattr`(1), `mkfs.btrfs`(8), `mount`(8), `btrfs`(8)
The chattr(1) manpage suffers from the same problems mount(1) had: many options listed, not kept up to date for various filesystems. I've submitted a manpage update for chattr(1) which says to refer to filesystem-specific manpages for supported attributes; this patch updates btrfs(5) to list the attributes supported by btrfs. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html