Page MenuHomeFreeBSD

Implement AT_RENAME_EXCHANGE flag for VFS and tmpfs
Needs ReviewPublic

Authored by kib on Fri, Jun 19, 9:23 AM.
Tags
None
Referenced Files
F160683556: D57658.id180542.diff
Fri, Jun 26, 6:25 PM
F160667253: D57658.diff
Fri, Jun 26, 3:08 PM
F160653829: D57658.id180251.diff
Fri, Jun 26, 12:38 PM
F160652201: D57658.id180251.diff
Fri, Jun 26, 12:21 PM
F160624893: D57658.id180169.diff
Fri, Jun 26, 7:40 AM
F160624715: D57658.id180045.diff
Fri, Jun 26, 7:38 AM
F160621145: D57658.diff
Fri, Jun 26, 7:04 AM
F160615584: D57658.id180172.diff
Fri, Jun 26, 5:58 AM

Details

Reviewers
markj
jah

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Fri, Jun 19, 9:23 AM

Rumors are that it is not too much work to add ZFS support.

I am sure that UFS is hard due to SU and esp. SU+J. I am adding Kirk in case he is interested.

I promise to handle msdosfs if any of large fs (AKA UFS or ZFS) get the flag.

sys/kern/vfs_syscalls.c
3789–3792

The Linux man pages I looked at seem to indicate EINVAL should be returned if both NOREPLACE and EXCHANGE are set, since they're mutually exclusive. Do we need to be concerned about that level of compatibility? The below check that returns EEXIST will still avoid modifying the target file in that case.

sys/kern/vfs_syscalls.c
3789–3792

It's probably worth mirroring the Linux behaviour here.

kib marked 2 inline comments as done.

The sys/kern/vfs_syscalls.c and sys/sys/fcntl.h changes LGTM with one small note

sys/sys/fcntl.h
263

Perhaps /* Atomically exchange from and to */

kib marked an inline comment as done.

Add doc comment for AT_RENAME_EXCHANGE

Do we have to do something w/ namecache?

Do we have to do something w/ namecache?

It is up to fs. Yes, there is a missing part for tmpfs when namecache is enabled.

Update namecache on rename exchange for tmpfs.

I've applied the patch to my tree and will give it a try in Halifax next week.
For reference, OpenZFS commit that added Linux support: https://github.com/openzfs/zfs/commit/dbf6108b4df92341eea40d0b41792ac16eabc514

Thanks to Air Canada giving me an unexpected few hours in YYZ I had a chance to try this out. One remaining issue w/ the tmpfs support - missing directory timestamp update.

Update atime/mtime for the directories

I now have a kernel panic from an attempt to swap with source missing, e.g.

#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
 
int main(void) {
    close(open("target", O_CREAT | O_WRONLY, 0644));
    renameat2(AT_FDCWD, "does_not_exist", AT_FDCWD, "target", RENAME_EXCHANGE);
}

Handle NULL result from namei(RENAME)

Handle 'option MAC' case for fromnd.
Do not leak fromnd on fvp == NULL.

Does filemon_wrapper_rename() need an update? I guess it should emit two records, one for each direction of the rename, but I'm not sure. cc @sjg @stevek

lib/libsys/rename.2
151

Which error number is returned if the filesystem does not support this? It should be documented.

Are there any command-line utilities in the base system which could make use of this?

sys/fs/tmpfs/tmpfs_vnops.c
984

guarantees

sys/kern/vfs_syscalls.c
3816

Do we need to set WILLBEDIR in the other direction?

kib marked 3 inline comments as done.Wed, Jun 24, 3:43 PM

Does filemon_wrapper_rename() need an update? I guess it should emit two records, one for each direction of the rename, but I'm not sure. cc @sjg @stevek

This thing does not even wrap renameat(2), neither it wraps renameat2(2).

Are there any command-line utilities in the base system which could make use of this?

Might be, but I do not have an example that I can thought of.

lib/libsys/rename.2
151

It is already documented below, in the renameat2() error section. It was done when renameat2() was added.

kib marked an inline comment as done.

Set WILLBEDIR for fromnd of tvp is VDIR.