[PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[Eric Sandeen]

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>
---

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)

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[David Sterba]

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.)

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[Eric Sandeen]

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.)
> 

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[David Sterba]

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.

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[Eric Sandeen]

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

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[David Sterba]

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.

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[Eric Sandeen]

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

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[David Sterba]

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.

Re: [PATCH] btrfs-progs: add supported attr flags to btrfs(5)

[David Sterba]

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.