Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OverlayFS support (d_revalidate out and support renameat2 flags) #9414

Open
wants to merge 6 commits into
base: master
from

Conversation

@snajpa
Copy link
Contributor

snajpa commented Oct 4, 2019

Description

This PR is comprised of several commits, the original motivation is to add the
functionality needed to enable OverlayFS on top of ZFS, which namely means
supporting renameat2() Linux syscall with its flags
(most interesting of which is the atomic swap of files with RENAME_EXCHANGE)
and getting rid of d_revalidate function, which makes ZFS appear as a remote
filesystem.

Bulk of the original work for supporting the renameat2() flags was done by
@cyphar; he proposes new IL record types, which I'd like as a best solution
as well - but I'm of the opinion that until more platforms adopt renameat2()
semantics in some way, it's not worth the trouble and I'm trying the compatible,
yet more ugly approach of emulating the otherwise atomic operations with
multiple non-atomic ones.

See the commit messages for more information on the relevant parts, I've kept
the code for new txtypes in the original @cyphar's commits.

Fixes #2256
Fixes #8648
Fixes #8774

How Has This Been Tested?

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@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)
  • Documentation (a change to man pages or other documentation)

Checklist:

@snajpa snajpa force-pushed the vpsfreecz:ovl branch from f3d65e0 to 008bc65 Oct 5, 2019
@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Oct 5, 2019

Btw, my branch with changes needed to get Docker working (and xattrs working in 1st level userns) can be found here:

https://github.com/vpsfreecz/zfs/commits/ovl-wip

@snajpa snajpa marked this pull request as ready for review Oct 5, 2019
@cyphar

This comment has been minimized.

Copy link

cyphar commented Oct 5, 2019

Have you tried mounting this with overlayfs directly? Given we no longer have a d_revalidate handler, overlayfs should work without additional hacks. (In fact, I would add that as a separate test.)

@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Oct 5, 2019

Hmm should have let the tests finish before publishing the PR as open for comments :) Now I have to figure out why is zfs_link_create playing the -ENOSYS note.

@snajpa snajpa force-pushed the vpsfreecz:ovl branch 3 times, most recently from c2e3c19 to aba3a50 Oct 6, 2019
@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Oct 7, 2019

Can I help myself and restart the tests somehow, please? Or is it up to the project leadership to come by and restart them?

Also, I'm trying running tests locally and even without my patches still, like > 150 tests are failing - is this normal? I see that some of them are documented to fail, but I still get a really long list of those without any annotation (like, acl tests fail? hm...).

@gmelikov

This comment has been minimized.

Copy link
Member

gmelikov commented Oct 7, 2019

@snajpa

  • to restart tests - you can just git commit --amend and git push origin HEAD -f, it will trigger bots
  • you may see some tests skipping due to non-installed deps, but you shouldn't see much more errors. One more recommendation - run tests inside VM to be safe from destroying your pools.
@snajpa snajpa force-pushed the vpsfreecz:ovl branch from aba3a50 to ce9b589 Oct 7, 2019
@snajpa snajpa force-pushed the vpsfreecz:ovl branch 10 times, most recently from 1e054d6 to d09d14b Oct 14, 2019
@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Oct 20, 2019

So I think this will be a last force-push if the tests finish up fine; I had a creds memory leak in there and an ASSERT-wrapped code before, which caused some space leak and associated entertaining cleanup afterwards on the staging nodes... :)

That is if I'm right in assuming all that's needed to fix the i386 build test is to restart it... is it so?

@snajpa snajpa force-pushed the vpsfreecz:ovl branch 6 times, most recently from 9ef43df to ccd1137 Oct 20, 2019
@behlendorf behlendorf self-requested a review Oct 21, 2019
vsecattr_t vsecp;

vsecp.vsa_mask |= VSA_ACE_ALLTYPES;
error = zfs_getacl(szp, &vsecp, B_TRUE, cr);

This comment has been minimized.

Copy link
@cyphar

