[Cluster-devel] [PATCH 0/6] gfs2-utils: Cleanups and fsck.gfs2 fixes

[Cluster-devel] [PATCH 0/6] gfs2-utils: Cleanups and fsck.gfs2 fixes

[Andrew Price]

This set is mainly cleanups but patches 5 and 6 are small fixes for
fsck.gfs2 which I would like to draw attention to.

PR is https://pagure.io/gfs2-utils/pull-request/14 awaiting CI testing
should you prefer to comment there.

Andy

Andrew Price (6):
  libgfs2: Return the inode from lgfs2_lookupi()
  libgfs2: Remove lgfs2_gfs_createi()
  libgfs2: Reorganise lgfs2_createi()
  fsck.gfs2: Remove de variable from dirref_find()
  fsck.gfs2: Fix wrong entry used in dentry comparison
  fsck.gfs2: fix_hashtable: Decrement i_blocks when freeing leaf blocks

 gfs2/convert/gfs2_convert.c |   7 +--
 gfs2/edit/hexedit.c         |   2 +-
 gfs2/fsck/fs_recovery.c     |  14 ++---
 gfs2/fsck/initialize.c      |  18 +++---
 gfs2/fsck/lost_n_found.c    |   6 +-
 gfs2/fsck/pass2.c           |  22 +++----
 gfs2/glocktop/glocktop.c    |   6 +-
 gfs2/libgfs2/check_fs_ops.c |  18 +++---
 gfs2/libgfs2/fs_ops.c       | 121 +++++++++++++++++++-----------------
 gfs2/libgfs2/gfs2l.c        |   2 +-
 gfs2/libgfs2/lang.c         |  10 +--
 gfs2/libgfs2/libgfs2.h      |   6 +-
 tests/nukerg.c              |   2 +-
 13 files changed, 113 insertions(+), 121 deletions(-)

-- 
2.39.0

[Cluster-devel] [PATCH 1/6] libgfs2: Return the inode from lgfs2_lookupi()

[Andrew Price]

Instead of returning a -errno with the inode set via a passed in
pointer, return the inode and use errno. This is more consistent and
simplifies some calling code.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/convert/gfs2_convert.c |  7 +++----
 gfs2/edit/hexedit.c         |  2 +-
 gfs2/fsck/fs_recovery.c     | 14 ++++++--------
 gfs2/fsck/initialize.c      | 18 +++++++++---------
 gfs2/fsck/pass2.c           |  8 ++++----
 gfs2/glocktop/glocktop.c    |  6 +++---
 gfs2/libgfs2/check_fs_ops.c | 18 +++++++-----------
 gfs2/libgfs2/fs_ops.c       | 38 +++++++++++++++++++------------------
 gfs2/libgfs2/gfs2l.c        |  2 +-
 gfs2/libgfs2/lang.c         | 10 ++++++----
 gfs2/libgfs2/libgfs2.h      |  3 +--
 tests/nukerg.c              |  2 +-
 12 files changed, 62 insertions(+), 66 deletions(-)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index e0ca7c71..bf9985ae 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -2139,11 +2139,10 @@ static void copy_quotas(struct lgfs2_sbd *sdp)
 {
 	struct lgfs2_inum inum;
 	struct lgfs2_inode *oq_ip, *nq_ip;
-	int err;
 
-	err = lgfs2_lookupi(sdp->master_dir, "quota", 5, &nq_ip);
-	if (err) {
-		fprintf(stderr, _("Couldn't lookup new quota file: %d\n"), err);
+	nq_ip = lgfs2_lookupi(sdp->master_dir, "quota", 5);
+	if (nq_ip == NULL) {
+		perror(_("Failed to look up new quota file"));
 		exit(1);
 	}
 
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 553b15a5..c779433c 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -925,7 +925,7 @@ static void read_superblock(int fd)
 		if (sbd.master_dir == NULL) {
 			sbd.md.riinode = NULL;
 		} else {
-			lgfs2_lookupi(sbd.master_dir, "rindex", 6, &sbd.md.riinode);
+			sbd.md.riinode = lgfs2_lookupi(sbd.master_dir, "rindex", 6);
 		}
 	}
 	lgfs2_brelse(bh);
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 8e572523..47433eac 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -789,10 +789,9 @@ int ji_update(struct lgfs2_sbd *sdp)
 				return -1;
 		} else {
 			/* FIXME check snprintf return code */
-			snprintf(journal_name, JOURNAL_NAME_SIZE,
-				 "journal%u", i);
-			lgfs2_lookupi(sdp->md.jiinode, journal_name,
-				     strlen(journal_name), &jip);
+			int len;
+			len = snprintf(journal_name, JOURNAL_NAME_SIZE, "journal%u", i);
+			jip = lgfs2_lookupi(sdp->md.jiinode, journal_name, len);
 			sdp->md.journal[i] = jip;
 		}
 	}
