[PATCH 13/15] Add ngnfs_dir_rename() and very basic debugfs command

Valerie Aurora val at versity.com
Wed Mar 12 07:33:52 PDT 2025


Add mostly functional ngnfs_dir_rename() that removes target and
checks for directory loops and non-empty target directories. The
debugfs command is very limited: it can only rename a file from the
current working directory to the parent directory.

This has a known bug where after around 600 renames it returns a
spurious ENOENT.

Signed-off-by: Valerie Aurora <val at versity.com>
---
 cli/debugfs.c |  31 +++++++++
 shared/dir.c  | 171 ++++++++++++++++++++++++++++++++++++++++++++++++--
 shared/dir.h  |   3 +
 3 files changed, 199 insertions(+), 6 deletions(-)

diff --git a/cli/debugfs.c b/cli/debugfs.c
index 32fc3f6..9a61e80 100644
--- a/cli/debugfs.c
+++ b/cli/debugfs.c
@@ -214,6 +214,36 @@ static void cmd_readdir(struct debugfs_context *ctx, int argc, char **argv)
 	free(buf);
 }
 
+/*
+ * Basic rename hack for now.
+ *
+ * XXX assumes that you want to move a file from the current directory to ..
+ */
+static void cmd_rename(struct debugfs_context *ctx, int argc, char **argv)
+{
+	struct ngnfs_dir_lookup_entry parent;
+	char *name;
+	size_t name_len;
+	int ret;
+
+	if (argc != 2) {
+		printf("usage: rename <filename>\n");
+		printf("will move <filename> to ..\n");
+		return;
+	}
+
+	name = argv[1];
+	name_len = strlen(name);
+
+	ret = ngnfs_dir_lookup(ctx->nfi, &ctx->cwd_ig, "..", strlen(".."), &parent)		?:
+	      ngnfs_dir_rename(ctx->nfi, &ctx->cwd_ig, name, name_len, &parent.ig, name, name_len);
+
+	if (ret < 0) {
+		printf("rename error: "ENOF"\n", ENOA(-ret));
+		return;
+	}
+}
+
 static void cmd_rmdir(struct debugfs_context *ctx, int argc, char **argv)
 {
 	char *name;
@@ -333,6 +363,7 @@ static struct command {
 	{ "mkfs", cmd_mkfs, },
 	{ "quit", cmd_quit, },
 	{ "readdir", cmd_readdir, },
+	{ "rename", cmd_rename, },
 	{ "rmdir", cmd_rmdir, },
 	{ "stat", cmd_stat, },
 	{ "sync", cmd_sync, },
diff --git a/shared/dir.c b/shared/dir.c
index a8a180a..9447b34 100644
--- a/shared/dir.c
+++ b/shared/dir.c
@@ -523,7 +523,7 @@ static int lookup_dots(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *txn,
 }
 
 static int lookup_dirent(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *txn,
-			 struct ngnfs_inode_txn_ref *dir, struct dirent_args *da)
+			 struct ngnfs_inode_txn_ref *dir, struct dirent_args *da, bool enoent_ok)
 {
 	struct ngnfs_btree_key key;
 	struct ngnfs_btree_key last;
@@ -544,6 +544,9 @@ static int lookup_dirent(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *tx
 	if (da->dent.ig.ino == 0)
 		ret = -ENOENT;
 out:
+	if (enoent_ok && (ret == -ENOENT))
+		ret = 0;
+
 	return ret;
 }
 
@@ -576,7 +579,7 @@ int ngnfs_dir_lookup(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir_
 	do {
 		ret = ngnfs_inode_get(nfi, &op->txn, NBF_READ, dir_ig, &op->dir)		?:
 		      check_ifmt(op->dir.ninode, S_IFDIR, -ENOTDIR)				?:
-		      lookup_dirent(nfi, &op->txn, &op->dir, &op->da)				?:
+		      lookup_dirent(nfi, &op->txn, &op->dir, &op->da, false)			?:
 		      copy_dent_to_lent(&op->da.dent, lent);
 
 	} while (ngnfs_txn_retry(nfi, &op->txn, &ret));
@@ -612,7 +615,7 @@ static int remove_dirent_wr(struct ngnfs_btree_key *key, void *val, size_t size,
  * Remove a dirent item from a directory inode's dirent btree.
  */
 static int remove_dirent(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *txn,
-			 struct ngnfs_inode_txn_ref *dir, struct dirent_args *da)
+			 struct ngnfs_inode_txn_ref *dir, struct dirent_args *da, bool enoent_ok)
 {
 	struct ngnfs_btree_key key;
 	struct ngnfs_btree_key last;
@@ -624,10 +627,13 @@ static int remove_dirent(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *tx
 	ret = ngnfs_btree_write_iter(nfi, txn, dir->tblk, &dir->ninode->dirents, &key, &last,
 				     remove_dirent_wr, da);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	if (da->dent.ig.ino == 0)
-		ret = ENOENT;
+		ret = -ENOENT;
+out:
+	if (enoent_ok && (ret == -ENOENT))
+		ret = 0;
 
 	return ret;
 }
@@ -701,7 +707,7 @@ static int do_unlink(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir_
 	do {
 		ret = ngnfs_inode_get(nfi, &op->txn, NBF_WRITE, dir_ig, &op->dir)		?:
 		      check_ifmt(op->dir.ninode, S_IFDIR, -ENOTDIR)				?:
-		      remove_dirent(nfi, &op->txn, &op->dir, &op->da)				?:
+		      remove_dirent(nfi, &op->txn, &op->dir, &op->da, false)			?:
 		      ngnfs_inode_get(nfi, &op->txn, NBF_WRITE, &op->da.ig, &op->target)	?:
 		      check_remove_dirent(nfi, &op->txn, &op->dir, &op->target, &op->da, flags)	?:
 		      ngnfs_inode_update(op->target.tblk, op->target.ninode, -1)		?:
@@ -726,3 +732,156 @@ int ngnfs_dir_rmdir(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir,
 {
 	return do_unlink(nfi, dir, name, name_len, NGNFS_DIRENT_WANT_DIR);
 }
+
+static int igs_equal(struct ngnfs_inode_ino_gen *a, struct ngnfs_inode_ino_gen *b)
+{
+	return (a->ino == b->ino) && (a->gen == b->gen);
+}
+
+/*
+ * To prevent loops when renaming a directory, check if the src is a
+ * directory that is an ancestor of the dst. If it is, we can't rename.
+ * Check by walking up the path starting from the dst parent directory
+ * and comparing each directory to the src.
+ *
+ * When this is called, the parent directories of the src and dst items
+ * are different, and src_da is a directory. src can be the root
+ * directory.
+ */
+static int check_ancestors(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *txn,
+			   struct dirent_args *src_da, struct ngnfs_inode_txn_ref *dst_dir)
+{
+	struct ngnfs_inode_txn_ref walk_dir;
+	struct ngnfs_inode_ino_gen walk_ig;
+	int ret;
+
+	walk_dir = *dst_dir;
+
+	while(le64_to_cpu(walk_dir.ninode->ig.ino) != NGNFS_ROOT_INO) {
+		walk_ig.ino = le64_to_cpu(walk_dir.ninode->ig.ino);
+		walk_ig.gen = le64_to_cpu(walk_dir.ninode->ig.gen);
+
+		if (igs_equal(&walk_ig, &src_da->ig)) {
+			/* directory loop detected! */
+			ret = -ELOOP;
+			goto out;
+		}
+
+		/* no loop yet, lookup up parent and repeat */
+		walk_ig.ino = le64_to_cpu(walk_dir.ninode->parent_ig.ino);
+		walk_ig.gen = le64_to_cpu(walk_dir.ninode->parent_ig.gen);
+		if (walk_ig.ino == NGNFS_ROOT_INO)
+			break;
+
+		ret = ngnfs_inode_get(nfi, txn, NBF_READ, &walk_ig, &walk_dir);
+		if (ret < 0)
+			goto out;
+	};
+
+	ret = 0;
+out:
+	return ret;
+}
+
+static int inodes_equal(struct ngnfs_inode *a, struct ngnfs_inode *b) {
+	return (a->ig.ino == b->ig.ino) && (a->ig.gen == b->ig.gen);
+}
+
+/*
+ * Check the src and dst paths to rename for problems that can't be
+ * caught during the individual remove/insert/update operations to the
+ * btrees and inodes.
+ */
+static int check_rename(struct ngnfs_fs_info *nfi, struct ngnfs_transaction *txn,
+			struct ngnfs_inode_txn_ref *src_dir, struct dirent_args *src_da,
+			struct ngnfs_inode_txn_ref *dst_dir, struct dirent_args *dst_da)
+{
+	int ret;
+
+	/* XXX permissions */
+	/* XXX orphan inode handling */
+
+	if (inodes_equal(src_dir->ninode, dst_dir->ninode) &&
+	    names_equal(src_da->dent.name, src_da->dent.name_len,
+			dst_da->dent.name, dst_da->dent.name_len))
+		return -EEXIST;
+
+	/*
+	 * If source is a non-directory, fail if target is a dir,
+	 * otherwise it's success.
+	 */
+	if (src_da->dent.pers_dtype != NGNFS_DT_DIR) {
+		if (dst_da->dent.pers_dtype == NGNFS_DT_DIR)
+			return -EEXIST;
+		return 0;
+	}
+
+	/*
+	 * Source is a directory. If target is a directory, it can only
+	 * be overwritten if it is empty.
+	 */
+	if (dst_da->dent.pers_dtype == NGNFS_DT_DIR) {
+		ret = check_empty_dir(nfi, txn, &dst_da->ig);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* directory loops can't happen when the parent dirs are the same */
+	if (inodes_equal(src_dir->ninode, dst_dir->ninode))
+		return 0;
+
+	/* if the source is the root inode, we sure can't rename it! */
+	if (src_da->ig.ino == NGNFS_ROOT_INO)
+		return -ELOOP;
+
+	/* directory loop possible, check relationship between src and dst path */
+	ret = check_ancestors(nfi, txn, src_da, dst_dir);
+
+	return ret;
+}
+
+int ngnfs_dir_rename(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *src_dir_ig,
+		     char *src_name, size_t src_name_len, struct ngnfs_inode_ino_gen *dst_dir_ig,
+		     char *dst_name, size_t dst_name_len)
+{
+	struct {
+		struct ngnfs_transaction txn;
+		struct ngnfs_inode_txn_ref src_dir, dst_dir, dst;
+		struct dirent_args src_da, dst_da;
+	} *op;
+	int ret;
+
+	op = kmalloc(sizeof(*op), GFP_NOFS);
+	if (!op) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ngnfs_txn_init(&op->txn);
+	init_dirent_args(&op->src_da, src_name, src_name_len, 0);
+	init_dirent_args(&op->dst_da, dst_name, dst_name_len, 0);
+
+	do {
+		ret = ngnfs_inode_get(nfi, &op->txn, NBF_READ, src_dir_ig, &op->src_dir)	?:
+		      check_ifmt(op->src_dir.ninode, S_IFDIR, -ENOTDIR)				?:
+		      lookup_dirent(nfi, &op->txn, &op->src_dir, &op->src_da, false)		?:
+		      ngnfs_inode_get(nfi, &op->txn, NBF_READ, dst_dir_ig, &op->dst_dir)	?:
+		      check_ifmt(op->dst_dir.ninode, S_IFDIR, -ENOTDIR)				?:
+		      lookup_dirent(nfi, &op->txn, &op->dst_dir, &op->dst_da, true)		?:
+		      check_rename(nfi, &op->txn, &op->src_dir, &op->src_da, &op->dst_dir,
+				   &op->dst_da)							?:
+		      update_dirent_args_ig(&op->dst_da, &op->src_da.ig)			?:
+		      remove_dirent(nfi, &op->txn, &op->src_dir, &op->src_da, false)		?:
+		      update_dir(op->src_dir.tblk, op->src_dir.ninode, &op->src_da, -1)		?:
+		      remove_dirent(nfi, &op->txn, &op->dst_dir, &op->dst_da, true)		?:
+		      update_dirent_args_ig(&op->dst_da, &op->src_da.ig)			?:
+		      insert_dirent(nfi, &op->txn, &op->dst_dir, &op->dst_da)			?:
+		      update_dir(op->dst_dir.tblk, op->dst_dir.ninode, &op->dst_da, 1);
+
+	} while (ngnfs_txn_retry(nfi, &op->txn, &ret));
+
+	ngnfs_txn_teardown(nfi, &op->txn);
+	kfree(op);
+out:
+	return ret;
+}
diff --git a/shared/dir.h b/shared/dir.h
index 8aba78b..2d7eac5 100644
--- a/shared/dir.h
+++ b/shared/dir.h
@@ -14,6 +14,9 @@ int ngnfs_dir_unlink(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir,
 		     size_t name_len);
 int ngnfs_dir_rmdir(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir, char *name,
 		    size_t name_len);
+int ngnfs_dir_rename(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *src_dir_ig,
+		     char *src_name, size_t src_name_len, struct ngnfs_inode_ino_gen *dst_dir_ig,
+		     char *dst_name, size_t dst_name_len);
 
 /*
  * Readdir fills the buffer with entries.  The start of the buffer must
-- 
2.48.1




More information about the ngnfs-devel mailing list