cyphar Oct 30, 2019

This error isn't handled anywhere -- there should probably at least be an ASSERT3U(error, ==, 0) if not a goto commit_unlink_td_szp.

This comment has been minimized.

Copy link
@snajpa

snajpa Nov 4, 2019

Author Contributor

I'm going to use VERIFY0 here (and the new version of the patch is going to use VERIFY instead of ASSERT for errors where such as this, which shouldn't really happen, so that the production builds don't suffer from silent problems).

This comment has been minimized.

Copy link
@cyphar

cyphar Nov 10, 2019

Fair enough -- when writing them I was unsure whether ASSERT or VERIFY were the more accepted practices within the ZFS codebase. I would also opt for VERIFY.

commit_link_szp:
if (zfs_link_create(sdl, szp, tx, ZRENAMING))
VERIFY3U(zfs_drop_nlink(szp, tx, NULL), ==, 0);
goto commit;

This comment has been minimized.

Copy link
@cyphar

cyphar Oct 30, 2019

I think in the original PR there was an agreement that this handling wasn't necessary anymore (and neither was zfs_drop_nlink) now that we take a dirent lock on the source to make sure the allocations won't fail. Or did the dirent locking change never land in this PR?

This comment has been minimized.

Copy link
@snajpa

snajpa Nov 4, 2019

Author Contributor

It's been a while ;) we've discussed this bit here -> #8667 (comment)

This comment has been minimized.

Copy link
@cyphar

cyphar Nov 10, 2019

You're quite right -- carry on. 😉

