On Fri, Sep 1, 2023 at 8:57 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Fri, Sep 1, 2023 at 1:14 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > On Fri, 1 Sept 2023 at 12:08, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > On Fri, Sep 1, 2023 at 12:59 AM Ruiwen Zhao <ruiwen@xxxxxxxxxx> wrote: > > > > > > > > Thanks for the reply Amir! Really helps. I will try the easy fix (i.e. ignoring ENXIO) and test it. Meanwhile I have two questions: > > > > > > > > 1. Since ENXIO comes from ovl_security_fileattr() trying to open the symlink, I was trying to find who returns ENXIO in the first place. I did some code search on libfuse (https://github.com/libfuse/libfuse), but cannot find ENXIO anywhere. Could it be from the kernel side? (I am trying to use this as a justification of the easy fix.) > > > > > > > > > > I think ENXIO comes from no_open() default ->open() method. > > > > > > > 2. Miklos's commit message says "The reason is that ovl_copy_fileattr() is triggered due to S_NOATIME being > > > > set on all inodes (by fuse) regardless of fileattr." Does that mean `ovl_copy_fileattr` should not be triggered on symlink files but it is, and therefore it is getting the errors like ENXIO and ENOTTY? > > > > > > > > > > Miklos' comment explains why ovl_copy_fileattr() passes the gate: > > > > > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK) { > > > > > > S_NOATIME is an indication that the file MAY have fileattr FS_NOATIME_FL, > > > but in the case of FUSE and NFS, S_NOATIME is there for another unrelated > > > reason. > > > > > > In any case, S_NOATIME on a symlink is never an indication of fileattr, > > > so I think the correct solution is to add the conditions to the gate: > > > > > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK && > > > ((S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode))) { > > > > > > > I don't think this is correct: symlink's atime is updated on readlink > > or following. > > I am not saying that symlink cannot have S_NOATIME, but > i_flags of the symlink have already been copied to ovl inode. > I don't think that symlink can have fileattr, because symlink > cannot be opened for the FS_IOC_SETFLAGS ioctl, so there > is never a reason to call ovl_copy_fileattr() for anything other > than dir or regular file. Right? > Well, not exactly right. Apparently, in ext*/btrfs/f2fs/... and recently also tmpfs, the FS_NOATIME_FL | FS_NODUMP_FL flags are inherited from parent to all file types, so they actually can exist on symlinks and ext4 symlinks created inside FS_NOATIME_FL parent do indeed exhibit S_NOATIME behavior. This is not the case with xfs, which takes care not to copy any flags to special files and symlinks. The thing is that for those fs that inherit NOATIME to symlink: 1. lsattr cannot be used to query the NOATIME flag on symlink 2. chattr cannot be used to remove the NOATIME flag on symlink 3. overlayfs fails to copy up those symlinks While technically, overlayfs can query and set flags directly without opening the symlink, I don't think we should bother with this at all and I think it should be fine if overlayfs only ever copied up fileattr flags for directories and regular files. Here is an xfstest for the regression [1]. It only fails when running fstests with ext4 or one of the other mentioned fs as base fs. Thanks, Amir. [1] https://github.com/amir73il/xfstests/blob/overlayfs-devel/tests/overlay/082
