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? Thanks, Amir.