do {
retries++;
(void) snprintf(tmpname, MAXPATHLEN,
"%s.zfs_renameat2_emul_%s%4d",

This comment has been minimized.

Copy link
@cyphar

cyphar Oct 30, 2019

I'd argue this should be a hidden file.

This comment has been minimized.

Copy link
@snajpa

snajpa Nov 4, 2019

Author Contributor

It will never be presented to the user anyway, going with the spirit of the rest of the review comments, I guess I would prefer not to add logic which is not needed, what do you think?

This comment has been minimized.

Copy link
@cyphar

cyphar Nov 10, 2019

You don't need to add logic -- my point was that it should be prefixed with a . but it's only a nit.


zfs_log_rename(zilog, tx, txtype, sdzp, sname, tdzp, tmpname, szp);
zfs_log_rename(zilog, tx, txtype, tdzp, dname, sdzp, sname, szp);
zfs_log_rename(zilog, tx, txtype, tdzp, tmpname, tdzp, dname, szp);

This comment has been minimized.

Copy link
@cyphar

cyphar Oct 30, 2019

Nit: I think the ordering should be the more traditional:

	/* dst -> tmp */
	zfs_log_rename(zilog, tx, txtype, tdzp, dname, tdzp, tmpname, szp);
	/* src -> dst */
	zfs_log_rename(zilog, tx, txtype, sdzp, sname, tdzp, dname, szp);
	/* tmp -> src */
	zfs_log_rename(zilog, tx, txtype, tdzp, tmpname, sdzp, sname, szp);
@snajpa snajpa force-pushed the vpsfreecz:ovl branch from ccd1137 to 5b7a4f5 Nov 2, 2019
@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Nov 2, 2019

Those failures do look bad enough to preclude test attempts on real data - uncorrectable pool errors are not a good thing.

Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/renameat2/setup (run as root) [00:00] [PASS]
Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/renameat2/renameat2_noreplace (run as root) [00:00] [FAIL]
Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/renameat2/renameat2_exchange (run as root) [00:00] [FAIL]
Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/renameat2/renameat2_whiteout (run as root) [00:00] [FAIL]
Test (Linux): /usr/share/zfs/zfs-tests/tests/functional/renameat2/cleanup (run as root) [00:00] [PASS]
@snajpa snajpa referenced this pull request Nov 4, 2019
5 of 12 tasks complete
@snajpa snajpa force-pushed the vpsfreecz:ovl branch 2 times, most recently from ca0539b to 09b358c Nov 5, 2019
@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 5, 2019

I'm lost, the tests fail with timeouts at

Test: /usr/share/zfs/zfs-tests/tests/functional/rename_dirs/setup (run as root) [00:00] [PASS]

with no further indication on what's wrong in the logs. My local setups seem to be passing the tests, even with Fedora 30...

@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 5, 2019

Okay, now I've got a total freeze while running the test suite, that definitely isn't healthy.

Edit: console over serial revealed a panic, so apparently I don't need to crfree the creds after zpl_vap_init as I thought; waiting what the tests say (locally it's finally passing all the tests, the logs of the bots were a bit misleading, those ended at rename_dirs, but it was really the renameat2 whiteout test, which panicked).

@snajpa snajpa force-pushed the vpsfreecz:ovl branch 4 times, most recently from c48c85b to d49f3af Nov 6, 2019
@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Nov 7, 2019

@snajpa: looks like tests (other than c7) are passing. How safe are you thinking for merge testing?

@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 7, 2019

@sempervictus most likely I broke something again, I'll fix it today.

@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 7, 2019

Hm, what. RHEL 7 seems to be supporting .rename2 op only for directory inode ops and .rename op doesn't take flags at all. Since when? What was the meaning of that? I have to dig more into this to understand what's happened there and why.

From EL7 fs/namei.c:

/**
 * vfs_rename - rename a filesystem object
 * @old_dir:	parent of source
 * @old_dentry:	source
 * @new_dir:	parent of destination
 * @new_dentry:	destination
 * @delegated_inode: returns an inode needing a delegation break
 * @flags:	rename flags
 *
 * The caller must hold multiple mutexes--see lock_rename()).
 *
 * If vfs_rename discovers a delegation in need of breaking at either
 * the source or destination, it will return -EWOULDBLOCK and return a
 * reference to the inode in delegated_inode.  The caller should then
 * break the delegation and retry.  Because breaking a delegation may
 * take a long time, the caller should drop all locks before doing
 * so.
 *
 * Alternatively, a caller may pass NULL for delegated_inode.  This may
 * be appropriate for callers that expect the underlying filesystem not
 * to be NFS exported.
 *
 * The worst of all namespace operations - renaming directory. "Perverted"
 * doesn't even start to describe it. Somebody in UCB had a heck of a trip...
 * Problems:
 *	a) we can get into loop creation.
 *	b) race potential - two innocent renames can create a loop together.
 *	   That's where 4.4 screws up. Current fix: serialization on
 *	   sb->s_vfs_rename_mutex. We might be more accurate, but that's another
 *	   story.
 *	c) we have to lock _four_ objects - parents and victim (if it exists),
 *	   and source (if it is not a directory).
 *	   And that - after we got ->i_mutex on parents (until then we don't know
 *	   whether the target exists).  Solution: try to be smart with locking
 *	   order for inodes.  We rely on the fact that tree topology may change
 *	   only under ->s_vfs_rename_mutex _and_ that parent of the object we
 *	   move will be locked.  Thus we can rank directories by the tree
 *	   (ancestors first) and rank all non-directories after them.
 *	   That works since everybody except rename does "lock parent, lookup,
 *	   lock child" and rename is under ->s_vfs_rename_mutex.
 *	   HOWEVER, it relies on the assumption that any object with ->lookup()
 *	   has no more than 1 dentry.  If "hybrid" objects will ever appear,
 *	   we'd better make sure that there's no link(2) for them.
 *	d) conversion from fhandle to dentry may come in the wrong moment - when
 *	   we are removing the target. Solution: we will have to grab ->i_mutex
 *	   in the fhandle_to_dentry code. [FIXME - current nfsfh.c relies on
 *	   ->i_mutex on parents, which works but leads to some truly excessive
 *	   locking].
 */
int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
	       struct inode *new_dir, struct dentry *new_dentry,
	       struct inode **delegated_inode, unsigned int flags)
{
	int error;
	bool is_dir = d_is_dir(old_dentry);
	struct inode *source = old_dentry->d_inode;
	struct inode *target = new_dentry->d_inode;
	bool new_is_dir = false;
	unsigned max_links = new_dir->i_sb->s_max_links;
	struct name_snapshot old_name;
	iop_rename2_t rename2;

