On Mon, Jun 10, 2024 at 04:21:39PM +0300, Amir Goldstein wrote: > On Mon, Jun 10, 2024 at 2:50 PM Andrey Albershteyn <aalbersh@xxxxxxxxxx> wrote: > > > > On 2024-06-10 12:19:50, Amir Goldstein wrote: > > > On Mon, Jun 10, 2024 at 11:17 AM Andrey Albershteyn <aalbersh@xxxxxxxxxx> wrote: > > > > > > > > On 2024-06-06 12:27:38, Dave Chinner wrote: > > > ... > > > > > > > > > > The only reason XFS returns -EXDEV to rename across project IDs is > > > > > because nobody wanted to spend the time to work out how to do the > > > > > quota accounting of the metadata changed in the rename operation > > > > > accurately. So for that rare case (not something that would happen > > > > > on the NAS product) we returned -EXDEV to trigger the mv command to > > > > > copy the file to the destination and then unlink the source instead, > > > > > thereby handling all the quota accounting correctly. > > > > > > > > > > IOWs, this whole "-EXDEV on rename across parent project quota > > > > > boundaries" is an implementation detail and nothing more. > > > > > Filesystems that implement project quotas and the directory tree > > > > > sub-variant don't need to behave like this if they can accurately > > > > > account for the quota ID changes during an atomic rename operation. > > > > > If that's too hard, then the fallback is to return -EXDEV and let > > > > > userspace do it the slow way which will always acocunt the resource > > > > > usage correctly to the individual projects. > > > > > > > > > > Hence I think we should just fix the XFS kernel behaviour to do the > > > > > right thing in this special file case rather than return -EXDEV and > > > > > then forget about the rest of it. > > > > > > > > I see, I will look into that, this should solve the original issue. > > > > > > I see that you already got Darrick's RVB on the original patch: > > > https://lore.kernel.org/linux-xfs/20240315024826.GA1927156@frogsfrogsfrogs/ > > > > > > What is missing then? > > > A similar patch for rename() that allows rename of zero projid special > > > file as long as (target_dp->i_projid == src_dp->i_projid)? > > > > > > In theory, it would have been nice to fix the zero projid during the > > > above link() and rename() operations, but it would be more challenging > > > and I see no reason to do that if all the other files remain with zero > > > projid after initial project setup (i.e. if not implementing the syscalls). > > > > I think Dave suggests to get rid of this if-guard and allow > > link()/rename() for special files but with correct quota calculation. > > > > > > > > > > > > > But those special file's inodes still will not be accounted by the > > > > quota during initial project setup (xfs_quota will skip them), would > > > > it worth it adding new syscalls anyway? > > > > > > > > > > Is it worth it to you? > > > > > > Adding those new syscalls means adding tests and documentation > > > and handle all the bugs later. > > > > > > If nobody cared about accounting of special files inodes so far, > > > there is no proof that anyone will care that you put in all this work. > > > > I already have patch and some simple man-pages prepared, I'm > > wondering if this would be useful for any other usecases > > Yes, I personally find it useful. > I have applications that query the fsx_xflags and would rather > be able to use O_PATH to query/set those flags, since > internally in vfs, fileattr_[gs]et() do not really need an open file. > > > which would > > require setting extended attributes on spec indodes. > > Please do not use the terminology "extended attributes" in the man page > to describe struct fsxattr. "XFS file attributes" perhaps? Though that's anachronistic since ext4 supports /some/ of them now. --D > Better follow the "additional attributes" terminology of xfs ioctl man page [1], > even though it is already confusing enough w.r.t "extended attributes" IMO. > > Thanks, > Amir. > > [1] https://man7.org/linux/man-pages/man2/ioctl_xfs_fsgetxattr.2.html >
