linux: renameat2 flags support #12209

cyphar · 2021-06-09T06:27:56Z

Motivation and Context

Being able to support overlayfs on top of ZFS would allow container runtimes to efficiently use ZFS as a container filesystem, with overlayfs used for layering rather than ZFS snapshots. In addition, supporting renameat2 flags allows for userspace programs to make use of RENAME_EXCHANGE and other very useful rename flags (which some distributions have started requiring in their system management scripts).

This PR only handles the renameat2(2) flags portion needed for overlayfs support -- the other part is being tracked by #8774, and requires some more complicated testing revamps in order to be in a mergable state.

Fixes #2256
Ref #8648
Closes #8667
Closes #9414

Description

This is a carry of #9414 (by @snajpa) which was in turn a carry of #8667 (by me). This PR is a pretty significant rewrite of #9414 because the trick implemented by @snajpa (emulating the new rename operations using existing ZIL record types) is not crash-safe and thus turned out to be unsuitable.

How Has This Been Tested?

I tested the original patches like so:

I conducted a variety of smoke-tests to check that all of the renameat(2) flags (and ordinary rename) all operated correctly. After all the runs, I checked that there were no leaks with zdb -bbb. However, I have not tested the error path at all (and I have a feeling this is almost certainly where I've made mistakes).

And @snajpa tested his patches like so:

Tested with vanilla packaged Docker from Docker's repo, however, current Docker
checks for ZFS's magic and then refuses to even consider the overlay2 driver.

So I've been testing with a little cheat patch - vpsfreecz/zfs@6c7873e

The patches have been deployed to @vpsfreecz staging nodes, which revealed some bugs, that have been addressed in the later force-pushes.

Also tested with the @cyphar's renameat2 util and some parallel script-fu.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

module/zfs/zfs_log.c

cyphar · 2021-06-09T10:04:18Z

Because the zfs_revalidate change is still somewhat questionable (see #9600) and seems to warrant implementing a whole new integration testing infrastracture to stress-test rollbacks, I will punt on that particular patch for now -- let's just try to get renameat2 flags support merged first (which is useful on its own) and we can do that separately.

asomers · 2021-06-11T13:40:36Z

Only one test case failed. I've seen it fail before, so I think it's intermittent.

behlendorf · 2021-06-11T15:47:53Z

Right, the receive-o-x_props_override failure is unrelated. @asomers I believe it used to be intermittent, but now we're hitting it reliably on FreeBSD main (and only there). If you be chance have some time to look at it that would be very helpful.

behlendorf · 2021-06-16T20:03:01Z

I've opened openzfs/zfs-buildbot#232 to install rsync on all the test platforms.

I'm a little surprised we weren't already using rsync, it's a completely reasonable addition so I've merged the buildbot PR. It should be available in the CI the next time you refresh this PR

Since mv(1) doesn't expose the RENAME_* flags we need to have our own variation in the tests/ tree. The tests are fairly obvious functional tests, though in the future (once we solve the d_revalidate issue) we might also add a full-stack overlayfs integration test. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

amotin · 2022-10-11T14:43:48Z

We can not require user to do some magic dance before switching the OS. It may be impossible to cleanly export pool on a live system for different reasons.

The "magic dance" we're talking about here is umount (or a clean shutdown, or zfs export if you really want).

I understand. But I am thinking about scenario when user does not have access to the original system any more to do anything. For example old NAS burned in flames and disks were reattached to some other. We do have TrueNAS versions on both FreeBSD and Linux that can import each-other pools. I'd personally accept if the log replay on other OS would be less perfect, since it is an extremely rate scenario, but pool should be importable.

amotin · 2022-10-11T14:49:54Z

Actually, I think most of people would be OK if in case of OS change log replay would just stop on first incompatible record. The damage would probably be smaller.

ghost · 2022-10-11T19:56:02Z

include/sys/zil.h

+	uint64_t	lr_wgid;	/* gid of whiteout */
+	uint64_t	lr_wgen;	/* generation (txg of creation) */
+	uint64_t	lr_wcrtime[2];	/* creation time */
+	uint64_t	lr_wrdev;	/* always makedev(0, 0) */


What's the point of this (and makedev() for that matter) if it's always 0?

I didn't really want to pass a bare 0 to the functions that ask for it, but if you prefer I can just use 0.

Jörgen Lundman on Slack mentioned that macOS has vnop_exchange(), that was never implemented due to needed ZIL changes. For TX_RENAME_EXCHANGE log record format obviously should be the same, but I wonder if any other OS implement TX_RENAME_WHITEOUT, will this be sufficient?

This should be enough. FreeBSD has vop_whiteout() implemented for tmpfs and ufs, but it does not rename, it does LOOKUP or CREATE or DELETE on a whiteout dirent (d_type = DT_WHT) depending on flags. If there was some vop_rename_whiteout() then I think we should not need anything more than what is needed for rename plus to create the whiteout dirent, which is covered by these fields.

I don't mind leaving lr_wrdev in that case.

amotin · 2022-10-11T20:28:37Z

For a note, on the Leadership Meeting call @allanjude has agreed that just stopping ZIL replay on first unsupported record would be acceptable and much less invasive alternative to half-implemented OS-specific features. Nobody else has also objected.

cyphar · 2022-10-12T15:42:31Z

@amotin What does "stopping ZIL replay on the first unsupported record" mean in practice?

Does it mean you get read-write access to a pool that hasn't had its ZIL replayed? I can't see a way that would work which wouldn't lead to some kind of data loss or otherwise bad conditions. Would the remaining ZIL get cleared despite it having entries (meaning data loss) or would you keep it around forever (in which case the log could never be replayed after the pool has been written to because now the log would be replayed on a different state to the one it was written for)? Furthermore if not, how do we append anything to the log? (We can't meaningfully append anything because the log won't get replayed at all on the current machine and thus the ZIL is basically disabled, causing obvious issues.)

If you're okay with a read-only view (which I would assume is the only reasonable way of handing this?) then you'll be happy to know this is already implemented. The feature flags for the new transaction types are tagged as being read-only compatible, meaning you'd be able to import the pool (read-only) without issue.

But maybe I misunderstood and you're talking about making it so that we delegate the read-only decision until we hit an unknown ZIL entry (so effectively baking the feature activate/disactivate logic into ZIL for any new ZIL record types)? And thus remove the feature logic entirely? I'm not sure how well ZFS -- both today and historically -- handles seeing an unknown ZIL entry, but it seems like if this was trivial then it would've been implemented for zilsaxattr. But if that is an option, we could do that (though this does have downsides to using features -- aside from making this "more magical" it has basically the same compatibility issue as the features version but using a mechanism that is not as well-understood and integrated as features).

amotin · 2022-10-12T17:02:31Z

@cyphar I do speak about potentially dropping few last seconds of traffic for specific dataset that used the unsupported by the new OS ZIL records in those few seconds. Sure, it is not great, but if the alternative is getting read-only pool and having to manually copy out the data (with even bigger loss, considering ZIL won't be replayed at all and for all datasets, not for one and after the unknown record, and snapshot for replication can't be created in read-only), then I'll take it.

I don't think it should be any problem to not replay log beyond unsupported record. ZFS already replays the log in best effort basis, stopping any time when checksum of the next block does not match. So it may need some additional polishing and may be some warning message on console that some data loss is expected, but should not be a problem to implement, IMHO.

Either this or implement the log record support for every OS.

ghost · 2022-10-19T18:23:59Z

I've tested this without the feature flags. I used zpool freeze to freeze the zil after getting one started with a sync dd command. Then I created a pair of files, used rename2 -x to exchange them, and exported the pool.
I rebuilt zfs from master with a release configuration, reloaded the modules, and imported the pool. An error message was logged in dmesg but otherwise the pool imported and all datasets mounted:

[ 9455.990640] WARNING: ZFS replay transaction error 22, dataset p/d, seq 0x5, txtype 22

The exchange was of course lost, but that is expected.

The same test with a debug build of master hits an assert at the end of zil_parse().

ghost · 2022-10-19T21:04:07Z

AUTHORS

@@ -19,6 +19,7 @@ CONTRIBUTORS:
    Albert Lee <trisk@nexenta.com>
    Alec Salazar <alec.j.salazar@gmail.com>
    Alejandro R. Sedeño <asedeno@mit.edu>
+    Aleksa Sarai <cyphar@cyphar.com>


This should be added after the next line.

ghost · 2022-10-20T17:51:53Z

The slots for the new TX types should be filled with zvol_replay_err in zvol_replay_vector.

Implement support for Linux's RENAME_* flags (for renameat2). Aside from being quite useful for userspace (providing race-free ways to exchange paths and implement mv --no-clobber), they are used by overlayfs and are thus required in order to use overlayfs-on-ZFS. In order for us to represent the new renameat2(2) flags in the ZIL, we create two new transaction types for the two flags which need transactional-level support (RENAME_EXCHANGE and RENAME_WHITEOUT). RENAME_NOREPLACE does not need any ZIL support because we know that if the operation succeeded before creating the ZIL entry, there was no file to be clobbered and thus it can be treated as a regular TX_RENAME. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Pavel Snajdr <snajpa@snajpa.net> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes #12209 Closes #14070

This is in preparation for RENAME_EXCHANGE and RENAME_WHITEOUT support for ZoL, but the changes here allow for far nicer fallbacks than the previous implementation (the source and target are re-linked in case of the final link failing). In addition, a small cleanup was done for the "target exists but is a different type" codepath so that it's more understandable. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12209 Closes openzfs#14070

Implement support for Linux's RENAME_* flags (for renameat2). Aside from being quite useful for userspace (providing race-free ways to exchange paths and implement mv --no-clobber), they are used by overlayfs and are thus required in order to use overlayfs-on-ZFS. In order for us to represent the new renameat2(2) flags in the ZIL, we create two new transaction types for the two flags which need transactional-level support (RENAME_EXCHANGE and RENAME_WHITEOUT). RENAME_NOREPLACE does not need any ZIL support because we know that if the operation succeeded before creating the ZIL entry, there was no file to be clobbered and thus it can be treated as a regular TX_RENAME. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Pavel Snajdr <snajpa@snajpa.net> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com> Closes openzfs#12209 Closes openzfs#14070