	if (source == target)
		return 0;

	error = may_delete(old_dir, old_dentry, is_dir);
	if (error)
		return error;

	if (!target) {
		error = may_create(new_dir, new_dentry);
	} else {
		new_is_dir = d_is_dir(new_dentry);

		if (!(flags & RENAME_EXCHANGE))
			error = may_delete(new_dir, new_dentry, is_dir);
		else
			error = may_delete(new_dir, new_dentry, new_is_dir);
	}
	if (error)
		return error;

	rename2 = get_rename2_iop(old_dir);
	if (!old_dir->i_op->rename && !rename2)
		return -EPERM;

	if (flags && !rename2)
		return -EINVAL;

	/*
	 * If we are going to change the parent - check write permissions,
	 * we'll need to flip '..'.
	 */
	if (new_dir != old_dir) {
		if (is_dir) {
			error = inode_permission(source, MAY_WRITE);
			if (error)
				return error;
		}
		if ((flags & RENAME_EXCHANGE) && new_is_dir) {
			error = inode_permission(target, MAY_WRITE);
			if (error)
				return error;
		}
	}

	error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry,
				      flags);
	if (error)
		return error;

	take_dentry_name_snapshot(&old_name, old_dentry);
	dget(new_dentry);
	if (!is_dir || (flags & RENAME_EXCHANGE))
		lock_two_nondirectories(source, target);
	else if (target)
		mutex_lock(&target->i_mutex);

	error = -EBUSY;
	if (is_local_mountpoint(old_dentry) || is_local_mountpoint(new_dentry))
		goto out;

	if (max_links && new_dir != old_dir) {
		error = -EMLINK;
		if (is_dir && !new_is_dir && new_dir->i_nlink >= max_links)
			goto out;
		if ((flags & RENAME_EXCHANGE) && !is_dir && new_is_dir &&
		    old_dir->i_nlink >= max_links)
			goto out;
	}
	if (is_dir && !(flags & RENAME_EXCHANGE) && target)
		shrink_dcache_parent(new_dentry);
	if (!is_dir) {
		error = try_break_deleg(source, delegated_inode);
		if (error)
			goto out;
	}
	if (target && !new_is_dir) {
		error = try_break_deleg(target, delegated_inode);
		if (error)
			goto out;
	}
	if (!rename2) {
		error = old_dir->i_op->rename(old_dir, old_dentry,
					      new_dir, new_dentry);
	} else {
		//WARN_ON(old_dir->i_op->rename != NULL);
		error = rename2(old_dir, old_dentry,
				new_dir, new_dentry, flags);
	}
	if (error)
		goto out;

	if (!(flags & RENAME_EXCHANGE) && target) {
		if (is_dir)
			target->i_flags |= S_DEAD;
		dont_mount(new_dentry);
		detach_mounts(new_dentry);
	}
	if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE)) {
		if (!(flags & RENAME_EXCHANGE))
			d_move(old_dentry, new_dentry);
		else
			d_exchange(old_dentry, new_dentry);
	}
out:
	if (!is_dir || (flags & RENAME_EXCHANGE))
		unlock_two_nondirectories(source, target);
	else if (target)
		mutex_unlock(&target->i_mutex);
	dput(new_dentry);
	if (!error) {
		fsnotify_move(old_dir, new_dir, old_name.name, is_dir,
			      !(flags & RENAME_EXCHANGE) ? target : NULL, old_dentry);
		if (flags & RENAME_EXCHANGE) {
			fsnotify_move(new_dir, old_dir, old_dentry->d_name.name,
				      new_is_dir, NULL, new_dentry);
		}
	}
	release_dentry_name_snapshot(&old_name);

	return error;
}