@@ -884,7 +883,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
 	if (sdp->gfs1)
 		sdp->md.jiinode = lgfs2_inode_read(sdp, sdp->sd_jindex_di.in_addr);
 	else
-		lgfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
+		sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir, "jindex", 6);
 
 	if (!sdp->md.jiinode) {
 		int err;
@@ -906,7 +905,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
 			log_crit(_("Error %d rebuilding jindex\n"), err);
 			return err;
 		}
-		lgfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
+		sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir, "jindex", 6);
 	}
 
 	/* check for irrelevant entries in jindex. Can't use check_dir because
@@ -938,8 +937,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
 					  "index: Cannot continue.\n"));
 				return error;
 			}
-			lgfs2_lookupi(sdp->master_dir, "jindex", 6,
-				     &sdp->md.jiinode);
+			sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir, "jindex", 6);
 		}
 	}
 
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index edb0b7fd..e8a8cb8b 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -597,7 +597,7 @@ static void lookup_per_node(struct fsck_cx *cx, int allow_rebuild)
 	if (sdp->md.pinode)
 		return;
 
-	lgfs2_lookupi(sdp->master_dir, "per_node", 8, &sdp->md.pinode);
+	sdp->md.pinode = lgfs2_lookupi(sdp->master_dir, "per_node", 8);
 	if (sdp->md.pinode)
 		return;
 	if (!allow_rebuild) {
@@ -618,7 +618,7 @@ static void lookup_per_node(struct fsck_cx *cx, int allow_rebuild)
 			exit(FSCK_ERROR);
 		}
 	}
-	lgfs2_lookupi(sdp->master_dir, "per_node", 8, &sdp->md.pinode);
+	sdp->md.pinode = lgfs2_lookupi(sdp->master_dir, "per_node", 8);
 	if (!sdp->md.pinode) {
 		log_err( _("Unable to rebuild per_node; aborting.\n"));
 		exit(FSCK_ERROR);
@@ -799,7 +799,7 @@ static int init_system_inodes(struct fsck_cx *cx)
 	 *******************************************************************/
 	if (!sdp->gfs1) {
 		/* Look for "inum" entry in master dinode */
-		lgfs2_lookupi(sdp->master_dir, "inum", 4, &sdp->md.inum);
+		sdp->md.inum = lgfs2_lookupi(sdp->master_dir, "inum", 4);
 		if (!sdp->md.inum) {
 			if (!query(cx, _("The gfs2 system inum inode is missing. "
 				      "Okay to rebuild it? (y/n) "))) {
@@ -850,7 +850,7 @@ static int init_system_inodes(struct fsck_cx *cx)
 			goto fail;
 		}
 	} else
-		lgfs2_lookupi(sdp->master_dir, "statfs", 6, &sdp->md.statfs);
+		sdp->md.statfs = lgfs2_lookupi(sdp->master_dir, "statfs", 6);
 	if (!sdp->gfs1 && !sdp->md.statfs) {
 		if (!query(cx, _("The gfs2 system statfs inode is missing. "
 			      "Okay to rebuild it? (y/n) "))) {
@@ -907,7 +907,7 @@ static int init_system_inodes(struct fsck_cx *cx)
 			goto fail;
 		}
 	} else
-		lgfs2_lookupi(sdp->master_dir, "quota", 5, &sdp->md.qinode);
+		sdp->md.qinode = lgfs2_lookupi(sdp->master_dir, "quota", 5);
 	if (!sdp->gfs1 && !sdp->md.qinode) {
 		if (!query(cx, _("The gfs2 system quota inode is missing. "
 			      "Okay to rebuild it? (y/n) "))) {
@@ -996,7 +996,7 @@ static void peruse_system_dinode(struct fsck_cx *cx, struct lgfs2_inode *ip)
 	} else if (!sdp->gfs1 && is_dir(ip, sdp->gfs1)) {
 		/* Check for a jindex dir entry. Only one system dir has a
 		   jindex: master */
-		lgfs2_lookupi(ip, "jindex", 6, &child_ip);
+		child_ip = lgfs2_lookupi(ip, "jindex", 6);
 		if (child_ip) {
 			if (fix_md.jiinode || is_journal_copy(ip)) {
 				lgfs2_inode_put(&child_ip);
@@ -1011,7 +1011,7 @@ static void peruse_system_dinode(struct fsck_cx *cx, struct lgfs2_inode *ip)
 
 		/* Check for a statfs_change0 dir entry. Only one system dir
 		   has a statfs_change: per_node, and its .. will be master. */
-		lgfs2_lookupi(ip, "statfs_change0", 14, &child_ip);
+		child_ip = lgfs2_lookupi(ip, "statfs_change0", 14);
 		if (child_ip) {
 			lgfs2_inode_put(&child_ip);
 			if (fix_md.pinode || is_journal_copy(ip))
@@ -1100,7 +1100,7 @@ static void peruse_user_dinode(struct fsck_cx *cx, struct lgfs2_inode *ip)
 		return;
 	}
 	while (ip) {
-		lgfs2_lookupi(ip, "..", 2, &parent_ip);
+		parent_ip = lgfs2_lookupi(ip, "..", 2);
 		if (parent_ip && parent_ip->i_num.in_addr == ip->i_num.in_addr) {
 			log_warn(_("Found the root directory at: 0x%"PRIx64"\n"),
 				 ip->i_num.in_addr);
@@ -1534,7 +1534,7 @@ static int init_rindex(struct fsck_cx *cx)
 	if (sdp->gfs1)
 		sdp->md.riinode = lgfs2_inode_read(sdp, sdp->sd_rindex_di.in_addr);
 	else
-		lgfs2_lookupi(sdp->master_dir, "rindex", 6, &sdp->md.riinode);
+		sdp->md.riinode = lgfs2_lookupi(sdp->master_dir, "rindex", 6);
 
 	if (sdp->md.riinode)
 		return 0;
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 6792f795..0c2e0146 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1825,8 +1825,8 @@ static int check_pernode_for(struct fsck_cx *cx, int x, struct lgfs2_inode *pern
 	int error, valid_size = 1;
 
 	log_debug(_("Checking system file %s\n"), fn);
-	error = lgfs2_lookupi(pernode, fn, strlen(fn), &ip);
-	if (error) {
+	ip = lgfs2_lookupi(pernode, fn, strlen(fn));
+	if (ip == NULL) {
 		log_err(_("System file %s is missing.\n"), fn);
 		if (!query(cx, _("Rebuild the system file? (y/n) ")))
 			return 0;
@@ -1867,8 +1867,8 @@ build_it:
 		log_err(_("Error building %s\n"), fn);
 		return -1;
 	}
-	error = lgfs2_lookupi(pernode, fn, strlen(fn), &ip);
-	if (error) {
+	ip = lgfs2_lookupi(pernode, fn, strlen(fn));
+	if (ip == NULL) {
 		log_err(_("Error rebuilding %s.\n"), fn);
 		return -1;
 	}
diff --git a/gfs2/glocktop/glocktop.c b/gfs2/glocktop/glocktop.c
index f0053853..d35c841f 100644
--- a/gfs2/glocktop/glocktop.c
+++ b/gfs2/glocktop/glocktop.c
@@ -623,15 +623,15 @@ static const char *show_inode(const char *id, int fd, uint64_t block)
 	if (S_ISDIR(ip->i_mode)) {
 		struct lgfs2_inode *parent;
 		uint64_t dirarray[256];
-		int subdepth = 0, error;
+		int subdepth = 0;
 
 		inode_type = "directory ";
 		dirarray[0] = block;
 		subdepth++;
 		/* Backtrack the directory to its source */
 		while (1) {
-			error = lgfs2_lookupi(ip, "..", 2, &parent);
-			if (error)
+			parent = lgfs2_lookupi(ip, "..", 2);
+			if (parent == NULL)
 				break;
 			/* Stop at the root inode */
 			if (ip->i_num.in_addr == parent->i_num.in_addr) {
diff --git a/gfs2/libgfs2/check_fs_ops.c b/gfs2/libgfs2/check_fs_ops.c
index c93e11a4..abe31312 100644
--- a/gfs2/libgfs2/check_fs_ops.c
+++ b/gfs2/libgfs2/check_fs_ops.c
@@ -91,14 +91,13 @@ START_TEST(test_lookupi_bad_name_size)
 {
 	struct lgfs2_inode idir;
 	struct lgfs2_inode *ret = NULL;
-	int e;
 
-	e = lgfs2_lookupi(&idir, ".", 0, &ret);
-	ck_assert(e == -ENAMETOOLONG);
+	ret = lgfs2_lookupi(&idir, ".", 0);
+	ck_assert(errno == ENAMETOOLONG);
 	ck_assert(ret == NULL);
 
-	e = lgfs2_lookupi(&idir, ".", GFS2_FNAMESIZE + 1, &ret);
-	ck_assert(e == -ENAMETOOLONG);
+	ret = lgfs2_lookupi(&idir, ".", GFS2_FNAMESIZE + 1);
+	ck_assert(errno == ENAMETOOLONG);
 	ck_assert(ret == NULL);
 }
 END_TEST
@@ -107,11 +106,9 @@ START_TEST(test_lookupi_dot)
 {
 	struct lgfs2_inode idir;
 	struct lgfs2_inode *ret;
-	int e;
 
 	/* The contents of idir shouldn't matter, a "." lookup should just return it */
-	e = lgfs2_lookupi(&idir, ".", 1, &ret);
-	ck_assert(e == 0);
+	ret = lgfs2_lookupi(&idir, ".", 1);
 	ck_assert(ret == &idir);
 }
 END_TEST
@@ -133,7 +130,6 @@ START_TEST(test_lookupi_dotdot)
 	};
 	struct gfs2_dirent *dent = (void *)(buf + sizeof(struct gfs2_dinode));
 	struct lgfs2_inode *ret;
-	int e;
 
 	/* "." */
 	dent->de_inum.no_addr = cpu_to_be64(42);
@@ -153,8 +149,8 @@ START_TEST(test_lookupi_dotdot)
 	*(char *)(dent + 1) = '.';
 	*((char *)(dent + 1) + 1) = '.';
 
-	e = lgfs2_lookupi(&idir, "..", 2, &ret);
-	ck_assert(e == 0);
+	ret = lgfs2_lookupi(&idir, "..", 2);
+	ck_assert(ret != NULL);
 	ck_assert(ret != &idir);
 	lgfs2_inode_put(&ret);
 }
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index fb47e4f3..e72871ed 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -1509,7 +1509,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
 	int err = 0;
 	int is_dir;
 
-	lgfs2_lookupi(dip, filename, strlen(filename), &ip);
+	ip = lgfs2_lookupi(dip, filename, strlen(filename));
 	if (!ip) {
 		struct lgfs2_inum parent = dip->i_num;
 
@@ -1775,7 +1775,7 @@ int lgfs2_dir_search(struct lgfs2_inode *dip, const char *filename, int len,
 {
 	int error;
 
-	if(!S_ISDIR(dip->i_mode) && !lgfs2_is_gfs_dir(dip))
+	if (!S_ISDIR(dip->i_mode) && !lgfs2_is_gfs_dir(dip))
 		return -1;
 
 	if (dip->i_flags & GFS2_DIF_EXHASH)
@@ -1875,33 +1875,35 @@ int lgfs2_dirent_del(struct lgfs2_inode *dip, const char *filename, int len)
 }
 
 /**
- * lgfs2_lookupi - Look up a filename in a directory and return its inode
+ * Look up a filename in a directory and return its inode, which can be the
+ * the directory inode when "." is looked up.
  * @dip: The directory to search
  * @name: The name of the inode to look for
- * @ipp: Used to return the found inode if any
+ * @len: The length of name
  *
- * Returns: 0 on success, -EXXXX on failure
+ * Returns: The inode on success or NULL on failure with errno set.
  */
-int lgfs2_lookupi(struct lgfs2_inode *dip, const char *filename, int len,
-                  struct lgfs2_inode **ipp)
+struct lgfs2_inode *lgfs2_lookupi(struct lgfs2_inode *dip, const char *filename, int len)
 {
 	struct lgfs2_sbd *sdp = dip->i_sbd;
-	int error = 0;
 	struct lgfs2_inum inum;
+	int error = 0;
 
-	*ipp = NULL;
+	errno = EINVAL;
+	if (dip == NULL)
+		return NULL;
 
+	errno = ENAMETOOLONG;
 	if (!len || len > GFS2_FNAMESIZE)
-		return -ENAMETOOLONG;
-	if (gfs2_filecmp(filename, ".", 1)) {
-		*ipp = dip;
-		return 0;
-	}
-	error = lgfs2_dir_search(dip, filename, len, NULL, &inum);
-	if (!error)
-		*ipp = lgfs2_inode_read(sdp, inum.in_addr);
+		return NULL;
 
-	return error;
+	if (gfs2_filecmp(filename, ".", 1))
+		return dip;
+
+	error = lgfs2_dir_search(dip, filename, len, NULL, &inum);
+	if (error)
+		return NULL;
+	return lgfs2_inode_read(sdp, inum.in_addr);
 }
 
 /**
diff --git a/gfs2/libgfs2/gfs2l.c b/gfs2/libgfs2/gfs2l.c
index 3694a3d0..42f1b6c8 100644
--- a/gfs2/libgfs2/gfs2l.c
+++ b/gfs2/libgfs2/gfs2l.c
@@ -139,7 +139,7 @@ static int openfs(const char *path, struct lgfs2_sbd *sdp)
 	}
 
 	sdp->master_dir = lgfs2_inode_read(sdp, sdp->sd_meta_dir.in_addr);
-	lgfs2_lookupi(sdp->master_dir, "rindex", 6, &sdp->md.riinode);
+	sdp->md.riinode = lgfs2_lookupi(sdp->master_dir, "rindex", 6);
 	sdp->fssize = sdp->device.length;
 	if (sdp->md.riinode) {
 		lgfs2_rindex_read(sdp, &count, &ok);
diff --git a/gfs2/libgfs2/lang.c b/gfs2/libgfs2/lang.c
index 96ba0146..1b959671 100644
--- a/gfs2/libgfs2/lang.c
+++ b/gfs2/libgfs2/lang.c
@@ -150,7 +150,6 @@ static void ast_string_unescape(char *str)
 
 static uint64_t ast_lookup_path(char *path, struct lgfs2_sbd *sbd)
 {
-	int err = 0;
 	char *c = NULL;
 	struct lgfs2_inode *ip, *iptmp;
 	char *segment;
@@ -160,16 +159,19 @@ static uint64_t ast_lookup_path(char *path, struct lgfs2_sbd *sbd)
 	ip = lgfs2_inode_read(sbd, sbd->sd_root_dir.in_addr);
 
 	while (ip != NULL) {
+		int err = 0;
+
 		if (segment == NULL) { // No more segments
 			bn = ip->i_num.in_addr;
 			lgfs2_inode_put(&ip);
 			return bn;
 		}
 		ast_string_unescape(segment);
-		err = lgfs2_lookupi(ip, segment, strlen(segment), &iptmp);
+		iptmp = lgfs2_lookupi(ip, segment, strlen(segment));
+		err = errno;
 		lgfs2_inode_put(&ip);
-		if (err != 0) {
-			errno = -err;
+		if (iptmp == NULL) {
+			errno = err;
 			break;
 		}
 		ip = iptmp;
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 6f8afe0d..bcbc5e47 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -522,8 +522,7 @@ extern void lgfs2_dirent2_del(struct lgfs2_inode *dip, struct lgfs2_buffer_head
 			struct gfs2_dirent *prev, struct gfs2_dirent *cur);
 extern int lgfs2_dir_search(struct lgfs2_inode *dip, const char *filename, int len,
 		      unsigned int *type, struct lgfs2_inum *inum);
-extern int lgfs2_lookupi(struct lgfs2_inode *dip, const char *filename, int len,
-			struct lgfs2_inode **ipp);
+extern struct lgfs2_inode *lgfs2_lookupi(struct lgfs2_inode *dip, const char *filename, int len);
 extern int lgfs2_dir_add(struct lgfs2_inode *dip, const char *filename, int len,
 		    struct lgfs2_inum *inum, unsigned int type);
 extern int lgfs2_dirent_del(struct lgfs2_inode *dip, const char *filename, int name_len);
diff --git a/tests/nukerg.c b/tests/nukerg.c
index a4c25a81..09b6dfd7 100644
--- a/tests/nukerg.c
+++ b/tests/nukerg.c
@@ -250,7 +250,7 @@ static lgfs2_rgrps_t read_rindex(struct lgfs2_sbd *sdp)
 	unsigned rgcount;
 	unsigned i;
 
-	lgfs2_lookupi(sdp->master_dir, "rindex", 6, &sdp->md.riinode);
+	sdp->md.riinode = lgfs2_lookupi(sdp->master_dir, "rindex", 6);
 	if (sdp->md.riinode == NULL) {
 		perror("Failed to look up rindex");
 		return NULL;
-- 
2.39.0

[Cluster-devel] [PATCH 2/6] libgfs2: Remove lgfs2_gfs_createi()

[Andrew Price]

The one caller sets sdp->gfs1 and modifies the mode itself so we can use
sdp->gfs1 in __createi() and update the caller to use lgfs2_createi()
instead.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/lost_n_found.c |  6 +-----
 gfs2/libgfs2/fs_ops.c    | 19 ++++++-------------
 gfs2/libgfs2/libgfs2.h   |  3 ---
 3 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index ca5ab89e..97fc16b0 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -109,11 +109,7 @@ void make_sure_lf_exists(struct fsck_cx *cx, struct lgfs2_inode *ip)
 	if (sdp->gfs1)
 		sdp->md.next_inum = find_free_blk(sdp);
 	mode = (sdp->gfs1 ? DT2IF(GFS_FILE_DIR) : S_IFDIR) | 0700;
-	if (sdp->gfs1)
-		lf_dip = lgfs2_gfs_createi(sdp->md.rooti, "lost+found", mode, 0);
-	else
-		lf_dip = lgfs2_createi(sdp->md.rooti, "lost+found",
-				 S_IFDIR | 0700, 0);
+	lf_dip = lgfs2_createi(sdp->md.rooti, "lost+found", mode, 0);
 	if (lf_dip == NULL) {
 		log_crit(_("Error creating lost+found: %s\n"),
 			 strerror(errno));
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index e72871ed..5003c1ae 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -1497,9 +1497,8 @@ int lgfs2_write_filemeta(struct lgfs2_inode *ip)
 	return 0;
 }
 
-static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
-				    const char *filename, unsigned int mode,
-				    uint32_t flags, int if_gfs1)
+static struct lgfs2_inode *__createi(struct lgfs2_inode *dip, const char *filename,
+                                     unsigned int mode, uint32_t flags)
 {
 	struct lgfs2_sbd *sdp = dip->i_sbd;
 	uint64_t bn;
@@ -1517,7 +1516,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
 		if (err != 0)
 			return NULL;
 
-		if (if_gfs1)
+		if (sdp->gfs1)
 			inum.in_formal_ino = bn;
 		else
 			inum.in_formal_ino = sdp->md.next_inum++;
@@ -1527,7 +1526,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
 		if (err)
 			return NULL;
 
-		if (if_gfs1)
+		if (sdp->gfs1)
 			is_dir = (IF2DT(mode) == GFS_FILE_DIR);
 		else
 			is_dir = S_ISDIR(mode);
@@ -1536,7 +1535,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
 			dip->i_nlink++;
 		}
 
-		err = __init_dinode(sdp, &bh, &inum, mode, flags, &parent, if_gfs1);
+		err = __init_dinode(sdp, &bh, &inum, mode, flags, &parent, sdp->gfs1);
 		if (err != 0)
 			return NULL;
 
@@ -1552,13 +1551,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
 struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char *filename,
                                  unsigned int mode, uint32_t flags)
 {
-	return __createi(dip, filename, mode, flags, 0);
-}
-
-struct lgfs2_inode *lgfs2_gfs_createi(struct lgfs2_inode *dip, const char *filename,
-                                     unsigned int mode, uint32_t flags)
-{
-	return __createi(dip, filename, mode, flags, 1);
+	return __createi(dip, filename, mode, flags);
 }
 
 /**
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index bcbc5e47..69a7552f 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -515,9 +515,6 @@ extern int lgfs2_init_dinode(struct lgfs2_sbd *sdp, struct lgfs2_buffer_head **b
                        unsigned int mode, uint32_t flags, struct lgfs2_inum *parent);
 extern struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char *filename,
 				  unsigned int mode, uint32_t flags);
-extern struct lgfs2_inode *lgfs2_gfs_createi(struct lgfs2_inode *dip,
-				      const char *filename, unsigned int mode,
-				      uint32_t flags);
 extern void lgfs2_dirent2_del(struct lgfs2_inode *dip, struct lgfs2_buffer_head *bh,
 			struct gfs2_dirent *prev, struct gfs2_dirent *cur);
 extern int lgfs2_dir_search(struct lgfs2_inode *dip, const char *filename, int len,
-- 
2.39.0

[Cluster-devel] [PATCH 3/6] libgfs2: Reorganise lgfs2_createi()

[Andrew Price]

Move the lookup for an existing inode to lgfs2_createi itself and then
create the inode in __createi (renamed to do_createi) unconditionally.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/fs_ops.c | 80 ++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index 5003c1ae..0df78e5a 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -1497,9 +1497,10 @@ int lgfs2_write_filemeta(struct lgfs2_inode *ip)
 	return 0;
 }
 
-static struct lgfs2_inode *__createi(struct lgfs2_inode *dip, const char *filename,
-                                     unsigned int mode, uint32_t flags)
+static struct lgfs2_inode *do_createi(struct lgfs2_inode *dip, const char *filename,
+                                      unsigned int mode, uint32_t flags)
 {
+	struct lgfs2_inum parent = dip->i_num;
 	struct lgfs2_sbd *sdp = dip->i_sbd;
 	uint64_t bn;
 	struct lgfs2_inum inum;
@@ -1508,50 +1509,59 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode *dip, const char *filena
 	int err = 0;
 	int is_dir;
 
-	ip = lgfs2_lookupi(dip, filename, strlen(filename));
-	if (!ip) {
-		struct lgfs2_inum parent = dip->i_num;
-
-		err = lgfs2_dinode_alloc(sdp, 1, &bn);
-		if (err != 0)
-			return NULL;
-
-		if (sdp->gfs1)
-			inum.in_formal_ino = bn;
-		else
-			inum.in_formal_ino = sdp->md.next_inum++;
-		inum.in_addr = bn;
-
-		err = lgfs2_dir_add(dip, filename, strlen(filename), &inum, IF2DT(mode));
-		if (err)
-			return NULL;
+	err = lgfs2_dinode_alloc(sdp, 1, &bn);
+	if (err != 0)
+		return NULL;
 
-		if (sdp->gfs1)
-			is_dir = (IF2DT(mode) == GFS_FILE_DIR);
-		else
-			is_dir = S_ISDIR(mode);
-		if (is_dir) {
-			lgfs2_bmodified(dip->i_bh);
-			dip->i_nlink++;
-		}
+	if (sdp->gfs1)
+		inum.in_formal_ino = bn;
+	else
+		inum.in_formal_ino = sdp->md.next_inum++;
+	inum.in_addr = bn;
 
-		err = __init_dinode(sdp, &bh, &inum, mode, flags, &parent, sdp->gfs1);
-		if (err != 0)
-			return NULL;
+	err = lgfs2_dir_add(dip, filename, strlen(filename), &inum, IF2DT(mode));
+	if (err)
+		return NULL;
 
-		ip = lgfs2_inode_get(sdp, bh);
-		if (ip == NULL)
-			return NULL;
-		lgfs2_bmodified(bh);
+	if (sdp->gfs1)
+		is_dir = (IF2DT(mode) == GFS_FILE_DIR);
+	else
+		is_dir = S_ISDIR(mode);
+	if (is_dir) {
+		lgfs2_bmodified(dip->i_bh);
+		dip->i_nlink++;
 	}
+	err = __init_dinode(sdp, &bh, &inum, mode, flags, &parent, sdp->gfs1);
+	if (err != 0)
+		return NULL;
+
+	ip = lgfs2_inode_get(sdp, bh);
+	if (ip == NULL)
+		return NULL;
+	lgfs2_bmodified(bh);
 	ip->bh_owned = 1;
 	return ip;
 }
 
+/**
+ * Create an inode and link it into a directory. If it already exists, return
+ * the existing inode. To create gfs1 inodes, dip->i_sbd->gfs1 must be set.
+ * @dip: The inode of the parent directory.
+ * @filename: The new inode's filename.
+ * @mode: The mode of the new inode.
+ * @flags: The flags of the new inode.
+ * Returns the new or existing inode on success or NULL on failure with errno set.
+ */
 struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char *filename,
                                  unsigned int mode, uint32_t flags)
 {
-	return __createi(dip, filename, mode, flags);
+	struct lgfs2_inode *ip = lgfs2_lookupi(dip, filename, strlen(filename));
+
+	if (ip != NULL) {
+		ip->bh_owned = 1;
+		return ip;
+	}
+	return do_createi(dip, filename, mode, flags);
 }
 
 /**
-- 
2.39.0

[Cluster-devel] [PATCH 4/6] fsck.gfs2: Remove de variable from dirref_find()

[Andrew Price]

It always points to 'dentry' so it can be replaced in all uses.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/pass2.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 0c2e0146..002bf2cb 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -621,18 +621,17 @@ static int dirref_find(struct fsck_cx *cx, struct lgfs2_inode *ip, struct gfs2_d
 	/* the metawalk_fxn's private field must be set to the dentry
 	 * block we want to clear */
 	struct lgfs2_inum *entry = private;
-	struct lgfs2_dirent dentry, *de;
+	struct lgfs2_dirent dentry;
 	char fn[MAX_FILENAME];
 
 	memset(&dentry, 0, sizeof(dentry));
 	lgfs2_dirent_in(&dentry, dent);
-	de = &dentry;
 
-	if (de->dr_inum.in_addr != entry->in_addr) {
+	if (dentry.dr_inum.in_addr != entry->in_addr) {
 		(*count)++;
 		return 0;
 	}
-	if (de->dr_inum.in_formal_ino == be64_to_cpu(dent->de_inum.no_formal_ino)) {
+	if (dentry.dr_inum.in_formal_ino == be64_to_cpu(dent->de_inum.no_formal_ino)) {
 		log_debug("Formal inode number matches; must be a hard "
 			  "link.\n");
 		goto out;
@@ -641,13 +640,13 @@ static int dirref_find(struct fsck_cx *cx, struct lgfs2_inode *ip, struct gfs2_d
 		  "directory %"PRIu64" (0x%"PRIx64") has the wrong 'formal' inode "
 		  "number.\n"), entry->in_addr, entry->in_addr, ip->i_num.in_addr, ip->i_num.in_addr);
 	memset(fn, 0, sizeof(fn));
-	if (de->dr_name_len < MAX_FILENAME)
-		strncpy(fn, filename, de->dr_name_len);
+	if (dentry.dr_name_len < MAX_FILENAME)
+		strncpy(fn, filename, dentry.dr_name_len);
 	else
 		strncpy(fn, filename, MAX_FILENAME - 1);
 	log_err(_("The bad reference '%s' had formal inode number: %"PRIu64
 		  " (0x%"PRIx64") but the correct value is: %"PRIu64" (0x%"PRIx64")\n"),
-	        fn, de->dr_inum.in_formal_ino, de->dr_inum.in_formal_ino,
+	        fn, dentry.dr_inum.in_formal_ino, dentry.dr_inum.in_formal_ino,
 	        entry->in_formal_ino, entry->in_formal_ino);
 	if (!query(cx, _("Delete the bad directory entry? (y/n) "))) {
 		log_err(_("The corrupt directory entry was not fixed.\n"));
-- 
2.39.0

[Cluster-devel] [PATCH 5/6] fsck.gfs2: Fix wrong entry used in dentry comparison

[Andrew Price]

'dent' points to the on-disk data that 'dentry' was converted from, so
this comparison always evaluates to true. Compare to 'entry' instead.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/pass2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 002bf2cb..5327ee43 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -631,7 +631,7 @@ static int dirref_find(struct fsck_cx *cx, struct lgfs2_inode *ip, struct gfs2_d
 		(*count)++;
 		return 0;
 	}
-	if (dentry.dr_inum.in_formal_ino == be64_to_cpu(dent->de_inum.no_formal_ino)) {
+	if (dentry.dr_inum.in_formal_ino == entry->in_formal_ino) {
 		log_debug("Formal inode number matches; must be a hard "
 			  "link.\n");
 		goto out;
-- 
2.39.0

[Cluster-devel] [PATCH 6/6] fsck.gfs2: fix_hashtable: Decrement i_blocks when freeing leaf blocks

[Andrew Price]

fsck.gfs2 can leave i_blocks too large when it removes empty leaf
blocks, so decrease the count when freeing them.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/fsck/pass2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 5327ee43..71459978 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1334,6 +1334,7 @@ static int fix_hashtable(struct fsck_cx *cx, struct lgfs2_inode *ip, __be64 *tbl
 	    (dentry.dr_inum.in_formal_ino == 0)) {
 		lgfs2_brelse(lbh);
 		lgfs2_free_block(ip->i_sbd, leafblk);
+		ip->i_blocks--;
 		log_err(_("Out of place leaf block %"PRIu64" (0x%"PRIx64") had no "
 			"entries, so it was deleted.\n"),
 		        leafblk, leafblk);
-- 
2.39.0