On Wed, Aug 21, 2024 at 02:57:56PM +0200, Christoph Hellwig wrote: > On Wed, Aug 21, 2024 at 08:44:31AM -0400, Brian Foster wrote: > > > + error = xfs_reflink_unshare(XFS_I(inode), offset, len); > > > + if (error) > > > + return error; > > > + > > > > Doesn't unshare imply alloc? > > Yes, ooks like that got lost and no test noticed it > > > > - if (xfs_file_sync_writes(file)) > > > + if (!error && xfs_file_sync_writes(file)) > > > error = xfs_log_force_inode(ip); > > > > I'd think if you hit -ENOSPC or something after doing a partial alloc to > > a sync inode, you'd still want to flush the changes that were made..? > > Persistence behavior on error is always undefined. And that's also > what the current code does, as it jumps past the log force from all > error exits. > Ok, if this preserves existing behavior then I'm not too worried about it. Thanks. Brian