SYSCALL_DEFINE5(renameat2, int, olddfd, const char __user *, oldname,
		int, newdfd, const char __user *, newname, unsigned int, flags)
{
	struct dentry *old_dir, *new_dir;
	struct dentry *old_dentry, *new_dentry;
	struct dentry *trap;
	struct nameidata oldnd, newnd;
	struct inode *delegated_inode = NULL;
	struct filename *from;
	struct filename *to;
	unsigned int lookup_flags = 0;
	bool should_retry = false;
	int error;

	if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
		return -EINVAL;

	if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
	    (flags & RENAME_EXCHANGE))
		return -EINVAL;

	if ((flags & RENAME_WHITEOUT) && !capable(CAP_MKNOD))
		return -EPERM;

retry:
	from = user_path_parent(olddfd, oldname, &oldnd, lookup_flags);
	if (IS_ERR(from)) {
		error = PTR_ERR(from);
		goto exit;
	}

	to = user_path_parent(newdfd, newname, &newnd, lookup_flags);
	if (IS_ERR(to)) {
		error = PTR_ERR(to);
		goto exit1;
	}

	error = -EXDEV;
	if (oldnd.path.mnt != newnd.path.mnt)
		goto exit2;

	old_dir = oldnd.path.dentry;
	error = -EBUSY;
	if (oldnd.last_type != LAST_NORM)
		goto exit2;

	new_dir = newnd.path.dentry;
	if (flags & RENAME_NOREPLACE)
		error = -EEXIST;
	if (newnd.last_type != LAST_NORM)
		goto exit2;

	error = mnt_want_write(oldnd.path.mnt);
	if (error)
		goto exit2;

	oldnd.flags &= ~LOOKUP_PARENT;
	newnd.flags &= ~LOOKUP_PARENT;
	if (!(flags & RENAME_EXCHANGE))
		newnd.flags |= LOOKUP_RENAME_TARGET;

retry_deleg:
	trap = lock_rename(new_dir, old_dir);

	old_dentry = lookup_hash(&oldnd);
	error = PTR_ERR(old_dentry);
	if (IS_ERR(old_dentry))
		goto exit3;
	/* source must exist */
	error = -ENOENT;
	if (d_is_negative(old_dentry))
		goto exit4;
	new_dentry = lookup_hash(&newnd);
	error = PTR_ERR(new_dentry);
	if (IS_ERR(new_dentry))
		goto exit4;
	error = -EEXIST;
	if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry))
		goto exit5;
	if (flags & RENAME_EXCHANGE) {
		error = -ENOENT;
		if (d_is_negative(new_dentry))
			goto exit5;

		if (!d_is_dir(new_dentry)) {
			error = -ENOTDIR;
			if (newnd.last.name[newnd.last.len])
				goto exit5;
		}
	}
	/* unless the source is a directory trailing slashes give -ENOTDIR */
	if (!d_is_dir(old_dentry)) {
		error = -ENOTDIR;
		if (oldnd.last.name[oldnd.last.len])
			goto exit5;
		if (!(flags & RENAME_EXCHANGE) && newnd.last.name[newnd.last.len])
			goto exit5;
	}
	/* source should not be ancestor of target */
	error = -EINVAL;
	if (old_dentry == trap)
		goto exit5;
	/* target should not be an ancestor of source */
	if (!(flags & RENAME_EXCHANGE))
		error = -ENOTEMPTY;
	if (new_dentry == trap)
		goto exit5;

	error = security_path_rename(&oldnd.path, old_dentry,
				     &newnd.path, new_dentry, flags);
	if (error)
		goto exit5;
	error = vfs_rename(old_dir->d_inode, old_dentry,
			   new_dir->d_inode, new_dentry,
			   &delegated_inode, flags);
exit5:
	dput(new_dentry);
exit4:
	dput(old_dentry);
