Re: [RFC] xfs: fix reporting supported extra file attributes for statx()

[Luis R. Rodriguez]

On Tue, Oct 31, 2017 at 3:27 PM, Darrick J. Wong
<darrick.wong@xxxxxxxxxx> wrote:
> On Tue, Oct 31, 2017 at 03:13:05PM -0700, Luis R. Rodriguez wrote:
>> statx(2) notes that any attribute that is not indicated as supported by
>> stx_attributes_mask has no usable value. Commit 5f955f26f3d42d ("xfs: report
>> crtime and attribute flags to statx") added support for informing userspace
>> of extra file attributes but forgot to list these flags as supported
>> making reporting them rather useless for the pedantic userspace author.
>
> Maybe the vfs should WARN_ON(stat->attributes & ~stat->attributes_mask); to
> prevent us from screwing this up further?

Its unclear, what was the mask devised for then, was it envisioned
that there will be filesystems that will set some values but yet
purposely list them as unsupported?

>> $ git describe --contains 5f955f26f3d42d04aba65590a32eb70eedb7f37d
>> v4.11-rc6~5^2^2~2
>>
>> Fixes: 5f955f26f3d42d ("xfs: report crtime and attribute flags to statx")
>> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
>> ---
>>
>> Its unclear why David left these out or if it was just a mistake, I checked
>> the original patch thread [0] but it was not clear in the end. David?
>>
>> Also, I posted a patch to add support to clearing these flags through
>> xfs_repair on symlinks [1] due to the pain this can cause as you have no option
>> but to use xfs_db to fix these otherwise. Since we *didn't* list these extra
>> file attributes as supported, it begs the question if instead we should only
>> set them *and* this mask if !S_ISLNK(inode->i_mode)).
>>
>> If so, that also begs the question if we should add further sanity check
>> on the xfs setattr to ensure these are never set on symbolic links, despite the
>> fact that FS_IOC_FSSETXATTR ioctl won't be able to set them...
>
> Sure.

Alright, I'll let things wind down and if I don't hear otherwise back
will make the change.

  Luis