[Virtio-fs] [PATCH v3 0/2] Fix xattr operation

[misono.tomohiro at fujitsu.com]

> On Thu, Feb 20, 2020 at 08:47:02PM +0900, Misono Tomohiro wrote:
> > This fixes the xattr operation for directory and special files (which
> > can be tested by xfstests generic/062 with -o xattr option).
> >
> > The overall logic is switched back to the same as v1 in favor of
> > performance (i.e. keep original implementation for regular
> > files/directories) but I add a cleanup patch to improve readability as requested by Vivek.
> >
> > Known issue is that if xattr enabled, seek sanity tests (generic/285,
> > 436) will fail. However, I understand this is not a very serious bug
> > like data corruption so leave it for now.
> 
> Hi Misono,
> 
> Do you know why generic/285 and generic/436 fail with xattr enabled.  Is it something easily fixable later.

Not sure if it can be fixed easily, but here is what I figured out:

The failure case checks lseek SEEK_DATA/SEEK_HOLE with some unwritten extent (by fallocate).
I notice that open/close affects lseek behavior in that case on xfs: 

# on xfs
 xfs_io> open -ft temp
 xfs_io> falloc 0 40k
 xfs_io> seek -a 0 # show both SEEK_HOLE/SEEK_DATA with offset 0
 Whence  Result
 HOLE    0      # hole only (fallocated block is treated as hole at this time)

# close and open again
 xfs_io> close
 xfs_io> open temp
 xfs_io> seek -a 0
 Whence  Result
 DATA    0       # fallocated block is treated as data now
 HOLE    40960  # hole position is now EOF

I have not yet understand what really causes this in detail (didn't check other fs neither).

Since getxattr operation happens during the test to check "security.capability"
file attribute when writing some data, it seems this behavior affects the test on virtiofs with xfs.

Thanks.
Misono

> 
> >
> > One question; I remove error handling of fchdir() in v3 since I
> > believe fchdir to proc_self_fd/root.fd cannot fail in the situation
> > but should I add error handling?
> 
> I would think its good to have error handling for fchdir() case. We don't expect it, but if something goes wrong, we will like
> to catch it.
> 
> Thanks
> Vivek
> 
> >
> > change v2 -> v3:
> >  - rebased to current dev branch
> >  - add cleanup path (first one) to simplify main patch (second patch)
> >  - restore the logic of v1 in favor of performance
> >    (as a result seek sanity test failure is not fixed by this series)
> >  - remove error handling of fchdir
> >  - drop ACL fix included in v2 for now to focus xattr
> >
> > v2 patch:
> > https://www.redhat.com/archives/virtio-fs/2020-January/msg00131.html
> >
> > Thanks!
> >
> > Misono Tomohiro (2):
> >   virtiofs: passthrough_ll: cleanup getxattr/listxattr
> >   virtiofs: Fix xattr operations
> >
> >  tools/virtiofsd/fuse_virtio.c    |  13 +++
> >  tools/virtiofsd/passthrough_ll.c | 141 +++++++++++++++----------------
> >  tools/virtiofsd/seccomp.c        |   6 ++
> >  3 files changed, 87 insertions(+), 73 deletions(-)
> >
> > --
> > 2.21.1
> >