fix(client): clean up empty directory rename metadata for mdtest (9a87740e) · Commits · hpc / gekkofs

src/client/gkfs_metadata.cpp

+81 −3

Changes for src/client/gkfs_metadata.cpp: 81 added lines, 3 removed lines.
Original line number	Diff line number	Diff line
		@@ -625,11 +625,90 @@ gkfs_rename(const string& old_path, const string& new_path) {
		auto md_old = gkfs::utils::get_metadata(old_path, false, true);
		std::string original_path = old_path;
		// if the file is not found, or it is a renamed one cancel.
		if(!md_old \|\| md_old.value().blocks() == -1) {
		if(!md_old) {
		return -1;
		}
		if(md_old.value().blocks() == -1) {
		errno = ENOENT;
		return -1;
		}

		if(old_path == new_path) {
		return 0;
		}

		auto md_new = gkfs::utils::get_metadata(new_path, false);
		if(!md_new && errno != ENOENT) {
		return -1;
		}

		// Directories cannot use file-rename tombstones: a later rename onto
		// the old name sees that tombstone as an existing directory, and its
		// removal fails with ENOENT. Empty directories have no data to retain.
		if(S_ISDIR(md_old->mode())) {
		if(new_path.compare(0, old_path.size() + 1, old_path + "/") == 0) {
		errno = EINVAL;
		return -1;
		}
		if(check_parent_dir(new_path)) {
		return -1;
		}
		auto entries = gkfs::rpc::forward_get_dirents(old_path);
		if(entries.first) {
		errno = entries.first;
		return -1;
		}
		if(entries.second->size() != 2) {
		// Moving a populated subtree requires relocating its path keys.
		errno = ENOTSUP;
		return -1;
		}
		if(md_new) {
		if(!S_ISDIR(md_new->mode())) {
		errno = ENOTDIR;
		return -1;
		}
		if(md_new->blocks() == -1) {
		int64_t size = 0;
		uint32_t mode = 0;
		std::string target;
		const auto err = gkfs::rpc::forward_remove(
		new_path, true, CTX->get_replicas(), size, mode, target,
		true);
		if(err) {
		errno = err;
		return -1;
		}
		} else if(gkfs_rmdir(new_path) != 0) {
		return -1;
		}
		}
		auto err = gkfs::rpc::forward_rename(old_path, new_path, *md_old);
		if(!err) {
		int64_t size = 0;
		uint32_t mode = 0;
		std::string target;
		err = gkfs::rpc::forward_remove(old_path, true, CTX->get_replicas(),
		size, mode, target, true);
		}
		if(err) {
		errno = err;
		return -1;
		}
		if(CTX->use_dentry_cache() && CTX->dentry_cache()) {
		CTX->dentry_cache()->clear_dir(
		std::filesystem::path(old_path).parent_path().string());
		CTX->dentry_cache()->clear_dir(
		std::filesystem::path(new_path).parent_path().string());
		CTX->dentry_cache()->clear_dir(old_path);
		CTX->dentry_cache()->clear_dir(new_path);
		}
		return 0;
		}
		if(md_new && S_ISDIR(md_new->mode())) {
		errno = EISDIR;
		return -1;
		}
		if(md_new) {
		// the new file exists... check for circular...
		if(md_new.value().blocks() == -1 &&
		@@ -2017,8 +2096,7 @@ gkfs_close(unsigned int fd) {
		return -1;
		}
		}
		if(CTX->use_dentry_cache() &&
		CTX->clear_dentry_cache_on_close()) {
		if(CTX->use_dentry_cache() && CTX->clear_dentry_cache_on_close()) {
		// clear cache for directory
		if(file->type() == gkfs::filemap::FileType::directory) {
		CTX->dentry_cache()->clear_dir(path);

tests/integration/rename/test_directory_rename.py

0 → 100644

+73 −0

Changes for tests/integration/rename/test_directory_rename.py: 73 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		"""Directory replacement and cleanup regressions from mdtest's rename phase."""

		import errno
		import os
		import stat

		import pytest


		@pytest.mark.parametrize("client_fixture", ["gkfs_client", "gkfs_clientLibc"])
		def test_directory_rename_replacement(client_fixture, request, gkfs_daemon):
		client = request.getfixturevalue(client_fixture)
		root = gkfs_daemon.mountdir / "rename-cycle"
		assert client.mkdir(root, 0o777).retval == 0
		first, second, temporary = [root / name for name in ("first", "second", "temporary")]
		for path in (first, second):
		assert client.mkdir(path, 0o777).retval == 0

		# mdtest rotates names through an intermediate directory, replacing an
		# existing empty directory and then reusing a previously renamed name.
		for _ in range(3):
		assert client.rename(first, temporary).retval == 0
		assert client.stat(first).retval == -1
		assert client.rename(second, first).retval == 0
		assert client.rename(temporary, second).retval == 0
		assert client.rename(first, first).retval == 0
		for path in (first, second):
		result = client.stat(path)
		assert result.retval == 0
		assert stat.S_ISDIR(result.statbuf.st_mode)

		# Actual empty-directory replacement, rather than rotation alone.
		assert client.rename(first, second).retval == 0
		assert client.stat(first).retval == -1
		assert client.rmdir(second).retval == 0
		assert client.rmdir(root).retval == 0


		@pytest.mark.parametrize("client_fixture", ["gkfs_client", "gkfs_clientLibc"])
		def test_directory_rename_type_and_contents(client_fixture, request, gkfs_daemon):
		client = request.getfixturevalue(client_fixture)
		root = gkfs_daemon.mountdir
		source, destination, file = [root / name for name in ("source", "destination", "file")]
		for path in (source, destination):
		assert client.mkdir(path, 0o777).retval == 0
		assert client.open(file, os.O_CREAT \| os.O_WRONLY, 0o600).retval >= 0
		assert client.open(destination / "child", os.O_CREAT \| os.O_WRONLY, 0o600).retval >= 0

		result = client.rename(source, source / "descendant")
		assert result.retval == -1
		assert result.errno == errno.EINVAL
		result = client.rename(source, root / "missing" / "destination")
		assert result.retval == -1
		assert result.errno == errno.ENOENT
		result = client.rename(source, destination)
		assert result.retval == -1
		assert result.errno == errno.ENOTEMPTY
		result = client.rename(source, file)
		assert result.retval == -1
		assert result.errno == errno.ENOTDIR
		result = client.rename(file, source)
		assert result.retval == -1
		assert result.errno == errno.EISDIR
		# Unsupported populated-subtree moves must not silently lose children.
		result = client.rename(destination, root / "moved")
		assert result.retval == -1
		assert result.errno == errno.ENOTSUP
		assert client.stat(destination / "child").retval == 0
		assert client.stat(source).retval == 0
		assert client.unlink(destination / "child").retval == 0
		assert client.unlink(file).retval == 0
		assert client.rmdir(destination).retval == 0
		assert client.rmdir(source).retval == 0
		No newline at end of file