lsattr is broken

[ryao]

lsattr and python's xattr.list() use an ioctl instead of the VFS call, which is not currently supported:

https://github.com/zfsonlinux/zfs/blob/master/module/zfs/zpl_file.c#L480

The result is that anything using it breaks. This includes Gentoo Portage's FEATURES=xattr:

https://bugs.gentoo.org/show_bug.cgi?id=483516

[behlendorf]

This is a duplicate #229. File attributes should be relatively easy to get support added for but thus far it hasn't been a priority. There's a prototype patch in #229 which just need to be finalized, the only real hang up is the Solaris<->Linux mapping for the flags.

[ryao]

@behlendorf This is a development blocker for the Pentoo Linux project (run by another Gentoo developer), so I consider fixing this to be a high priority. I promised the Pentoo Linux project that I would write a fix within 24 hours. Making Gentoo Portage happy requires only implementing ZFS_IOC_GETFLAGS, which makes this a proper subset of #229, rather than a duplicate. It would be appropriate to reopen this, but I cannot do that because you closed it.

[lundman]

Unlikely it helps with the Linux version, we had to implement flags in OSX. Only weirdness was that OSX would sometimes set "only flags" when ZFS expects "flags AND mode". We have to look up mode in this case:

https://github.com/zfs-osx/zfs/blob/master/module/zfs/zfs_vnops_osx.c#L572

and Darwins flag mapping

https://github.com/zfs-osx/zfs/blob/master/module/zfs/zfs_vnops_osx_lib.c#L377

[ryao]

I have opened a pull request for the read case, which should be sufficient to make Gentoo Portage's FEATURES=xattr work and anything else that depends on python's xattr.list(). I consider the write case to be a separate issue with lower priority.

[behlendorf]

@ryao The FS_IOC_GETFLAGS implementation looks reasonable and if it resolves an issue you're having with Gentoo Portage by all means pull it in. I may have caused this when I merged 88c283952f0bfeab54612f9ce666601d83c4244f to return the correct error codes. However, I'd prefer to avoid pulling this work in upstream until we have a working get and set. It doesn't look like a lot of work to get there if we just handle the attributes which map directly.

My suggestion would be to do something similar to what @lundman suggests. This is in fact almost all done in the original patch. We should just need to fill in the xvattr for zfs_setattr() and provide the mode bits. The mode bits could be cheaply read from the in memory znode/inode since we have the file pointer.

There are existing test cases in xfstests to verify this was done correctly. @ryao and chance I can persuade you to finish the write side of this as well?

[ryao]

@behlendorf The approach taken in behlendorf/zfs@d80bd252adddae44f465d88650b0022723d3ff5d risks clearing existing attributes that cannot be set by the GNU chattr. It also treats the output of copy_from_user() as an error code when it returns the number of bytes that could not be copied upon error.

I will take some time to do the write case.