[Virtio-fs] [PATCH v2 0/2] virtiofsd: Fix xattr and ACL

[misono.tomohiro at fujitsu.com]

> On Tue, Jan 28, 2020 at 07:18:17PM +0900, Misono Tomohiro wrote:
> > Hi,
> >
> > This is a second version of xattr fix for virtiofsd.
> > I included ACL fix (which introduces new option posix_acl) in this
> > version too as ACL mostly depends on xattr.
> >
> > I run xfstests with XFS backend using "-o xattr -o posix_acl" option
> > and only new failure is generic/375 which checks if sgid bit is
> > cleared after setfacl. I'll try to investigate it.
> >
> > change in v1 -> v2
> >  - rebased to current dev branch
> >
> >  - Always chdir for xattr (1st patch)
> >    In v1, I keep current implementation for regular file/dir since it
> >    show better performance in my environment. But I notice opening file
> >    for xattr causes seek sanity test fails (xfstest generic/285, 436).
> >
> >    I'm not sure what is the fundamental problem here but I believe
> >    performance can be improved by introducing some caching mechanism
> >    in general.
> 
> Hi Misono,
> 
> How much is performance degradation due to fchdir(). If it is significant, then I will be inclined to keep original code for dir/file
> till some other mechanism is introduced to offset the perofrmance loss.

Please refer this replay: https://www.redhat.com/archives/virtio-fs/2020-January/msg00063.html

> 
> > So I change the code to always fchdir to avoid the
> >    problem for now. This results in simpler code too.
> >
> >  - Add ACL fix (2nd patch)
> >    ACL mostly works if xattr option is enabled.
> >    To support default ACL behavior, add new option posix_acl which
> >    handles umasking in virtiofsd instead of guest kernel.
> 
> Ideally I would preferred ACL changes in a separapte patch series. We had been discussing xattr related changes and it will be
> easier to test and review xattr changes and then followed by ACL changes. 

One motivation I included ACL fix comes from the fact that -o xattr option also enables ACL.

> Anyway, now you have already posted these changes in smae patch series, I will review it.
> But if you happen to post another patch series, then lets separate out xattr changes and ACL changes in separate patch series.

Thanks, If it is better I will do it next version.
(btw, I take a leave next week so my response will be delayed) 

Thanks for all the comments!
Misono

> 
> Thanks
> Vivek
> 
> >
> > Thanks,
> > Misono
> >
> > Misono Tomohiro (2):
> >   virtiofsd: Fix xattr operations
> >   virtiofsd: Add support of posix_acl
> >
> >  tools/virtiofsd/fuse_virtio.c    |  13 +++
> >  tools/virtiofsd/helper.c         |   3 +
> >  tools/virtiofsd/passthrough_ll.c | 136 +++++++++++++++++++++----------
> >  tools/virtiofsd/seccomp.c        |  11 ++-
> >  4 files changed, 115 insertions(+), 48 deletions(-)
> >
> > --
> > 2.21.1
> >