On 11/30/21 4:37 AM, Miklos Szeredi wrote:
On Tue, 30 Nov 2021 at 06:21, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
On Mon, Nov 29, 2021 at 11:33 PM Eric Sandeen <sandeen@xxxxxxxxxxx> wrote:
On 11/26/21 9:56 AM, Paolo Bonzini wrote:
Hi all,
I have reached the following ASSERT today running a kernel from
git commit 5d9f4cf36721:
/*
* If we are doing a whiteout operation, allocate the whiteout inode
* we will be placing at the target and ensure the type is set
* appropriately.
*/
if (flags & RENAME_WHITEOUT) {
ASSERT(!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)));
error = xfs_rename_alloc_whiteout(mnt_userns, target_dp, &wip);
if (error)
return error;
/* setup target dirent info as whiteout */
src_name->type = XFS_DIR3_FT_CHRDEV;
}
Hmm. Is our ASSERT correct? rename(2) says:
RENAME_NOREPLACE can't be employed together with RENAME_EXCHANGE.
RENAME_WHITEOUT can't be employed together with RENAME_EXCHANGE.
do_renameat2() does enforce this:
if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
(flags & RENAME_EXCHANGE))
goto put_names;
but our assert seems to check for something different: that neither
NOREPLACE nor EXCHANGE is employed with WHITEOUT. Is that a thinko?
Probably.
RENAME_NOREPLACE and RENAME_WHITEOUT are independent -
The former has to do with the target and enforced by generic vfs.
The latter has to do with the source and is implemented by specific fs.
Overlayfs adds RENAME_WHITEOUT flag is some cases to a rename
before performing it on underlying fs (i.e. xfs) to leave a whiteout instead
of the renamed path, so renameat2(NOREPLACE) on overlayfs could
end up with (RENAME_NOREPLACE | RENAME_WHITEOUT) to xfs.
Agreed, the assert makes no sense.
Thanks,
Miklos
Miklos, Amir - thanks for the confirmation. I'll send a patch.
-Eric