exit3:
	unlock_rename(new_dir, old_dir);
	if (delegated_inode) {
		error = break_deleg_wait(&delegated_inode);
		if (!error)
			goto retry_deleg;
	}
	mnt_drop_write(oldnd.path.mnt);
exit2:
	if (retry_estale(error, lookup_flags))
		should_retry = true;
	path_put(&newnd.path);
	putname(to);
exit1:
	path_put(&oldnd.path);
	putname(from);
	if (should_retry) {
		should_retry = false;
		lookup_flags |= LOOKUP_REVAL;
		goto retry;
	}
exit:
	return error;
}

SYSCALL_DEFINE4(renameat, int, olddfd, const char __user *, oldname,
		int, newdfd, const char __user *, newname)
{
	return sys_renameat2(olddfd, oldname, newdfd, newname, 0);
}

SYSCALL_DEFINE2(rename, const char __user *, oldname, const char __user *, newname)
{
	return sys_renameat2(AT_FDCWD, oldname, AT_FDCWD, newname, 0);
}
@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 7, 2019

rename2 is always a dir-only operation; RHEL 7 has const struct inode_operations_wrapper for directory inode operations, so I'll add a new config test and appropriate #ifdef's to zpl_inode.c.

@snajpa

This comment has been minimized.

Copy link
Contributor Author

snajpa commented Nov 7, 2019

@sempervictus I think it's good to go now.

@cyphar cyphar referenced this pull request Nov 11, 2019
5 of 13 tasks complete
@cyphar

This comment has been minimized.

Copy link

cyphar commented Nov 11, 2019

Should probably be rebased now that #9549 has been merged.

cyphar and others added 6 commits Apr 26, 2019
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.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
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
need to create a new transaction type (to be backwards-compatible).
Since RENAME_EXCHANGE and RENAME_WHITEOUT are mutually exclusive they
deserve separate types. We just re-use the logic of
zfs_{log,replay}_rename() with the only change being the transaction
types and the associate vfsflags passed to zfs_rename().

RENAME_NOREPLACE doesn't need an entry because if the renameat2(2) fails
because of RENAME_NOREPLACE there won't be a ZIL entry for the operation
(and if it succeeds then it should also succeed on-replay).

Unfortunately, more work is required in order use overlayfs-on-ZFS
(namely we have to remove our .d_revalidate hook, since overlayfs
refuses to use a filesystem with d_revalidate as an upperdir).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
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: Pavel Snajdr <snajpa@snajpa.net>
Removing new txtypes in favor of compound ZIL operations, see comment in
module/zfs/zfs_log.c.

Other notable changes:

- unlock after the inodes are updated
- pass whiteout znode pointer to zfs_log_rename_whiteout
- don't wrap code directly in ASSERT*(), it turn to noop on non-debug
  builds
- update configure time tests for rename2 to support kernels
  from 3.5 to 4.8

Fixes #2256
Fixes #8648
Fixes #8774

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
EL7 has .rename2 op upper in the inode_operations_wrapper,
so let's add a wrapping ifdef to dir inode_operations
and ifdef the ops assignment too.

Configure test lives in kernel-rename.m4 for now, as rename2/EL7 case
is the only reason why we need it - for now, until support for
tmpfile is needed. then it would me more logical to move this check
to a separate file.

Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
@snajpa snajpa force-pushed the vpsfreecz:ovl branch from 164ec05 to 449e0a3 Nov 11, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #9414 into master will increase coverage by 0.06%.
The diff coverage is 79.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9414      +/-   ##
==========================================
+ Coverage   79.05%   79.12%   +0.06%     
==========================================
  Files         418      419       +1     
  Lines      123680   123818     +138     
==========================================
+ Hits        97777    97965     +188     
+ Misses      25903    25853      -50
Flag Coverage Δ
#kernel 79.76% <78.14%> (+0.03%) ⬆️
#user 66.7% <83.87%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a6ac4c...449e0a3. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.