All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements
@ 2020-08-06 13:37 Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 01/32] savemeta: Allow saving to /dev/null Andrew Price
                   ` (31 more replies)
  0 siblings, 32 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This is some groundwork for ongoing performance improvements to gfs2_edit savemeta (and fsck.gfs2 indirectly). These patches are generally beneficial and have survived a bunch of testing so I'm posting them now.

The first handful are scattered cleanups and general removal of gfs2_buffer_heads uses to make things more flexible.

The theme of the savemeta changes is mainly about creating a better split between reading and saving blocks so that we can make better decisions between those paths, e.g. in some cases we want to collect together ranges of blocks for reading when we know that they're consecutive.

Andrew Price (32):
  savemeta: Allow saving to /dev/null
  mkfs.gfs2: Fix strncpy warnings
  libgfs2: Separate out gfs2l's language API
  glocktop: Improve mount info handling
  savemeta: Don't save bad xattr blocks twice
  libgfs2: Remove gfs2_buffer_head from gfs_dinode_in()
  libgfs2: Remove gfs2_buffer_head from lgfs2_gfs_inode_get()
  libgfs2: Remove gfs2_buffer_head from lgfs2_write_journal_data()
  libgfs2: Move get_file_buf() into structures.c
  gfs2l: Remove uses of gfs2_buffer_heads
  libgfs2: No need to use gfs2_buffer_head in metapointer()
  gfs2_edit: Don't use gfs2_buffer_head in do_dinode_extended() args
  libgfs2: Add a display name field to struct lgfs2_metadata
  gfs2_edit: get_block_type() improvements
  gfs2_edit: Don't use gfs2_buffer_head in display_block_type()
  gfs2_edit: Don't use gfs2_buffer_head in display_gfs2()
  gfs2_edit: restore_block() improvements
  savemeta: Simplify di_save_len()
  savemeta: Remove gfs2_buffer_head from get_gfs_struct_info()
  savemeta: Remove gfs2_buffer_head from save_bh() (and rename it)
  savemeta: Don't use gfs2_buffer_head in save_leaf_chain()
  savemeta: Remove gfs2_buffer_head from save_block()
  savemeta: Split block reading from saving
  savemeta: Call get_struct_info() in the read path
  savemeta: Introduce multi-block reads
  savemeta: Process indirect pointers in chunks
  savemeta: Don't trim off trailing zeroes when compressing
  savemeta: Leaf block processing improvements
  savemeta: Remove some unnecessary reads from save_inode_data()
  savemeta: Remove some unnecessary jindex reading code
  savemeta: Move block range queue ops into functions
  restoremeta: Fix up superblock processing

 gfs2/convert/gfs2_convert.c |   2 +-
 gfs2/edit/gfs2hex.c         |  52 ++-
 gfs2/edit/gfs2hex.h         |   5 +-
 gfs2/edit/hexedit.c         | 170 +++------
 gfs2/edit/hexedit.h         |   9 +-
 gfs2/edit/journal.c         |  59 ++--
 gfs2/edit/savemeta.c        | 672 ++++++++++++++++++++++--------------
 gfs2/fsck/metawalk.c        |   2 +-
 gfs2/glocktop/glocktop.c    | 189 +++++-----
 gfs2/libgfs2/Makefile.am    |  14 +-
 gfs2/libgfs2/buf.c          |   4 +-
 gfs2/libgfs2/fs_ops.c       |  33 +-
 gfs2/libgfs2/gfs1.c         |  49 ++-
 gfs2/libgfs2/gfs2l.c        |   1 +
 gfs2/libgfs2/lang.c         | 121 ++++---
 gfs2/libgfs2/lang.h         |  15 +
 gfs2/libgfs2/libgfs2.h      |  32 +-
 gfs2/libgfs2/meta.c         |  30 +-
 gfs2/libgfs2/structures.c   |  52 ++-
 gfs2/mkfs/main_mkfs.c       |   4 +-
 tests/edit.at               |   8 +
 21 files changed, 826 insertions(+), 697 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 01/32] savemeta: Allow saving to /dev/null
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 02/32] mkfs.gfs2: Fix strncpy warnings Andrew Price
                   ` (30 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Guard against calling ftruncate() on special files, which is an EINVAL.
This allows test 'saving' to /dev/null e.g. to measure read performance
better.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 8 ++++++--
 tests/edit.at        | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 3bd35843..b44e887e 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -447,6 +447,7 @@ static struct metafd savemetaopen(char *out_fn, int gziplevel)
 	char gzmode[3] = "w9";
 	char dft_fn[] = DFT_SAVE_FILE;
 	mode_t mask = umask(S_IXUSR | S_IRWXG | S_IRWXO);
+	struct stat st;
 
 	mfd.gziplevel = gziplevel;
 
@@ -463,8 +464,11 @@ static struct metafd savemetaopen(char *out_fn, int gziplevel)
 		fprintf(stderr, "Can't open %s: %s\n", out_fn, strerror(errno));
 		exit(1);
 	}
-
-	if (ftruncate(mfd.fd, 0)) {
+	if (fstat(mfd.fd, &st) == -1) {
+		fprintf(stderr, "Failed to stat %s: %s\n", out_fn, strerror(errno));
+		exit(1);
+	}
+	if (S_ISREG(st.st_mode) && ftruncate(mfd.fd, 0)) {
 		fprintf(stderr, "Can't truncate %s: %s\n", out_fn, strerror(errno));
 		exit(1);
 	}
diff --git a/tests/edit.at b/tests/edit.at
index c9c341b9..d70d012e 100644
--- a/tests/edit.at
+++ b/tests/edit.at
@@ -53,3 +53,11 @@ GFS_TGT_REGEN
 AT_CHECK([gfs2_edit restoremeta test.meta $GFS_TGT], 0, [ignore], [ignore])
 AT_CHECK([fsck.gfs2 -n $GFS_TGT], 0, [ignore], [ignore])
 AT_CLEANUP
+
+AT_SETUP([Save metadata to /dev/null])
+AT_KEYWORDS(gfs2_edit edit)
+GFS_TGT_REGEN
+AT_CHECK([$GFS_MKFS -p lock_nolock $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([gfs2_edit savemeta -z0 $GFS_TGT /dev/null], 0, [ignore], [ignore])
+AT_CHECK([gfs2_edit savemeta $GFS_TGT /dev/null], 0, [ignore], [ignore])
+AT_CLEANUP
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 02/32] mkfs.gfs2: Fix strncpy warnings
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 01/32] savemeta: Allow saving to /dev/null Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 03/32] libgfs2: Separate out gfs2l's language API Andrew Price
                   ` (29 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

New in gcc 10

  warning: ?__builtin_strncpy? specified bound 64 equals destination size
  [-Wstringop-truncation]

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index bb8762b0..36fd08e7 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -1144,8 +1144,8 @@ int main(int argc, char *argv[])
 	build_root(&sbd);
 	sb.sb_root_dir = sbd.md.rooti->i_di.di_num;
 
-	strncpy(sb.sb_lockproto, opts.lockproto, GFS2_LOCKNAME_LEN);
-	strncpy(sb.sb_locktable, opts.locktable, GFS2_LOCKNAME_LEN);
+	strncpy(sb.sb_lockproto, opts.lockproto, GFS2_LOCKNAME_LEN - 1);
+	strncpy(sb.sb_locktable, opts.locktable, GFS2_LOCKNAME_LEN - 1);
 	sb.sb_lockproto[GFS2_LOCKNAME_LEN - 1] = '\0';
 	sb.sb_locktable[GFS2_LOCKNAME_LEN - 1] = '\0';
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 03/32] libgfs2: Separate out gfs2l's language API
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 01/32] savemeta: Allow saving to /dev/null Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 02/32] mkfs.gfs2: Fix strncpy warnings Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 04/32] glocktop: Improve mount info handling Andrew Price
                   ` (28 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

It doesn't really make sense to have this interpreter be part of
libgfs2, at least while the language isn't rich enough to be broadly
useful.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/Makefile.am | 14 ++++++++------
 gfs2/libgfs2/gfs2l.c     |  1 +
 gfs2/libgfs2/lang.h      | 15 +++++++++++++++
 gfs2/libgfs2/libgfs2.h   | 19 -------------------
 4 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/gfs2/libgfs2/Makefile.am b/gfs2/libgfs2/Makefile.am
index 76df920e..03a955f2 100644
--- a/gfs2/libgfs2/Makefile.am
+++ b/gfs2/libgfs2/Makefile.am
@@ -41,10 +41,7 @@ libgfs2_la_SOURCES = \
 	fs_ops.c \
 	recovery.c \
 	structures.c \
-	meta.c \
-	lang.c \
-	parser.y \
-	lexer.l
+	meta.c
 
 libgfs2_la_CPPFLAGS = \
 	-D_FILE_OFFSET_BITS=64 \
@@ -53,13 +50,18 @@ libgfs2_la_CPPFLAGS = \
 	-I$(top_srcdir)/gfs2/include \
 	$(uuid_CFLAGS)
 
-gfs2l_SOURCES = gfs2l.c
+gfs2l_SOURCES = \
+	gfs2l.c \
+	lang.c \
+	parser.y \
+	lexer.l
 gfs2l_LDADD = \
 	libgfs2.la \
 	$(uuid_LIBS)
 gfs2l_CPPFLAGS = \
 	-I$(top_srcdir)/gfs2/include \
-	-D_FILE_OFFSET_BITS=64
+	-D_FILE_OFFSET_BITS=64 \
+	$(uuid_CFLAGS)
 
 # Autotools can't handle header files output by flex so we have to generate it manually
 lexer.h: lexer.l
diff --git a/gfs2/libgfs2/gfs2l.c b/gfs2/libgfs2/gfs2l.c
index ba2d5f15..bc42e888 100644
--- a/gfs2/libgfs2/gfs2l.c
+++ b/gfs2/libgfs2/gfs2l.c
@@ -1,6 +1,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <string.h>
+#include "lang.h"
 #include "libgfs2.h"
 
 static void usage(const char *cmd)
diff --git a/gfs2/libgfs2/lang.h b/gfs2/libgfs2/lang.h
index 7d9a6e98..f74a57bd 100644
--- a/gfs2/libgfs2/lang.h
+++ b/gfs2/libgfs2/lang.h
@@ -12,6 +12,21 @@ struct lgfs2_lang_state {
 	struct ast_node *ls_interp_curr;
 };
 
+struct lgfs2_lang_result {
+	uint64_t lr_blocknr;
+	struct gfs2_buffer_head *lr_bh;
+	const struct lgfs2_metadata *lr_mtype;
+	int lr_state; // GFS2_BLKST_*
+};
+
+extern struct lgfs2_lang_state *lgfs2_lang_init(void);
+extern int lgfs2_lang_parsef(struct lgfs2_lang_state *state, FILE *script);
+extern int lgfs2_lang_parses(struct lgfs2_lang_state *state, const char *script);
+extern struct lgfs2_lang_result *lgfs2_lang_result_next(struct lgfs2_lang_state *state, struct gfs2_sbd *sbd);
+extern int lgfs2_lang_result_print(struct lgfs2_lang_result *result);
+extern void lgfs2_lang_result_free(struct lgfs2_lang_result **result);
+extern void lgfs2_lang_free(struct lgfs2_lang_state **state);
+
 typedef enum {
 	AST_NONE,
 	// Statements
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index cf225af5..59fb18e8 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -769,25 +769,6 @@ extern void gfs2_log_descriptor_print(const struct gfs2_log_descriptor *ld);
 extern void gfs2_statfs_change_print(const struct gfs2_statfs_change *sc);
 extern void gfs2_quota_change_print(const struct gfs2_quota_change *qc);
 
-/* Language functions */
-
-struct lgfs2_lang_state;
-
-struct lgfs2_lang_result {
-	uint64_t lr_blocknr;
-	struct gfs2_buffer_head *lr_bh;
-	const struct lgfs2_metadata *lr_mtype;
-	int lr_state; // GFS2_BLKST_*
-};
-
-extern struct lgfs2_lang_state *lgfs2_lang_init(void);
-extern int lgfs2_lang_parsef(struct lgfs2_lang_state *state, FILE *script);
-extern int lgfs2_lang_parses(struct lgfs2_lang_state *state, const char *script);
-extern struct lgfs2_lang_result *lgfs2_lang_result_next(struct lgfs2_lang_state *state, struct gfs2_sbd *sbd);
-extern int lgfs2_lang_result_print(struct lgfs2_lang_result *result);
-extern void lgfs2_lang_result_free(struct lgfs2_lang_result **result);
-extern void lgfs2_lang_free(struct lgfs2_lang_state **state);
-
 __END_DECLS
 
 #endif /* __LIBGFS2_DOT_H__ */
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 04/32] glocktop: Improve mount info handling
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (2 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 03/32] libgfs2: Separate out gfs2l's language API Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 05/32] savemeta: Don't save bad xattr blocks twice Andrew Price
                   ` (27 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Using static arrays for the mount points causes glocktop to impose some
arbitrary limits, like 80 chars for device and mount point paths.
Improve on this situation by keeping the mount point info in a simple
linked list and add some error handling in various places. Also remove a
bunch of stale code from read_superblock. No more gfs2_buffer_heads in
glocktop as a side effect.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/glocktop/glocktop.c | 189 ++++++++++++++++++++-------------------
 1 file changed, 99 insertions(+), 90 deletions(-)

diff --git a/gfs2/glocktop/glocktop.c b/gfs2/glocktop/glocktop.c
index 2f74a7f2..bd7fcf02 100644
--- a/gfs2/glocktop/glocktop.c
+++ b/gfs2/glocktop/glocktop.c
@@ -26,7 +26,6 @@
 
 #define MAX_GLOCKS 20
 #define MAX_LINES 6000
-#define MAX_MOUNT_POINTS 100
 #define MAX_FILES 512
 #define MAX_CALLTRACE_LINES 4
 #define TITLE1 "glocktop - GFS2 glock monitor"
@@ -144,7 +143,7 @@ enum summary_types {
 	stypes = 7,
 };
 
-char debugfs[PATH_MAX];
+char *debugfs;
 int termcols = 80, termlines = 30, done = 0;
 unsigned glocks = 0;
 const char *termtype;
@@ -152,10 +151,14 @@ WINDOW *wind;
 int bufsize = 4 * 1024 * 1024;
 char *glock[MAX_GLOCKS];
 int iterations = 0, show_reservations = 0, iters_done = 0;
-char devices[MAX_MOUNT_POINTS][80];
-char mount_points[MAX_MOUNT_POINTS][80];
-int fs_fd[MAX_MOUNT_POINTS];
-int mounted = 0;
+struct mount_point {
+	struct mount_point *next;
+	char *device;
+	char *dir;
+	int fd;
+	struct gfs2_sb sb;
+};
+struct mount_point *mounts;
 char dlmwlines[100][96]; /* waiters lines */
 char dlmglines[MAX_LINES][97]; /* granted lines */
 char contended_filenames[MAX_FILES][PATH_MAX];
@@ -165,11 +168,6 @@ int line = 0;
 const char *prog_name;
 char dlm_dirtbl_size[32], dlm_rsbtbl_size[32], dlm_lkbtbl_size[32];
 int bsize = 0;
-struct gfs2_sb sd_sb[MAX_MOUNT_POINTS];
-int sd_diptrs = 0, sd_inptrs = 0;
-uint64_t sd_heightsize[GFS2_MAX_META_HEIGHT];
-uint64_t sd_jheightsize[GFS2_MAX_META_HEIGHT];
-int sd_max_height, sd_max_jheight;
 char print_dlm_grants = 1;
 char *gbuf = NULL; /* glocks buffer */
 char *gpos = NULL;
@@ -225,89 +223,104 @@ static void UpdateSize(int sig)
 	signal(SIGWINCH, UpdateSize);
 }
 
-static void read_superblock(int fd, int mntpt)
+static int read_superblock(int fd, struct gfs2_sb *sb)
 {
-	struct gfs2_sbd sbd = { .device_fd = fd, .bsize = GFS2_BASIC_BLOCK };
-	struct gfs2_buffer_head *bh;
-	int x;
-	uint64_t space = 0;
+	ssize_t r;
+	char *buf;
 
 	ioctl(fd, BLKFLSBUF, 0);
-	bh = bread(&sbd, GFS2_SB_ADDR);
-	gfs2_sb_in(&sd_sb[mntpt], bh->b_data);
-	bsize = sd_sb[mntpt].sb_bsize;
+	buf = calloc(1, GFS2_BASIC_BLOCK);
+	if (buf == NULL) {
+		perror("Failed to read superblock");
+		return -1;
+	}
+	r = pread(fd, buf, GFS2_BASIC_BLOCK, GFS2_BASIC_BLOCK * GFS2_SB_ADDR);
+	if (r != GFS2_BASIC_BLOCK) {
+		perror("Failed to read superblock");
+		free(buf);
+		return -1;
+	}
+	gfs2_sb_in(sb, buf);
+	free(buf);
+
+	bsize = sb->sb_bsize;
 	if (!bsize)
 		bsize = 4096;
-	sd_inptrs = (bsize - sizeof(struct gfs2_meta_header)) /
-		sizeof(uint64_t);
-	sd_diptrs = (bsize - sizeof(struct gfs2_dinode)) /
-		sizeof(uint64_t);
-	sd_heightsize[0] = bsize - sizeof(struct gfs2_dinode);
-	sd_heightsize[1] = bsize * sd_diptrs;
-	for (x = 2; ; x++) {
-		space = sd_heightsize[x - 1] * sd_inptrs;
-		if (space / sd_inptrs != sd_heightsize[x - 1] ||
-		    space % sd_inptrs != 0)
-			break;
-		sd_heightsize[x] = space;
-	}
-	sd_jheightsize[0] = bsize - sizeof(struct gfs2_dinode);
-	sd_jheightsize[1] = (bsize - sizeof(struct gfs2_meta_header)) *
-		sd_diptrs;
-	for (x = 2; ; x++){
-		space = sd_jheightsize[x - 1] * sd_inptrs;
-		if (space / sd_inptrs != sd_jheightsize[x - 1] ||
-		    space % sd_inptrs != 0)
-			break;
-		sd_jheightsize[x] = space;
+	return 0;
+}
+
+static void free_mounts(void)
+{
+	struct mount_point *mntp = mounts;
+
+	while (mntp != NULL) {
+		struct mount_point *nextp = mntp->next;
+
+		close(mntp->fd);
+		free(mntp);
+		mntp = nextp;
 	}
-	sd_max_jheight = x;
+	mounts = NULL;
 }
 
 static int parse_mounts(void)
 {
-	char str[PATH_MAX], dev[PATH_MAX], mnt[PATH_MAX], mtype[PATH_MAX];
-	char opts[PATH_MAX];
+	struct mount_point **mntp = &mounts;
+	struct mntent *mnt;
 	FILE *fp;
 
-	memset(debugfs, 0, sizeof(debugfs));
-	memset(mount_points, 0, sizeof(mount_points));
-	memset(devices, 0, sizeof(devices));
-
-	fp = fopen("/proc/mounts", "rt");
+	fp = setmntent("/proc/mounts", "r");
 	if (fp == NULL) {
 		perror("/proc/mounts");
 		return 1;
 	}
-	while (fgets(str, sizeof(str) - 1, fp)) {
-		sscanf(str, "%s %s %s %s", dev, mnt, mtype, opts);
-		if (!strcmp(mtype, "debugfs")) {
-			strcpy(debugfs, mnt);
+	while ((mnt = getmntent(fp)) != NULL) {
+		size_t devlen = strlen(mnt->mnt_fsname);
+		size_t dirlen = strlen(mnt->mnt_dir);
+		struct mount_point *newmnt;
+
+		if (strcmp(mnt->mnt_type, "debugfs") == 0) {
+			if (debugfs == NULL)
+				debugfs = strdup(mnt->mnt_dir);
 			continue;
 		}
-		if (strcmp(mtype, "gfs2")) /* if not gfs2 */
+		if (strcmp(mnt->mnt_type, "gfs2") != 0)
 			continue;
 
-		strncpy(mount_points[mounted], mnt, 79);
-		mount_points[mounted][79] = '\0';
-		strncpy(devices[mounted], dev, 79);
-		devices[mounted][79] = '\0';
-
-		/* Now find out the mount point's file system name */
-		fs_fd[mounted] = open(dev, O_RDONLY);
-		if (fs_fd[mounted])
-			read_superblock(fs_fd[mounted], mounted);
-		mounted++;
+		newmnt = calloc(1, sizeof(*newmnt) + devlen + 1 + dirlen + 1);
+		if (newmnt == NULL) {
+			perror("Failed to gather mount points");
+			return 1;
+		}
+		newmnt->device = (char *)(newmnt + 1);
+		newmnt->dir = newmnt->device + devlen + 1;
+		strcpy(newmnt->device, mnt->mnt_fsname);
+		strcpy(newmnt->dir, mnt->mnt_dir);
+		newmnt->fd = open(newmnt->device, O_RDONLY);
+
+		if (newmnt->fd < 0) {
+			fprintf(stderr, "Failed to open gfs2 filesystem '%s': %s\n",
+			        newmnt->device, strerror(errno));
+			free(newmnt);
+			continue;
+		}
+		if (read_superblock(newmnt->fd, &newmnt->sb) != 0) {
+			free(newmnt);
+			continue;
+		}
+		*mntp = newmnt;
+		mntp = &newmnt->next;
 	}
-	if (debugfs[0] == '\0') {
-		if (mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL)){
-			fprintf(stderr, "Unable to mount debugfs.\n");
-			fprintf(stderr, "Please mount it manually.\n");
-			exit(-1);
+	endmntent(fp);
+	if (debugfs == NULL) {
+		debugfs = strdup("/sys/kernel/debug");
+		if (!debugfs || mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL)) {
+			perror("debugfs not mounted and an attempt to mount it failed");
+			free(debugfs);
+			free_mounts();
+			return 1;
 		}
-		strcpy(debugfs, "/sys/kernel/debug");
 	}
-	fclose(fp);
 	return 0;
 }
 
@@ -562,19 +575,17 @@ void print_it(const char *label, const char *fmt, const char *fmt2, ...)
 static void display_filename(int fd, unsigned long long block,
 			     unsigned long long dirarray[256], int subdepth)
 {
+	struct mount_point *mp;
 	int i, subs;
-	char *mntpt = NULL;
 	char blk[32];
 	DIR *dir = NULL;
 	struct dirent *dent;
 
-	for (i = 0; i < mounted; i++) {
-		if (fd == fs_fd[i]) {
-			mntpt = mount_points[i];
+	for (mp = mounts; mp != NULL; mp = mp->next) {
+		if (fd == mp->fd)
 			break;
-		}
 	}
-	if (i == mounted)
+	if (mp == NULL)
 		return;
 	for (i = 0; i < contended_count; i++) {
 		if (contended_blocks[i] == block) {
@@ -584,7 +595,7 @@ static void display_filename(int fd, unsigned long long block,
 	sprintf(blk, "%lld", block);
 	if (i >= contended_count) {
 		memset(contended_filenames[i], 0, PATH_MAX);
-		strcat(contended_filenames[i], mntpt);
+		strcat(contended_filenames[i], mp->dir);
 		for (subs = subdepth - 2; subs >= 0; subs--) {
 			dir = opendir(contended_filenames[i]);
 			while ((dent = readdir(dir))) {
@@ -655,26 +666,28 @@ static const char *show_inode(const char *id, int fd, unsigned long long block)
 static const char *show_details(const char *id, const char *fsname, int btype,
 				int trace_dir_path)
 {
-	int mnt_num;
 	unsigned long long block = 0;
 	const char *blk_type = NULL;
+	struct mount_point *mp;
 	FILE *dlmf;
 
 	/* Figure out which mount point corresponds to this debugfs id */
-	for (mnt_num = 0; mnt_num < mounted; mnt_num++) {
+	for (mp = mounts; mp != NULL; mp = mp->next) {
 		char *p;
 
-		p = strchr(sd_sb[mnt_num].sb_locktable, ':');
+		p = strchr(mp->sb.sb_locktable, ':');
 		if (!p)
 			continue;
 		p++;
 		if (!strcmp(p, fsname))
 			break;
 	}
+	if (mp == NULL)
+		return "unknown";
 	memset(dlm_dirtbl_size, 0, sizeof(dlm_dirtbl_size));
 	memset(dlm_rsbtbl_size, 0, sizeof(dlm_rsbtbl_size));
 	memset(dlm_lkbtbl_size, 0, sizeof(dlm_lkbtbl_size));
-	if (!strcmp(sd_sb[mnt_num].sb_lockproto, "lock_dlm")) {
+	if (!strcmp(mp->sb.sb_lockproto, "lock_dlm")) {
 		char *sp;
 		char *p;
 
@@ -720,16 +733,12 @@ static const char *show_details(const char *id, const char *fsname, int btype,
 		strcpy(dlm_lkbtbl_size, "nolock");
 	}
 
-	if (mnt_num >= mounted) /* can't find the right superblock */
-		return "unknown";
-
 	/* Read the inode in so we can see its type. */
 	sscanf(id, "%llx", &block);
 	if (block) {
 		if (btype == 2)
 			if (trace_dir_path)
-				blk_type = show_inode(id, fs_fd[mnt_num],
-						      block);
+				blk_type = show_inode(id, mp->fd, block);
 			else
 				blk_type = "";
 		else
@@ -1658,7 +1667,7 @@ int main(int argc, char **argv)
 	int refresh_time = REFRESH_TIME;
 	fd_set readfds;
 	char string[96];
-	int ch, i, dlmwaiters = 0, dlmgrants = 0;
+	int ch, dlmwaiters = 0, dlmgrants = 0;
 	int cont = TRUE, optchar;
 	int trace_dir_path = 0;
 	int show_held = 1, help = 0;
@@ -1894,10 +1903,10 @@ int main(int argc, char **argv)
 		if (iterations && iters_done >= iterations)
 			break;
 	}
-	for (i = 0; i < mounted; i++)
-		close(fs_fd[i]);
+	free_mounts();
 	free(gbuf);
 	free(dbuf);
+	free(debugfs);
 	if (interactive) {
 		refresh();
 		endwin();
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 05/32] savemeta: Don't save bad xattr blocks twice
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (3 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 04/32] glocktop: Improve mount info handling Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 06/32] libgfs2: Remove gfs2_buffer_head from gfs_dinode_in() Andrew Price
                   ` (26 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This could only happen if the xattr block has a corrupt mh_type.

We've already saved the xattr block by the time we figure out that we
can't recognise its mh type so don't bother saving it again there.

Also relax mh type checking for the xattr blocks so that we save as much
of the (possibly corrupt) fs as possible. Just use the mh_type field to
decide whether to save indirect pointers or xattrs.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index b44e887e..10e3bcce 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -789,31 +789,15 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 		}
 	}
 	if (inode->i_di.di_eattr) { /* if this inode has extended attributes */
-		struct gfs2_meta_header mh;
 		struct gfs2_buffer_head *lbh;
+		int mhtype;
 
 		lbh = bread(&sbd, inode->i_di.di_eattr);
-		save_block(sbd.device_fd, mfd, inode->i_di.di_eattr, iblk, NULL);
-		gfs2_meta_header_in(&mh, lbh->b_data);
-		if (mh.mh_magic == GFS2_MAGIC &&
-		    mh.mh_type == GFS2_METATYPE_EA)
+		save_block(sbd.device_fd, mfd, inode->i_di.di_eattr, iblk, &mhtype);
+		if (mhtype == GFS2_METATYPE_EA)
 			save_ea_block(mfd, lbh->b_data, iblk);
-		else if (mh.mh_magic == GFS2_MAGIC &&
-			 mh.mh_type == GFS2_METATYPE_IN)
+		else if (mhtype == GFS2_METATYPE_IN)
 			save_indirect_blocks(mfd, cur_list, lbh, iblk, 2, 2);
-		else {
-			if (mh.mh_magic == GFS2_MAGIC) /* if it's metadata */
-				save_block(sbd.device_fd, mfd, inode->i_di.di_eattr,
-				           iblk, NULL);
-			fprintf(stderr,
-				"\nWarning: corrupt extended "
-				"attribute at block %llu (0x%llx) "
-				"detected in inode %lld (0x%llx).\n",
-				(unsigned long long)inode->i_di.di_eattr,
-				(unsigned long long)inode->i_di.di_eattr,
-				(unsigned long long)iblk,
-				(unsigned long long)iblk);
-		}
 		brelse(lbh);
 	}
 	inode_put(&inode);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 06/32] libgfs2: Remove gfs2_buffer_head from gfs_dinode_in()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (4 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 05/32] savemeta: Don't save bad xattr blocks twice Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 07/32] libgfs2: Remove gfs2_buffer_head from lgfs2_gfs_inode_get() Andrew Price
                   ` (25 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/hexedit.h |  1 -
 gfs2/libgfs2/gfs1.c | 11 ++++-------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/gfs2/edit/hexedit.h b/gfs2/edit/hexedit.h
index d2992d8e..ab2d44d6 100644
--- a/gfs2/edit/hexedit.h
+++ b/gfs2/edit/hexedit.h
@@ -227,7 +227,6 @@ extern void gfs_jindex_in(struct gfs_jindex *jindex, char *buf);
 extern void gfs_log_header_in(struct gfs_log_header *head,
 			      struct gfs2_buffer_head *bh);
 extern void gfs_log_header_print(struct gfs_log_header *lh);
-extern void gfs_dinode_in(struct gfs_dinode *di, struct gfs2_buffer_head *bh);
 extern void savemeta(char *out_fn, int saveoption, int gziplevel);
 extern void restoremeta(const char *in_fn, const char *out_device,
 			uint64_t printblocksonly);
diff --git a/gfs2/libgfs2/gfs1.c b/gfs2/libgfs2/gfs1.c
index 16c183b5..7da23168 100644
--- a/gfs2/libgfs2/gfs1.c
+++ b/gfs2/libgfs2/gfs1.c
@@ -248,14 +248,11 @@ int gfs1_writei(struct gfs2_inode *ip, char *buf, uint64_t offset,
 	return copied;
 }
 
-/* ------------------------------------------------------------------------ */
-/* gfs_dinode_in */
-/* ------------------------------------------------------------------------ */
-static void gfs_dinode_in(struct gfs_dinode *di, struct gfs2_buffer_head *bh)
+static void gfs_dinode_in(struct gfs_dinode *di, char *buf)
 {
-	struct gfs_dinode *str = (struct gfs_dinode *)bh->b_data;
+	struct gfs_dinode *str = (struct gfs_dinode *)buf;
 
-	gfs2_meta_header_in(&di->di_header, bh->b_data);
+	gfs2_meta_header_in(&di->di_header, buf);
 	gfs2_inum_in(&di->di_num, (char *)&str->di_num);
 
 	di->di_mode = be32_to_cpu(str->di_mode);
@@ -297,7 +294,7 @@ static struct gfs2_inode *__gfs_inode_get(struct gfs2_sbd *sdp,
 		bh = bread(sdp, di_addr);
 		ip->bh_owned = 1;
 	}
-	gfs_dinode_in(&gfs1_dinode, bh);
+	gfs_dinode_in(&gfs1_dinode, bh->b_data);
 	memcpy(&ip->i_di.di_header, &gfs1_dinode.di_header,
 	       sizeof(struct gfs2_meta_header));
 	memcpy(&ip->i_di.di_num, &gfs1_dinode.di_num,
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 07/32] libgfs2: Remove gfs2_buffer_head from lgfs2_gfs_inode_get()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (5 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 06/32] libgfs2: Remove gfs2_buffer_head from gfs_dinode_in() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 08/32] libgfs2: Remove gfs2_buffer_head from lgfs2_write_journal_data() Andrew Price
                   ` (24 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Give lgfs2_gfs_inode_read() its own reading code for now. Removing bh's
from that will involve removing the i_bh field from gfs2_inode, which
has a lot of users.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/convert/gfs2_convert.c |  2 +-
 gfs2/edit/savemeta.c        |  4 ++--
 gfs2/fsck/metawalk.c        |  2 +-
 gfs2/libgfs2/gfs1.c         | 33 ++++++++++++++++++---------------
 gfs2/libgfs2/libgfs2.h      |  6 ++----
 5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index 7de0f924..54a6ec1b 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -867,7 +867,7 @@ static int adjust_inode(struct gfs2_sbd *sbp, struct gfs2_buffer_head *bh)
 	struct inode_block *fixdir;
 	int inode_was_gfs1;
 
-	inode = lgfs2_gfs_inode_get(sbp, bh);
+	inode = lgfs2_gfs_inode_get(sbp, bh->b_data);
 	if (inode == NULL) {
 		log_crit(_("Error reading inode: %s\n"), strerror(errno));
 		return -1;
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 10e3bcce..cbecf281 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -303,7 +303,7 @@ static size_t di_save_len(struct gfs2_buffer_head *bh, uint64_t owner)
 	size_t len;
 
 	if (sbd.gfs1)
-		inode = lgfs2_gfs_inode_get(&sbd, bh);
+		inode = lgfs2_gfs_inode_get(&sbd, bh->b_data);
 	else
 		inode = lgfs2_inode_get(&sbd, bh);
 
@@ -724,7 +724,7 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 		osi_list_init(&metalist[i]);
 	metabh = bread(&sbd, iblk);
 	if (sbd.gfs1) {
-		inode = lgfs2_gfs_inode_get(&sbd, metabh);
+		inode = lgfs2_gfs_inode_get(&sbd, metabh->b_data);
 	} else {
 		inode = lgfs2_inode_get(&sbd, metabh);
 	}
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 265fa760..a425e884 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -292,7 +292,7 @@ struct gfs2_inode *fsck_inode_get(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
 		return sysip;
 
 	if (sdp->gfs1)
-		ip = lgfs2_gfs_inode_get(sdp, bh);
+		ip = lgfs2_gfs_inode_get(sdp, bh->b_data);
 	else
 		ip = lgfs2_inode_get(sdp, bh);
 	if (ip)
diff --git a/gfs2/libgfs2/gfs1.c b/gfs2/libgfs2/gfs1.c
index 7da23168..29e4f12d 100644
--- a/gfs2/libgfs2/gfs1.c
+++ b/gfs2/libgfs2/gfs1.c
@@ -277,9 +277,7 @@ static void gfs_dinode_in(struct gfs_dinode *di, char *buf)
 	di->di_eattr = be64_to_cpu(str->di_eattr);
 }
 
-static struct gfs2_inode *__gfs_inode_get(struct gfs2_sbd *sdp,
-					  struct gfs2_buffer_head *bh,
-					  uint64_t di_addr)
+static struct gfs2_inode *__gfs_inode_get(struct gfs2_sbd *sdp, char *buf, uint64_t di_addr)
 {
 	struct gfs_dinode gfs1_dinode;
 	struct gfs2_inode *ip;
@@ -288,13 +286,7 @@ static struct gfs2_inode *__gfs_inode_get(struct gfs2_sbd *sdp,
 	if (ip == NULL) {
 		return NULL;
 	}
-
-	ip->bh_owned = 0;
-	if (!bh) {
-		bh = bread(sdp, di_addr);
-		ip->bh_owned = 1;
-	}
-	gfs_dinode_in(&gfs1_dinode, bh->b_data);
+	gfs_dinode_in(&gfs1_dinode, buf);
 	memcpy(&ip->i_di.di_header, &gfs1_dinode.di_header,
 	       sizeof(struct gfs2_meta_header));
 	memcpy(&ip->i_di.di_num, &gfs1_dinode.di_num,
@@ -319,20 +311,31 @@ static struct gfs2_inode *__gfs_inode_get(struct gfs2_sbd *sdp,
 	ip->i_di.di_depth = gfs1_dinode.di_depth;
 	ip->i_di.di_entries = gfs1_dinode.di_entries;
 	ip->i_di.di_eattr = gfs1_dinode.di_eattr;
-	ip->i_bh = bh;
 	ip->i_sbd = sdp;
 	return ip;
 }
 
-struct gfs2_inode *lgfs2_gfs_inode_get(struct gfs2_sbd *sdp,
-				 struct gfs2_buffer_head *bh)
+struct gfs2_inode *lgfs2_gfs_inode_get(struct gfs2_sbd *sdp, char *buf)
 {
-	return __gfs_inode_get(sdp, bh, 0);
+	return __gfs_inode_get(sdp, buf, 0);
 }
 
 struct gfs2_inode *lgfs2_gfs_inode_read(struct gfs2_sbd *sdp, uint64_t di_addr)
 {
-	return __gfs_inode_get(sdp, NULL, di_addr);
+	struct gfs2_buffer_head *bh;
+	struct gfs2_inode *ip;
+
+	bh = bget(sdp, di_addr);
+	if (bh == NULL)
+		return NULL;
+	if (pread(sdp->device_fd, bh->b_data, sdp->bsize, di_addr * sdp->bsize) != sdp->bsize) {
+		brelse(bh);
+		return NULL;
+	}
+	ip = __gfs_inode_get(sdp, bh->b_data, di_addr);
+	ip->i_bh = bh;
+	ip->bh_owned = 1;
+	return ip;
 }
 
 /* ------------------------------------------------------------------------ */
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 59fb18e8..a34e77a7 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -639,10 +639,8 @@ extern void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 extern int gfs1_writei(struct gfs2_inode *ip, char *buf, uint64_t offset,
 		       unsigned int size);
 extern int gfs1_ri_update(struct gfs2_sbd *sdp, int fd, int *rgcount, int quiet);
-extern struct gfs2_inode *lgfs2_gfs_inode_get(struct gfs2_sbd *sdp,
-					struct gfs2_buffer_head *bh);
-extern struct gfs2_inode *lgfs2_gfs_inode_read(struct gfs2_sbd *sdp,
-					 uint64_t di_addr);
+extern struct gfs2_inode *lgfs2_gfs_inode_get(struct gfs2_sbd *sdp, char *buf);
+extern struct gfs2_inode *lgfs2_gfs_inode_read(struct gfs2_sbd *sdp, uint64_t di_addr);
 extern void gfs_jindex_in(struct gfs_jindex *jindex, char *buf);
 extern void gfs_rgrp_in(struct gfs_rgrp *rg, const char *buf);
 extern void gfs_rgrp_out(const struct gfs_rgrp *rg, char *buf);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 08/32] libgfs2: Remove gfs2_buffer_head from lgfs2_write_journal_data()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (6 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 07/32] libgfs2: Remove gfs2_buffer_head from lgfs2_gfs_inode_get() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 09/32] libgfs2: Move get_file_buf() into structures.c Andrew Price
                   ` (23 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

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

diff --git a/gfs2/libgfs2/structures.c b/gfs2/libgfs2/structures.c
index 36e02c2f..0aca7cd0 100644
--- a/gfs2/libgfs2/structures.c
+++ b/gfs2/libgfs2/structures.c
@@ -156,41 +156,41 @@ int lgfs2_write_journal_data(struct gfs2_inode *ip)
 		.lh_flags = GFS2_LOG_HEAD_UNMOUNT,
 #endif
 	};
-	struct gfs2_buffer_head *bh;
 	struct gfs2_sbd *sdp = ip->i_sbd;
 	unsigned blocks = (ip->i_di.di_size + sdp->bsize - 1) / sdp->bsize;
 	uint64_t jext0 = ip->i_di.di_num.no_addr + ip->i_di.di_blocks - blocks;
 	uint64_t seq = ((blocks) * (random() / (RAND_MAX + 1.0)));
+	uint64_t jblk = jext0;
+	char *buf;
 
-	bh = bget(sdp, jext0);
-	if (bh == NULL)
+	buf = calloc(1, sdp->bsize);
+	if (buf == NULL)
 		return -1;
 
 	crc32c_optimization_init();
 	do {
-		struct gfs2_log_header *buflh = (struct gfs2_log_header *)bh->b_data;
+		struct gfs2_log_header *buflh = (struct gfs2_log_header *)buf;
 
 		lh.lh_sequence = seq;
-		lh.lh_blkno = bh->b_blocknr - jext0;
-		gfs2_log_header_out(&lh, bh->b_data);
+		lh.lh_blkno = jblk - jext0;
+		gfs2_log_header_out(&lh, buf);
 
-		buflh->lh_hash = cpu_to_be32(lgfs2_log_header_hash(bh->b_data));
+		buflh->lh_hash = cpu_to_be32(lgfs2_log_header_hash(buf));
 #ifdef GFS2_HAS_LH_V2
-		buflh->lh_addr = cpu_to_be64(bh->b_blocknr);
-		buflh->lh_crc = cpu_to_be32(lgfs2_log_header_crc(bh->b_data, sdp->bsize));
+		buflh->lh_addr = cpu_to_be64(jblk);
+		buflh->lh_crc = cpu_to_be32(lgfs2_log_header_crc(buf, sdp->bsize));
 #endif
-
-		if (bwrite(bh)) {
-			free(bh);
+		if (pwrite(sdp->device_fd, buf, sdp->bsize, jblk * sdp->bsize) != sdp->bsize) {
+			free(buf);
 			return -1;
 		}
 
 		if (++seq == blocks)
 			seq = 0;
 
-	} while (++bh->b_blocknr < jext0 + blocks);
+	} while (++jblk < jext0 + blocks);
 
-	free(bh);
+	free(buf);
 	return 0;
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 09/32] libgfs2: Move get_file_buf() into structures.c
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (7 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 08/32] libgfs2: Remove gfs2_buffer_head from lgfs2_write_journal_data() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 10/32] gfs2l: Remove uses of gfs2_buffer_heads Andrew Price
                   ` (22 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

It's only called from there. Open code inode_is_stuffed() as it's simple
enough.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/fs_ops.c     | 27 ---------------------------
 gfs2/libgfs2/libgfs2.h    |  2 --
 gfs2/libgfs2/structures.c | 24 ++++++++++++++++++++++++
 3 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index b5363c05..ee512f67 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -744,33 +744,6 @@ int __gfs2_writei(struct gfs2_inode *ip, void *buf,
 	return copied;
 }
 
-struct gfs2_buffer_head *get_file_buf(struct gfs2_inode *ip, uint64_t lbn,
-				      int prealloc)
-{
-	struct gfs2_sbd *sdp = ip->i_sbd;
-	uint64_t dbn;
-	int new = TRUE;
-
-	if (inode_is_stuffed(ip))
-		unstuff_dinode(ip);
-
-	block_map(ip, lbn, &new, &dbn, NULL, prealloc);
-	if (!dbn) {
-		fprintf(stderr, "get_file_buf\n");
-		exit(1);
-	}
-
-	if (!prealloc && new &&
-	    ip->i_di.di_size < (lbn + 1) << sdp->sd_sb.sb_bsize_shift) {
-		bmodified(ip->i_bh);
-		ip->i_di.di_size = (lbn + 1) << sdp->sd_sb.sb_bsize_shift;
-	}
-	if (dbn == ip->i_di.di_num.no_addr)
-		return ip->i_bh;
-	else
-		return bread(sdp, dbn);
-}
-
 int gfs2_dirent_first(struct gfs2_inode *dip, struct gfs2_buffer_head *bh,
 					  struct gfs2_dirent **dent)
 {
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index a34e77a7..aa500dfd 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -432,8 +432,6 @@ extern int gfs2_readi(struct gfs2_inode *ip, void *buf, uint64_t offset,
 	__gfs2_writei(ip, buf, offset, size, 1)
 extern int __gfs2_writei(struct gfs2_inode *ip, void *buf, uint64_t offset,
 			 unsigned int size, int resize);
-extern struct gfs2_buffer_head *get_file_buf(struct gfs2_inode *ip,
-					     uint64_t lbn, int prealloc);
 extern int init_dinode(struct gfs2_sbd *sdp, struct gfs2_buffer_head **bhp, struct gfs2_inum *inum,
                        unsigned int mode, uint32_t flags, struct gfs2_inum *parent);
 extern struct gfs2_inode *createi(struct gfs2_inode *dip, const char *filename,
diff --git a/gfs2/libgfs2/structures.c b/gfs2/libgfs2/structures.c
index 0aca7cd0..1e275bca 100644
--- a/gfs2/libgfs2/structures.c
+++ b/gfs2/libgfs2/structures.c
@@ -194,6 +194,30 @@ int lgfs2_write_journal_data(struct gfs2_inode *ip)
 	return 0;
 }
 
+static struct gfs2_buffer_head *get_file_buf(struct gfs2_inode *ip, uint64_t lbn, int prealloc)
+{
+	struct gfs2_sbd *sdp = ip->i_sbd;
+	uint64_t dbn;
+	int new = TRUE;
+
+	if (ip->i_di.di_height == 0)
+		unstuff_dinode(ip);
+
+	block_map(ip, lbn, &new, &dbn, NULL, prealloc);
+	if (!dbn)
+		return NULL;
+
+	if (!prealloc && new &&
+	    ip->i_di.di_size < (lbn + 1) << sdp->sd_sb.sb_bsize_shift) {
+		bmodified(ip->i_bh);
+		ip->i_di.di_size = (lbn + 1) << sdp->sd_sb.sb_bsize_shift;
+	}
+	if (dbn == ip->i_di.di_num.no_addr)
+		return ip->i_bh;
+	else
+		return bread(sdp, dbn);
+}
+
 int write_journal(struct gfs2_inode *jnl, unsigned bsize, unsigned int blocks)
 {
 	struct gfs2_log_header lh;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 10/32] gfs2l: Remove uses of gfs2_buffer_heads
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (8 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 09/32] libgfs2: Move get_file_buf() into structures.c Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 11/32] libgfs2: No need to use gfs2_buffer_head in metapointer() Andrew Price
                   ` (21 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Required a similar treatment of lgfs2_get_block_type().

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/hexedit.c    |   2 +-
 gfs2/edit/journal.c    |   2 +-
 gfs2/libgfs2/buf.c     |   4 +-
 gfs2/libgfs2/lang.c    | 121 ++++++++++++++++++++++++-----------------
 gfs2/libgfs2/lang.h    |   2 +-
 gfs2/libgfs2/libgfs2.h |   2 +-
 6 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index b31acd8b..bc4bcc6c 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -296,7 +296,7 @@ static void print_usage(void)
 /* ------------------------------------------------------------------------ */
 uint32_t get_block_type(const struct gfs2_buffer_head *lbh, int *structlen)
 {
-	uint32_t ty = lgfs2_get_block_type(lbh);
+	uint32_t ty = lgfs2_get_block_type(lbh->b_data);
 
 	if (ty != 0 && structlen != NULL) {
 		unsigned ver = sbd.gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2;
diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index 9139855b..06a5f618 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -231,7 +231,7 @@ static int print_ld_blks(const uint64_t *b, const char *end, int start_line,
 							  bcount);
 					sprintf(str, "0x%llx %2s",
 						(unsigned long long)be64_to_cpu(*b),
-						mtypes[lgfs2_get_block_type(j_bmap_bh)]);
+						mtypes[lgfs2_get_block_type(j_bmap_bh->b_data)]);
 					brelse(j_bmap_bh);
 				} else {
 					sprintf(str, "0x%llx",
diff --git a/gfs2/libgfs2/buf.c b/gfs2/libgfs2/buf.c
index 097ac33b..eb7a94cf 100644
--- a/gfs2/libgfs2/buf.c
+++ b/gfs2/libgfs2/buf.c
@@ -118,9 +118,9 @@ int brelse(struct gfs2_buffer_head *bh)
 	return error;
 }
 
-uint32_t lgfs2_get_block_type(const struct gfs2_buffer_head *lbh)
+uint32_t lgfs2_get_block_type(const char *buf)
 {
-	const struct gfs2_meta_header *mh = lbh->iov.iov_base;
+	const struct gfs2_meta_header *mh = (void *)buf;
 
 	if (be32_to_cpu(mh->mh_magic) == GFS2_MAGIC)
 		return be32_to_cpu(mh->mh_type);
diff --git a/gfs2/libgfs2/lang.c b/gfs2/libgfs2/lang.c
index 69a98aad..d48b123f 100644
--- a/gfs2/libgfs2/lang.c
+++ b/gfs2/libgfs2/lang.c
@@ -2,6 +2,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <unistd.h>
 #include <sys/queue.h>
 #include <errno.h>
 #include <limits.h>
@@ -286,15 +287,14 @@ static uint64_t ast_lookup_block_num(struct ast_node *ast, struct gfs2_sbd *sbd)
 	return bn;
 }
 
-static struct gfs2_buffer_head *ast_lookup_block(struct ast_node *node, struct gfs2_sbd *sbd)
+static uint64_t ast_lookup_block(struct ast_node *node, struct gfs2_sbd *sbd)
 {
 	uint64_t bn = ast_lookup_block_num(node, sbd);
 	if (bn == 0) {
 		fprintf(stderr, "Block not found: %s\n", node->ast_text);
-		return NULL;
+		return 0;
 	}
-
-	return bread(sbd, bn);
+	return bn;
 }
 
 static const char *bitstate_strings[] = {
@@ -308,12 +308,12 @@ static const char *bitstate_strings[] = {
  * Print a representation of an arbitrary field of an arbitrary GFS2 block to stdout
  * Returns 0 if successful, 1 otherwise
  */
-static int field_print(const struct gfs2_buffer_head *bh, const struct lgfs2_metadata *mtype,
+static int field_print(char *buf, uint64_t addr, const struct lgfs2_metadata *mtype,
                       const struct lgfs2_metafield *field)
 {
-	const char *fieldp = (char *)bh->iov.iov_base + field->offset;
+	const char *fieldp = buf + field->offset;
 
-	printf("%s\t%"PRIu64"\t%u\t%u\t%s\t", mtype->name, bh->b_blocknr, field->offset, field->length, field->name);
+	printf("%s\t%"PRIu64"\t%u\t%u\t%s\t", mtype->name, addr, field->offset, field->length, field->name);
 	if (field->flags & LGFS2_MFF_UUID) {
 #ifdef GFS2_HAS_UUID
 		char readable_uuid[36+1];
@@ -356,7 +356,8 @@ int lgfs2_lang_result_print(struct lgfs2_lang_result *result)
 	int i;
 	if (result->lr_mtype != NULL) {
 		for (i = 0; i < result->lr_mtype->nfields; i++) {
-			field_print(result->lr_bh, result->lr_mtype, &result->lr_mtype->fields[i]);
+			field_print(result->lr_buf, result->lr_blocknr,
+			            result->lr_mtype, &result->lr_mtype->fields[i]);
 		}
 	} else {
 		printf("%"PRIu64": %s\n", result->lr_blocknr, bitstate_strings[result->lr_state]);
@@ -390,21 +391,37 @@ static int ast_get_bitstate(uint64_t bn, struct gfs2_sbd *sbd)
 	return state;
 }
 
-static const struct lgfs2_metadata *ast_lookup_mtype(const struct gfs2_buffer_head *bh)
+static void result_lookup_mtype(struct lgfs2_lang_result *result, int gfs1)
 {
-	const struct lgfs2_metadata *mtype;
-	const uint32_t mh_type = lgfs2_get_block_type(bh);
+	const uint32_t mh_type = lgfs2_get_block_type(result->lr_buf);
+	uint64_t addr = result->lr_blocknr;
+
+	result->lr_mtype = NULL;
 	if (mh_type == 0) {
-		fprintf(stderr, "Could not determine type for block %"PRIu64"\n", bh->b_blocknr);
-		return NULL;
+		fprintf(stderr, "Could not determine type for block %"PRIu64"\n", addr);
+		return;
 	}
+	result->lr_mtype = lgfs2_find_mtype(mh_type, gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2);
+	if (result->lr_mtype == NULL)
+		fprintf(stderr, "Could not determine meta type for block %"PRIu64"\n", addr);
+}
+
+static char *lang_read_block(int fd, unsigned bsize, uint64_t addr)
+{
+	off_t off = addr * bsize;
+	char *buf;
 
-	mtype = lgfs2_find_mtype(mh_type, bh->sdp->gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2);
-	if (mtype == NULL) {
-		fprintf(stderr, "Could not determine meta type for block %"PRIu64"\n", bh->b_blocknr);
+	buf = calloc(1, bsize);
+	if (buf == NULL) {
+		perror("Failed to read block");
 		return NULL;
 	}
-	return mtype;
+	if (pread(fd, buf, bsize, off) != bsize) {
+		fprintf(stderr, "Failed to read block %"PRIu64": %s\n", addr, strerror(errno));
+		free(buf);
+		return NULL;
+	}
+	return buf;
 }
 
 /**
@@ -420,13 +437,17 @@ static struct lgfs2_lang_result *ast_interp_get(struct lgfs2_lang_state *state,
 	}
 
 	if (ast->ast_right->ast_right == NULL) {
-		result->lr_bh = ast_lookup_block(ast->ast_right, sbd);
-		if (result->lr_bh == NULL) {
+		result->lr_blocknr = ast_lookup_block(ast->ast_right, sbd);
+		if (result->lr_blocknr == 0) {
 			free(result);
 			return NULL;
 		}
-		result->lr_blocknr = result->lr_bh->b_blocknr;
-		result->lr_mtype = ast_lookup_mtype(result->lr_bh);
+		result->lr_buf = lang_read_block(sbd->device_fd, sbd->bsize, result->lr_blocknr);
+		if (result->lr_buf == NULL) {
+			free(result);
+			return NULL;
+		}
+		result_lookup_mtype(result, sbd->gfs1);
 
 	} else if (ast->ast_right->ast_right->ast_type == AST_KW_STATE) {
 		result->lr_blocknr = ast_lookup_block_num(ast->ast_right, sbd);
@@ -444,8 +465,8 @@ static struct lgfs2_lang_result *ast_interp_get(struct lgfs2_lang_state *state,
  * Set a field of a gfs2 block of a given type to a given value.
  * Returns AST_INTERP_* to signal success, an invalid field/value or an error.
  */
-static int ast_field_set(struct gfs2_buffer_head *bh, const struct lgfs2_metafield *field,
-                                                                        struct ast_node *val)
+static int ast_field_set(char *buf, const struct lgfs2_metafield *field,
+                         struct ast_node *val)
 {
 	int err = 0;
 
@@ -457,15 +478,15 @@ static int ast_field_set(struct gfs2_buffer_head *bh, const struct lgfs2_metafie
 			fprintf(stderr, "Invalid UUID\n");
 			return AST_INTERP_INVAL;
 		}
-		err = lgfs2_field_assign(bh->b_data, field, uuid);
+		err = lgfs2_field_assign(buf, field, uuid);
 #else
 		fprintf(stderr, "No UUID support\n");
 		err = 1;
 #endif
 	} else if (field->flags & LGFS2_MFF_STRING) {
-		err = lgfs2_field_assign(bh->b_data, field, val->ast_str);
+		err = lgfs2_field_assign(buf, field, val->ast_str);
 	} else {
-		err = lgfs2_field_assign(bh->b_data, field, &val->ast_num);
+		err = lgfs2_field_assign(buf, field, &val->ast_num);
 	}
 
 	if (err) {
@@ -473,12 +494,10 @@ static int ast_field_set(struct gfs2_buffer_head *bh, const struct lgfs2_metafie
 	                field->name, field->length, val->ast_text);
 		return AST_INTERP_INVAL;
 	}
-
-	bmodified(bh);
 	return AST_INTERP_SUCCESS;
 }
 
-static const struct lgfs2_metadata *lang_find_mtype(struct ast_node *node, struct gfs2_buffer_head *bh, unsigned ver)
+static const struct lgfs2_metadata *lang_find_mtype(struct ast_node *node, const char *buf, unsigned ver)
 {
 	const struct lgfs2_metadata *mtype = NULL;
 
@@ -487,7 +506,7 @@ static const struct lgfs2_metadata *lang_find_mtype(struct ast_node *node, struc
 		if (mtype == NULL)
 			fprintf(stderr, "Invalid block type: %s\n", node->ast_text);
 	} else {
-		mtype = lgfs2_find_mtype(lgfs2_get_block_type(bh), ver);
+		mtype = lgfs2_find_mtype(lgfs2_get_block_type(buf), ver);
 		if (mtype == NULL)
 			fprintf(stderr, "Unrecognised block at: %s\n", node->ast_text);
 	}
@@ -495,6 +514,18 @@ static const struct lgfs2_metadata *lang_find_mtype(struct ast_node *node, struc
 	return mtype;
 }
 
+static int lang_write_result(int fd, unsigned bsize, struct lgfs2_lang_result *result)
+{
+	off_t off = bsize * result->lr_blocknr;
+
+	if (pwrite(fd, result->lr_buf, bsize, off) != bsize) {
+		fprintf(stderr, "Failed to write modified block %"PRIu64": %s\n",
+		                result->lr_blocknr, strerror(errno));
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * Interpret an assignment (set)
  */
@@ -514,12 +545,14 @@ static struct lgfs2_lang_result *ast_interp_set(struct lgfs2_lang_state *state,
 		return NULL;
 	}
 
-	result->lr_bh = ast_lookup_block(lookup, sbd);
-	if (result->lr_bh == NULL) {
+	result->lr_blocknr = ast_lookup_block(lookup, sbd);
+	if (result->lr_blocknr == 0)
+		goto out_err;
+	result->lr_buf = lang_read_block(sbd->device_fd, sbd->bsize, result->lr_blocknr);
+	if (result->lr_buf == NULL)
 		goto out_err;
-	}
 
-	result->lr_mtype = lang_find_mtype(lookup->ast_right, result->lr_bh, ver);
+	result->lr_mtype = lang_find_mtype(lookup->ast_right, result->lr_buf, ver);
 	if (result->lr_mtype == NULL) {
 		fprintf(stderr, "Unrecognised block at: %s\n", lookup->ast_str);
 		goto out_err;
@@ -531,7 +564,7 @@ static struct lgfs2_lang_result *ast_interp_set(struct lgfs2_lang_state *state,
 			.mh_type = result->lr_mtype->mh_type,
 			.mh_format = result->lr_mtype->mh_format,
 		};
-		gfs2_meta_header_out(&mh, result->lr_bh->iov.iov_base);
+		gfs2_meta_header_out(&mh, result->lr_buf);
 		lookup = lookup->ast_right;
 	}
 
@@ -550,21 +583,17 @@ static struct lgfs2_lang_result *ast_interp_set(struct lgfs2_lang_state *state,
 			goto out_err;
 		}
 
-		ret = ast_field_set(result->lr_bh, mfield, fieldval);
+		ret = ast_field_set(result->lr_buf, mfield, fieldval);
 		if (ret != AST_INTERP_SUCCESS) {
 			goto out_err;
 		}
 	}
 
-	ret = bwrite(result->lr_bh);
-	if (ret != 0) {
-		fprintf(stderr, "Failed to write modified block %"PRIu64": %s\n",
-		                        result->lr_bh->b_blocknr, strerror(errno));
+	ret = lang_write_result(sbd->device_fd, sbd->bsize, result);
+	if (ret != 0)
 		goto out_err;
-	}
 
 	return result;
-
 out_err:
 	lgfs2_lang_result_free(&result);
 	return NULL;
@@ -606,13 +635,7 @@ void lgfs2_lang_result_free(struct lgfs2_lang_result **result)
 		fprintf(stderr, "Warning: attempted to free a null result\n");
 		return;
 	}
-
-	if ((*result)->lr_mtype != NULL) {
-		(*result)->lr_bh->b_modified = 0;
-		brelse((*result)->lr_bh);
-		(*result)->lr_bh = NULL;
-	}
-
+	free((*result)->lr_buf);
 	free(*result);
 	*result = NULL;
 }
diff --git a/gfs2/libgfs2/lang.h b/gfs2/libgfs2/lang.h
index f74a57bd..211abd46 100644
--- a/gfs2/libgfs2/lang.h
+++ b/gfs2/libgfs2/lang.h
@@ -14,7 +14,7 @@ struct lgfs2_lang_state {
 
 struct lgfs2_lang_result {
 	uint64_t lr_blocknr;
-	struct gfs2_buffer_head *lr_bh;
+	char *lr_buf;
 	const struct lgfs2_metadata *lr_mtype;
 	int lr_state; // GFS2_BLKST_*
 };
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index aa500dfd..7ede1967 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -369,7 +369,7 @@ extern struct gfs2_buffer_head *__bread(struct gfs2_sbd *sdp, uint64_t num,
 extern int __breadm(struct gfs2_sbd *sdp, struct gfs2_buffer_head **bhs, size_t n, uint64_t block, int line, const char *caller);
 extern int bwrite(struct gfs2_buffer_head *bh);
 extern int brelse(struct gfs2_buffer_head *bh);
-extern uint32_t lgfs2_get_block_type(const struct gfs2_buffer_head *lbh);
+extern uint32_t lgfs2_get_block_type(const char *buf);
 
 #define bmodified(bh) do { bh->b_modified = 1; } while(0)
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 11/32] libgfs2: No need to use gfs2_buffer_head in metapointer()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (9 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 10/32] gfs2l: Remove uses of gfs2_buffer_heads Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 12/32] gfs2_edit: Don't use gfs2_buffer_head in do_dinode_extended() args Andrew Price
                   ` (20 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/fs_ops.c | 6 +++---
 gfs2/libgfs2/gfs1.c   | 7 +++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index ee512f67..77648a16 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -15,14 +15,14 @@
 #include "libgfs2.h"
 #include "rgrp.h"
 
-static __inline__ uint64_t *metapointer(struct gfs2_buffer_head *bh,
+static __inline__ uint64_t *metapointer(char *buf,
 					unsigned int height,
 					struct metapath *mp)
 {
 	unsigned int head_size = (height > 0) ?
 		sizeof(struct gfs2_meta_header) : sizeof(struct gfs2_dinode);
 
-	return ((uint64_t *)(bh->b_data + head_size)) + mp->mp_list[height];
+	return ((uint64_t *)(buf + head_size)) + mp->mp_list[height];
 }
 
 /* Detect directory is a stuffed inode */
@@ -447,7 +447,7 @@ void lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 		  unsigned int height, struct metapath *mp,
 		  int create, int *new, uint64_t *block)
 {
-	uint64_t *ptr = metapointer(bh, height, mp);
+	uint64_t *ptr = metapointer(bh->b_data, height, mp);
 
 	if (*ptr) {
 		*block = be64_to_cpu(*ptr);
diff --git a/gfs2/libgfs2/gfs1.c b/gfs2/libgfs2/gfs1.c
index 29e4f12d..da2a0804 100644
--- a/gfs2/libgfs2/gfs1.c
+++ b/gfs2/libgfs2/gfs1.c
@@ -25,13 +25,12 @@ static __inline__ int fs_is_jdata(struct gfs2_inode *ip)
 }
 
 static __inline__ uint64_t *
-gfs1_metapointer(struct gfs2_buffer_head *bh, unsigned int height,
-		 struct metapath *mp)
+gfs1_metapointer(char *buf, unsigned int height, struct metapath *mp)
 {
 	unsigned int head_size = (height > 0) ?
 		sizeof(struct gfs_indirect) : sizeof(struct gfs_dinode);
 
-	return ((uint64_t *)(bh->b_data + head_size)) + mp->mp_list[height];
+	return ((uint64_t *)(buf + head_size)) + mp->mp_list[height];
 }
 
 int is_gfs_dir(struct gfs2_dinode *dinode)
@@ -45,7 +44,7 @@ void gfs1_lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 		  unsigned int height, struct metapath *mp,
 		  int create, int *new, uint64_t *block)
 {
-	uint64_t *ptr = gfs1_metapointer(bh, height, mp);
+	uint64_t *ptr = gfs1_metapointer(bh->b_data, height, mp);
 
 	if (*ptr) {
 		*block = be64_to_cpu(*ptr);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 12/32] gfs2_edit: Don't use gfs2_buffer_head in do_dinode_extended() args
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (10 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 11/32] libgfs2: No need to use gfs2_buffer_head in metapointer() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 13/32] libgfs2: Add a display name field to struct lgfs2_metadata Andrew Price
                   ` (19 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/gfs2hex.c  | 15 +++++++--------
 gfs2/edit/gfs2hex.h  |  3 +--
 gfs2/edit/hexedit.c  |  4 ++--
 gfs2/edit/journal.c  |  2 +-
 gfs2/edit/savemeta.c |  4 ++--
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index 62d5cab5..ea9b6d3a 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -236,7 +236,7 @@ static int indirect_dirent(struct indirect_info *indir, char *ptr, int d)
 	return de.de_rec_len;
 }
 
-void do_dinode_extended(struct gfs2_dinode *dine, struct gfs2_buffer_head *lbh)
+void do_dinode_extended(struct gfs2_dinode *dine, char *buf)
 {
 	unsigned int x, y, ptroff = 0;
 	uint64_t p, last;
@@ -249,7 +249,7 @@ void do_dinode_extended(struct gfs2_dinode *dine, struct gfs2_buffer_head *lbh)
 		/* Indirect pointers */
 		for (x = sizeof(struct gfs2_dinode); x < sbd.bsize;
 			 x += sizeof(uint64_t)) {
-			p = be64_to_cpu(*(uint64_t *)(lbh->b_data + x));
+			p = be64_to_cpu(*(uint64_t *)(buf + x));
 			if (p) {
 				indirect->ii[indirect_blocks].block = p;
 				indirect->ii[indirect_blocks].mp.mp_list[0] =
@@ -270,7 +270,7 @@ void do_dinode_extended(struct gfs2_dinode *dine, struct gfs2_buffer_head *lbh)
 		indirect->ii[0].block = block;
 		indirect->ii[0].is_dir = TRUE;
 		for (x = sizeof(struct gfs2_dinode); x < sbd.bsize; x += skip) {
-			skip = indirect_dirent(indirect->ii, lbh->b_data + x,
+			skip = indirect_dirent(indirect->ii, buf + x,
 					       indirect->ii[0].dirents);
 			if (skip <= 0)
 				break;
@@ -280,14 +280,13 @@ void do_dinode_extended(struct gfs2_dinode *dine, struct gfs2_buffer_head *lbh)
 			 (dine->di_flags & GFS2_DIF_EXHASH) &&
 			 dine->di_height == 0) {
 		/* Leaf Pointers: */
-		
-		last = be64_to_cpu(*(uint64_t *)(lbh->b_data +
-						 sizeof(struct gfs2_dinode)));
-    
+
+		last = be64_to_cpu(*(uint64_t *)(buf + sizeof(struct gfs2_dinode)));
+
 		for (x = sizeof(struct gfs2_dinode), y = 0;
 			 y < (1 << dine->di_depth);
 			 x += sizeof(uint64_t), y++) {
-			p = be64_to_cpu(*(uint64_t *)(lbh->b_data + x));
+			p = be64_to_cpu(*(uint64_t *)(buf + x));
 
 			if (p != last || ((y + 1) * sizeof(uint64_t) == dine->di_size)) {
 				struct gfs2_buffer_head *tmp_bh;
diff --git a/gfs2/edit/gfs2hex.h b/gfs2/edit/gfs2hex.h
index c3efb27e..6c6ddde3 100644
--- a/gfs2/edit/gfs2hex.h
+++ b/gfs2/edit/gfs2hex.h
@@ -5,8 +5,7 @@
 
 extern int display_gfs2(struct gfs2_buffer_head *dbh);
 extern int edit_gfs2(void);
-extern void do_dinode_extended(struct gfs2_dinode *di,
-			       struct gfs2_buffer_head *lbh);
+extern void do_dinode_extended(struct gfs2_dinode *di, char *buf);
 extern void print_gfs2(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern uint64_t do_leaf_extended(char *dlebuf, struct iinfo *indir);
 extern void eol(int col);
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index bc4bcc6c..8a8e0225 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1057,7 +1057,7 @@ static int read_master_dir(void)
 	if (bh == NULL)
 		return 1;
 	gfs2_dinode_in(&di, bh->b_data);
-	do_dinode_extended(&di, bh); /* get extended data, if any */
+	do_dinode_extended(&di, bh->b_data); /* get extended data, if any */
 	memcpy(&masterdir, &indirect[0], sizeof(struct indirect_info));
 	return 0;
 }
@@ -1128,7 +1128,7 @@ int display(int identify_only, int trunc_zeros, uint64_t flagref,
 	}
 	else if (gfs2_struct_type == GFS2_METATYPE_DI) {
 		gfs2_dinode_in(&di, bh->b_data);
-		do_dinode_extended(&di, bh); /* get extended data, if any */
+		do_dinode_extended(&di, bh->b_data); /* get extended data, if any */
 	}
 	else if (gfs2_struct_type == GFS2_METATYPE_IN) { /* indirect block list */
 		if (blockhist) {
diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index 06a5f618..aa945920 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -57,7 +57,7 @@ uint64_t find_journal_block(const char *journal, uint64_t *j_size)
 	gfs2_dinode_in(&di, jindex_bh->b_data);
 
 	if (!sbd.gfs1)
-		do_dinode_extended(&di, jindex_bh); /* parse dir. */
+		do_dinode_extended(&di, jindex_bh->b_data); /* parse dir. */
 
 	if (sbd.gfs1) {
 		struct gfs2_inode *jiinode;
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index cbecf281..c42f2da0 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -274,7 +274,7 @@ static int init_per_node_lookup(void)
 		return 1;
 	}
 
-	do_dinode_extended(&per_node_di->i_di, per_node_di->i_bh);
+	do_dinode_extended(&per_node_di->i_di, per_node_di->i_bh->b_data);
 	inode_put(&per_node_di);
 
 	for (i = 0; i < indirect_blocks; i++) {
@@ -993,7 +993,7 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 	lbh = bread(&sbd, jindex_block);
 	gfs2_dinode_in(&di, lbh->b_data);
 	if (!sbd.gfs1)
-		do_dinode_extended(&di, lbh);
+		do_dinode_extended(&di, lbh->b_data);
 	brelse(lbh);
 
 	printf("Filesystem size: %.2fGB\n", (sbd.fssize * sbd.bsize) / ((float)(1 << 30)));
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 13/32] libgfs2: Add a display name field to struct lgfs2_metadata
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (11 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 12/32] gfs2_edit: Don't use gfs2_buffer_head in do_dinode_extended() args Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 14/32] gfs2_edit: get_block_type() improvements Andrew Price
                   ` (18 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

.name is used for the name of the struct and we have places that need to
tag blocks with a more descriptive name for users to read, so add a
display name for each block type.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h |  3 ++-
 gfs2/libgfs2/meta.c    | 30 +++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 7ede1967..5350f3d9 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -146,7 +146,8 @@ struct lgfs2_metadata {
 	const unsigned header:1;
 	const uint32_t mh_type;
 	const uint32_t mh_format;
-	const char *name;
+	const char *name; /* Struct name */
+	const char *display; /* Short name for non-programmers */
 	const struct lgfs2_metafield *fields;
 	const unsigned nfields;
 	const unsigned size;
diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index 1da74146..5ac808c2 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -560,6 +560,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_SB,
 		.mh_format = GFS2_FORMAT_SB,
 		.name = "gfs2_sb",
+		.display = "superblock",
 		.fields = gfs2_sb_fields,
 		.nfields = ARRAY_SIZE(gfs2_sb_fields),
 		.size = sizeof(struct gfs2_sb),
@@ -570,13 +571,15 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_SB,
 		.mh_format = GFS_FORMAT_SB,
 		.name = "gfs_sb",
+		.display = "superblock",
 		.fields = gfs_sb_fields,
 		.nfields = ARRAY_SIZE(gfs_sb_fields),
 		.size = sizeof(struct gfs_sb),
 	},
 	[LGFS2_MT_RINDEX] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
-		.name = "rindex",
+		.name = "gfs2_rindex",
+		.display = "resource group index",
 		.fields = gfs2_rindex_fields,
 		.nfields = ARRAY_SIZE(gfs2_rindex_fields),
 		.size = sizeof(struct gfs2_rindex),
@@ -587,6 +590,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_RG,
 		.mh_format = GFS2_FORMAT_RG,
 		.name = "gfs2_rgrp",
+		.display = "resource group",
 		.fields = gfs2_rgrp_fields,
 		.nfields = ARRAY_SIZE(gfs2_rgrp_fields),
 		.size = sizeof(struct gfs2_rgrp),
@@ -597,6 +601,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_RG,
 		.mh_format = GFS2_FORMAT_RG,
 		.name = "gfs_rgrp",
+		.display = "resource group",
 		.fields = gfs_rgrp_fields,
 		.nfields = ARRAY_SIZE(gfs_rgrp_fields),
 		.size = sizeof(struct gfs_rgrp),
@@ -607,6 +612,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_RB,
 		.mh_format = GFS2_FORMAT_RB,
 		.name = "gfs2_rgrp_bitmap",
+		.display = "allocation bitmap",
 		.fields = gfs2_rgrp_bitmap_fields,
 		.nfields = ARRAY_SIZE(gfs2_rgrp_bitmap_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -617,6 +623,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_DI,
 		.mh_format = GFS2_FORMAT_DI,
 		.name = "gfs2_dinode",
+		.display = "inode",
 		.fields = gfs2_dinode_fields,
 		.nfields = ARRAY_SIZE(gfs2_dinode_fields),
 		.size = sizeof(struct gfs2_dinode),
@@ -627,6 +634,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_DI,
 		.mh_format = GFS2_FORMAT_DI,
 		.name = "gfs_dinode",
+		.display = "inode",
 		.fields = gfs_dinode_fields,
 		.nfields = ARRAY_SIZE(gfs_dinode_fields),
 		.size = sizeof(struct gfs_dinode),
@@ -637,6 +645,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_IN,
 		.mh_format = GFS2_FORMAT_IN,
 		.name = "gfs2_indirect",
+		.display = "indirect",
 		.fields = gfs2_indirect_fields,
 		.nfields = ARRAY_SIZE(gfs2_indirect_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -647,6 +656,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_IN,
 		.mh_format = GFS2_FORMAT_IN,
 		.name = "gfs_indirect",
+		.display = "indirect",
 		.fields = gfs_indirect_fields,
 		.nfields = ARRAY_SIZE(gfs_indirect_fields),
 		.size = sizeof(struct gfs_indirect),
@@ -657,6 +667,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LF,
 		.mh_format = GFS2_FORMAT_LF,
 		.name = "gfs2_leaf",
+		.display = "leaf",
 		.fields = gfs2_leaf_fields,
 		.nfields = ARRAY_SIZE(gfs2_leaf_fields),
 		.size = sizeof(struct gfs2_leaf),
@@ -667,6 +678,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_JD,
 		.mh_format = GFS2_FORMAT_JD,
 		.name = "gfs2_jdata",
+		.display = "journal data",
 		.fields = gfs2_jdata_fields,
 		.nfields = ARRAY_SIZE(gfs2_jdata_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -677,6 +689,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LH,
 		.mh_format = GFS2_FORMAT_LH,
 		.name = "gfs2_log_header",
+		.display = "log header",
 		.fields = gfs2_log_header_fields,
 		.nfields = ARRAY_SIZE(gfs2_log_header_fields),
 		.size = sizeof(struct gfs2_log_header),
@@ -687,6 +700,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LH,
 		.mh_format = GFS2_FORMAT_LH,
 		.name = "gfs_log_header",
+		.display = "log header",
 		.fields = gfs_log_header_fields,
 		.nfields = ARRAY_SIZE(gfs_log_header_fields),
 		.size = sizeof(struct gfs_log_header),
@@ -697,6 +711,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LD,
 		.mh_format = GFS2_FORMAT_LD,
 		.name = "gfs2_log_desc",
+		.display = "log descriptor",
 		.fields = gfs2_log_desc_fields,
 		.nfields = ARRAY_SIZE(gfs2_log_desc_fields),
 		.size = sizeof(struct gfs2_log_descriptor),
@@ -707,6 +722,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LD,
 		.mh_format = GFS2_FORMAT_LD,
 		.name = "gfs_log_desc",
+		.display = "log descriptor",
 		.fields = gfs_log_desc_fields,
 		.nfields = ARRAY_SIZE(gfs_log_desc_fields),
 		.size = sizeof(struct gfs_log_descriptor),
@@ -717,6 +733,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_LB,
 		.mh_format = GFS2_FORMAT_LB,
 		.name = "gfs2_log_block",
+		.display = "log",
 		.fields = gfs2_log_block_fields,
 		.nfields = ARRAY_SIZE(gfs2_log_block_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -727,6 +744,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_EA,
 		.mh_format = GFS2_FORMAT_EA,
 		.name = "gfs2_ea_attr",
+		.display = "extended attribute",
 		.fields = gfs2_ea_attr_fields,
 		.nfields = ARRAY_SIZE(gfs2_ea_attr_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -737,6 +755,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 		.mh_type = GFS2_METATYPE_ED,
 		.mh_format = GFS2_FORMAT_ED,
 		.name = "gfs2_ea_data",
+		.display = "extended attribute data",
 		.fields = gfs2_ea_data_fields,
 		.nfields = ARRAY_SIZE(gfs2_ea_data_fields),
 		.size = sizeof(struct gfs2_meta_header),
@@ -744,6 +763,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_GFS2_QUOTA_CHANGE] = {
 		.versions = LGFS2_MD_GFS2,
 		.name = "gfs2_quota_change",
+		.display = "quota change",
 		.fields = gfs2_quota_change_fields,
 		.nfields = ARRAY_SIZE(gfs2_quota_change_fields),
 		.size = sizeof(struct gfs2_quota_change),
@@ -751,6 +771,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_DIRENT] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
 		.name = "gfs2_dirent",
+		.display = "directory entry",
 		.fields = gfs2_dirent_fields,
 		.nfields = ARRAY_SIZE(gfs2_dirent_fields),
 		.size = sizeof(struct gfs2_dirent),
@@ -758,6 +779,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_EA_HEADER] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
 		.name = "gfs2_ea_header",
+		.display = "extended attribute header",
 		.fields = gfs2_ea_header_fields,
 		.nfields = ARRAY_SIZE(gfs2_ea_header_fields),
 		.size = sizeof(struct gfs2_ea_header),
@@ -765,6 +787,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_GFS2_INUM_RANGE] = {
 		.versions = LGFS2_MD_GFS2,
 		.name = "gfs2_inum_range",
+		.display = "inum range",
 		.fields = gfs2_inum_range_fields,
 		.nfields = ARRAY_SIZE(gfs2_inum_range_fields),
 		.size = sizeof(struct gfs2_inum_range),
@@ -772,6 +795,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_STATFS_CHANGE] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
 		.name = "gfs2_statfs_change",
+		.display = "statfs change",
 		.fields = gfs2_statfs_change_fields,
 		.nfields = ARRAY_SIZE(gfs2_statfs_change_fields),
 		.size = sizeof(struct gfs2_statfs_change),
@@ -779,6 +803,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_GFS_JINDEX] = {
 		.versions = LGFS2_MD_GFS1,
 		.name = "gfs_jindex",
+		.display = "journal index",
 		.fields = gfs_jindex_fields,
 		.nfields = ARRAY_SIZE(gfs_jindex_fields),
 		.size = sizeof(struct gfs_jindex),
@@ -786,6 +811,7 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_GFS_BLOCK_TAG] = {
 		.versions = LGFS2_MD_GFS1,
 		.name = "gfs_block_tag",
+		.display = "block tag",
 		.fields = gfs_block_tag_fields,
 		.nfields = ARRAY_SIZE(gfs_block_tag_fields),
 		.size = sizeof(struct gfs_block_tag),
@@ -793,10 +819,12 @@ const struct lgfs2_metadata lgfs2_metadata[] = {
 	[LGFS2_MT_DATA] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
 		.name = "data",
+		.display = "data",
 	},
 	[LGFS2_MT_FREE] = {
 		.versions = LGFS2_MD_GFS1 | LGFS2_MD_GFS2,
 		.name = "free",
+		.display = "free",
 	},
 };
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 14/32] gfs2_edit: get_block_type() improvements
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (12 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 13/32] libgfs2: Add a display name field to struct lgfs2_metadata Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 15/32] gfs2_edit: Don't use gfs2_buffer_head in display_block_type() Andrew Price
                   ` (17 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Use the metadata description better to clean up the interface to
get_block_type() and remove the use of gfs2_buffer_head there. Since we
already have the structure length in mtype->size there's no need to set
it separately, just return the metadata type structure instead of the
mh_type. Use the new ->display strings to simplify display_block_type().

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/gfs2hex.c |   2 +-
 gfs2/edit/hexedit.c | 147 ++++++++++++--------------------------------
 gfs2/edit/hexedit.h |   6 +-
 gfs2/edit/journal.c |  55 +++++++++--------
 4 files changed, 72 insertions(+), 138 deletions(-)

diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index ea9b6d3a..7274303c 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -526,7 +526,7 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 
 		case GFS2_METATYPE_LH:
 			if (sbd.gfs1) {
-				gfs_log_header_in(&lh1, dbh);
+				gfs_log_header_in(&lh1, dbh->b_data);
 				gfs_log_header_print(&lh1);
 			} else {
 				gfs2_log_header_in(&lh, dbh->b_data);
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 8a8e0225..357e790a 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -289,24 +289,13 @@ static void print_usage(void)
 	Erase();
 }
 
-/* ------------------------------------------------------------------------ */
-/* get_block_type                                                           */
-/* returns: metatype if block is a GFS2 structure block type                */
-/*          0 if block is not a GFS2 structure                              */
-/* ------------------------------------------------------------------------ */
-uint32_t get_block_type(const struct gfs2_buffer_head *lbh, int *structlen)
+const struct lgfs2_metadata *get_block_type(char *buf)
 {
-	uint32_t ty = lgfs2_get_block_type(lbh->b_data);
+	uint32_t t = lgfs2_get_block_type(buf);
 
-	if (ty != 0 && structlen != NULL) {
-		unsigned ver = sbd.gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2;
-		const struct lgfs2_metadata *mtype = lgfs2_find_mtype(ty, ver);
-		if (mtype != NULL)
-			*structlen = mtype->size;
-		else
-			*structlen = sbd.bsize;
-	}
-	return ty;
+	if (t != 0)
+		return lgfs2_find_mtype(t, sbd.gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2);
+	return NULL;
 }
 
 /* ------------------------------------------------------------------------ */
@@ -316,6 +305,7 @@ uint32_t get_block_type(const struct gfs2_buffer_head *lbh, int *structlen)
 /* ------------------------------------------------------------------------ */
 int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 {
+	const struct lgfs2_metadata *mtype;
 	const struct gfs2_meta_header *mh;
 	int ret_type = 0; /* return type */
 
@@ -350,53 +340,14 @@ int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 		ret_type = GFS2_METATYPE_DI;
 		struct_len = 0;
 	} else {
-		ret_type = get_block_type(dbh, &struct_len);
-		switch (ret_type) {
-		case GFS2_METATYPE_SB:   /* 1 */
-			print_gfs2("(superblock)");
-			break;
-		case GFS2_METATYPE_RG:   /* 2 */
-			print_gfs2("(rsrc grp hdr)");
-			break;
-		case GFS2_METATYPE_RB:   /* 3 */
-			print_gfs2("(rsrc grp bitblk)");
-			break;
-		case GFS2_METATYPE_DI:   /* 4 */
-			print_gfs2("(disk inode)");
-			break;
-		case GFS2_METATYPE_IN:   /* 5 */
-			print_gfs2("(indir blklist)");
-			break;
-		case GFS2_METATYPE_LF:   /* 6 */
-			print_gfs2("(directory leaf)");
-			break;
-		case GFS2_METATYPE_JD:
-			print_gfs2("(journal data)");
-			break;
-		case GFS2_METATYPE_LH:
-			print_gfs2("(log header)");
-			break;
-		case GFS2_METATYPE_LD:
-		 	print_gfs2("(log descriptor)");
-			break;
-		case GFS2_METATYPE_EA:
-			print_gfs2("(extended attr hdr)");
-			break;
-		case GFS2_METATYPE_ED:
-			print_gfs2("(extended attr data)");
-			break;
-		case GFS2_METATYPE_LB:
-			print_gfs2("(log buffer)");
-			break;
-		case GFS2_METATYPE_QC:
-			print_gfs2("(quota change)");
-			break;
-		case 0:
+		mtype = get_block_type(dbh->b_data);
+		if (mtype != NULL) {
+			print_gfs2("(%s)", mtype->display);
+			struct_len = mtype->size;
+			ret_type = mtype->mh_type;
+		} else {
 			struct_len = sbd.bsize;
-			break;
-		default:
-			print_gfs2("(wtf?)");
-			break;
+			ret_type = 0;
 		}
 	}
 	mh = dbh->iov.iov_base;
@@ -531,20 +482,6 @@ int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 	return ret_type;
 }
 
-static const struct lgfs2_metadata *find_mtype(uint32_t mtype, const unsigned versions)
-{
-	const struct lgfs2_metadata *m = lgfs2_metadata;
-	unsigned n = 0;
-
-	do {
-		if ((m[n].versions & versions) && m[n].mh_type == mtype)
-			return &m[n];
-		n++;
-	} while (n < lgfs2_metadata_size);
-
-	return NULL;
-}
-
 static int get_pnum(int ptroffset)
 {
 	int pnum;
@@ -568,7 +505,7 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 	const char *lpBuffer = bh->b_data;
 	const char *zeros_strt = lpBuffer + sbd.bsize;
 	int print_field, cursor_line;
-	const uint32_t block_type = get_block_type(bh, NULL);
+	const struct lgfs2_metadata *m = get_block_type(bh->b_data);
 	uint64_t *ref;
 	int ptroffset = 0;
 
@@ -673,8 +610,6 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 		}
 		print_gfs2("] ");
 		if (print_field >= 0) {
-			const struct lgfs2_metadata *m = find_mtype(block_type,
-			               sbd.gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2);
 			if (m) {
 				const struct lgfs2_metafield *f;
 				unsigned n;
@@ -689,7 +624,9 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 			}
 
 		}
-		if (cursor_line) {
+		if (m && cursor_line) {
+			const uint32_t block_type = m->mh_type;
+
 			if (block_type == GFS2_METATYPE_IN ||
 			    block_type == GFS2_METATYPE_LD ||
 			    ((block_type == GFS2_METATYPE_DI) &&
@@ -723,7 +660,7 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 		if ((const char *)pointer >= zeros_strt)
 			break;
 	} /* while */
-	if (block_type == GFS2_METATYPE_LD && ptroffset >= struct_len) {
+	if (m && m->mh_type == GFS2_METATYPE_LD && ptroffset >= struct_len) {
 		COLORS_NORMAL;
 		eol(0);
 		print_gfs2("         * 'j' will jump to the journaled block, "
@@ -1637,8 +1574,12 @@ static void jump(void)
 	if (dmode == HEX_MODE) {
 		unsigned int col2;
 		uint64_t *b;
-		const uint32_t block_type = get_block_type(bh, NULL);
-		
+		const struct lgfs2_metadata *mtype = get_block_type(bh->b_data);
+		uint32_t block_type = 0;
+
+		if (mtype != NULL)
+			block_type = mtype->mh_type;
+
 		/* special exception for log descriptors: jump the journaled
 		   version of the block, not the "real" block */
 		if (block_type == GFS2_METATYPE_LD) {
@@ -1667,33 +1608,26 @@ static void jump(void)
 	}
 }
 
-/* ------------------------------------------------------------------------ */
-/* print block type                                                         */
-/* ------------------------------------------------------------------------ */
-static void print_block_type(uint64_t tblock, int type, const char *additional)
+static void print_block_type(uint64_t tblock, const struct lgfs2_metadata *type)
 {
-	if (type <= GFS2_METATYPE_QC)
-		printf("%d (Block %lld is type %d: %s%s)\n", type,
-		       (unsigned long long)tblock, type, block_type_str[type],
-		       additional);
+	if (type != NULL && type->nfields > 0)
+		printf("%d (Block %"PRIu64" is type %d: %s)\n", type->mh_type,
+		       tblock, type->mh_type, type->display);
 	else
-		printf("%d (Block %lld is type %d: unknown%s)\n", type,
-		       (unsigned long long)tblock, type, additional);
+		printf("%d (Block %"PRIu64" is type %d: unknown)\n", type->mh_type,
+		       tblock, type->mh_type);
 }
 
-/* ------------------------------------------------------------------------ */
-/* find_print block type                                                    */
-/* ------------------------------------------------------------------------ */
 static void find_print_block_type(void)
 {
 	uint64_t tblock;
 	struct gfs2_buffer_head *lbh;
-	int type;
+	const struct lgfs2_metadata *type;
 
 	tblock = blockstack[blockhist % BLOCK_STACK_SIZE].block;
 	lbh = bread(&sbd, tblock);
-	type = get_block_type(lbh, NULL);
-	print_block_type(tblock, type, "");
+	type = get_block_type(lbh->b_data);
+	print_block_type(tblock, type);
 	brelse(lbh);
 	gfs2_rgrp_free(&sbd, &sbd.rgtree);
 	exit(0);
@@ -1803,17 +1737,15 @@ static void process_field(const char *field, const char *nstr)
 {
 	uint64_t fblock;
 	struct gfs2_buffer_head *rbh;
-	int type;
 	const struct lgfs2_metadata *mtype;
 	const struct lgfs2_metafield *mfield;
 
 	fblock = blockstack[blockhist % BLOCK_STACK_SIZE].block;
 	rbh = bread(&sbd, fblock);
-	type = get_block_type(rbh, NULL);
-
-	mtype = lgfs2_find_mtype(type, sbd.gfs1 ? LGFS2_MD_GFS1 : LGFS2_MD_GFS2);
+	mtype = get_block_type(rbh->b_data);
 	if (mtype == NULL) {
-		fprintf(stderr, "Metadata type '%d' invalid\n", type);
+		fprintf(stderr, "Metadata type of block %"PRIx64" not recognised\n",
+		        fblock);
 		exit(1);
 	}
 
@@ -2116,12 +2048,11 @@ static void interactive_mode(void)
 /* ------------------------------------------------------------------------ */
 /* gfs_log_header_in - read in a gfs1-style log header                      */
 /* ------------------------------------------------------------------------ */
-void gfs_log_header_in(struct gfs_log_header *head,
-		       struct gfs2_buffer_head *lbh)
+void gfs_log_header_in(struct gfs_log_header *head, const char *buf)
 {
-	struct gfs_log_header *str = lbh->iov.iov_base;
+	const struct gfs_log_header *str = (void *)buf;
 
-	gfs2_meta_header_in(&head->lh_header, lbh->b_data);
+	gfs2_meta_header_in(&head->lh_header, buf);
 
 	head->lh_flags = be32_to_cpu(str->lh_flags);
 	head->lh_pad = be32_to_cpu(str->lh_pad);
diff --git a/gfs2/edit/hexedit.h b/gfs2/edit/hexedit.h
index ab2d44d6..a73c150d 100644
--- a/gfs2/edit/hexedit.h
+++ b/gfs2/edit/hexedit.h
@@ -224,8 +224,7 @@ extern int block_is_quota_file(uint64_t blk);
 extern int block_is_per_node(uint64_t blk);
 extern int display_block_type(struct gfs2_buffer_head *bh, int from_restore);
 extern void gfs_jindex_in(struct gfs_jindex *jindex, char *buf);
-extern void gfs_log_header_in(struct gfs_log_header *head,
-			      struct gfs2_buffer_head *bh);
+extern void gfs_log_header_in(struct gfs_log_header *head, const char *buf);
 extern void gfs_log_header_print(struct gfs_log_header *lh);
 extern void savemeta(char *out_fn, int saveoption, int gziplevel);
 extern void restoremeta(const char *in_fn, const char *out_device,
@@ -236,7 +235,6 @@ extern uint64_t check_keywords(const char *kword);
 extern uint64_t masterblock(const char *fn);
 extern void gfs_rgrp_print(struct gfs_rgrp *rg);
 extern int has_indirect_blocks(void);
-extern uint32_t get_block_type(const struct gfs2_buffer_head *lbh,
-			       int *structlen);
+extern const struct lgfs2_metadata *get_block_type(char *buf);
 
 #endif /* __HEXVIEW_DOT_H__ */
diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index aa945920..1adbc4b1 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -306,14 +306,14 @@ static int print_ld_blks(const uint64_t *b, const char *end, int start_line,
 
 static int is_wrap_pt(char *buf, uint64_t *highest_seq)
 {
-	struct gfs2_buffer_head tbh = { .b_data = buf };
+	const struct lgfs2_metadata *mtype = get_block_type(buf);
 
-	if (get_block_type(&tbh, NULL) == GFS2_METATYPE_LH) {
+	if (mtype != NULL && mtype->mh_type == GFS2_METATYPE_LH) {
 		uint64_t seq;
 
 		if (sbd.gfs1) {
 			struct gfs_log_header lh;
-			gfs_log_header_in(&lh, &tbh);
+			gfs_log_header_in(&lh, buf);
 			seq = lh.lh_sequence;
 		} else {
 			struct gfs2_log_header lh;
@@ -421,20 +421,24 @@ static int process_ld(uint64_t abs_block, uint64_t wrappt, uint64_t j_size,
  */
 static int meta_has_ref(uint64_t abs_block, int tblk)
 {
+	const struct lgfs2_metadata *mtype;
 	struct gfs2_buffer_head *mbh;
-	int structlen, ty, has_ref = 0;
+	int structlen = 0, has_ref = 0;
 	uint64_t *b;
 	struct gfs2_dinode *dinode;
 
 	mbh = bread(&sbd, abs_block);
-	ty = get_block_type(mbh, &structlen);
-	if (ty == GFS2_METATYPE_DI) {
-		dinode = (struct gfs2_dinode *)mbh->b_data;
-		if (be64_to_cpu(dinode->di_eattr) == tblk)
-			has_ref = 1;
+	mtype = get_block_type(mbh->b_data);
+	if (mtype != NULL) {
+		structlen = mtype->size;
+		if (mtype->mh_type == GFS2_METATYPE_DI) {
+			dinode = (struct gfs2_dinode *)mbh->b_data;
+			if (be64_to_cpu(dinode->di_eattr) == tblk)
+				has_ref = 1;
+		}
 	}
 	b = (uint64_t *)(mbh->b_data + structlen);
-	while (!has_ref && ty && (char *)b < mbh->b_data + sbd.bsize) {
+	while (!has_ref && mtype && (char *)b < mbh->b_data + sbd.bsize) {
 		if (be64_to_cpu(*b) == tblk)
 			has_ref = 1;
 		b++;
@@ -478,7 +482,7 @@ void dump_journal(const char *journal, int tblk)
 {
 	const struct lgfs2_metadata *mtype;
 	const struct lgfs2_metafield *lh_flags_field;
-	struct gfs2_buffer_head *j_bh = NULL, dummy_bh;
+	struct gfs2_buffer_head *j_bh = NULL;
 	uint64_t jblock, j_size, jb, abs_block, saveblk, wrappt = 0;
 	int start_line, journal_num;
 	struct gfs2_inode *j_inode = NULL;
@@ -486,6 +490,7 @@ void dump_journal(const char *journal, int tblk)
 	uint64_t tblk_off = 0, bblk_off = 0, bitblk = 0;
 	uint64_t highest_seq = 0;
 	char *jbuf = NULL;
+	char *buf = NULL;
 	struct rgrp_tree *rgd = NULL;
 	uint64_t abs_ld = 0;
 
@@ -564,20 +569,23 @@ void dump_journal(const char *journal, int tblk)
 				brelse(j_bh);
 			abs_block = jblock + ((jb + wrappt) % j_size);
 			j_bh = bread(&sbd, abs_block);
-			dummy_bh.b_data = j_bh->b_data;
+			buf = j_bh->b_data;
 		} else {
 			int error = fsck_readi(j_inode, (void *)jbuf,
 					   ((jb + wrappt) % j_size),
 					   sbd.bsize, &abs_block);
 			if (!error) /* end of file */
 				break;
-			dummy_bh.b_data = jbuf;
+			buf = jbuf;
 		}
 		offset_from_ld++;
-		block_type = get_block_type(&dummy_bh, NULL);
+		mtype = get_block_type(buf);
+		if (mtype != NULL)
+			block_type = mtype->mh_type;
+
 		if (block_type == GFS2_METATYPE_LD) {
 			ld_blocks = process_ld(abs_block, wrappt, j_size, jb,
-					       dummy_bh.b_data, tblk, &tblk_off,
+					       buf, tblk, &tblk_off,
 					       bitblk, rgd, &is_pertinent,
 					       &bblk_off);
 			offset_from_ld = 0;
@@ -587,7 +595,7 @@ void dump_journal(const char *journal, int tblk)
 			struct gfs_log_header lh1;
 
 			if (sbd.gfs1) {
-				gfs_log_header_in(&lh1, &dummy_bh);
+				gfs_log_header_in(&lh1, buf);
 				check_journal_wrap(lh1.lh_sequence,
 						   &highest_seq);
 				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: "
@@ -600,11 +608,11 @@ void dump_journal(const char *journal, int tblk)
 			} else {
 				char flags_str[256];
 
-				gfs2_log_header_in(&lh, dummy_bh.b_data);
+				gfs2_log_header_in(&lh, buf);
 				check_journal_wrap(lh.lh_sequence,
 						   &highest_seq);
 				lgfs2_field_str(flags_str, sizeof(flags_str),
-						dummy_bh.b_data, lh_flags_field,
+						buf, lh_flags_field,
 						(dmode == HEX_MODE));
 				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: Seq"
 					   ": 0x%llx, tail: 0x%x, blk: 0x%x [%s]",
@@ -616,18 +624,15 @@ void dump_journal(const char *journal, int tblk)
 			eol(0);
 		} else if ((ld_blocks > 0) &&
 			   (sbd.gfs1 || block_type == GFS2_METATYPE_LB)) {
+			uint64_t *b = (uint64_t *)(buf + (sbd.gfs1 ? 0 : sizeof(struct gfs2_meta_header)));
+
 			print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log descriptor"
 				   " continuation block", abs_block,
 				   ((jb + wrappt) % j_size) / sbd.bsize);
 			eol(0);
 			print_gfs2("                    ");
-			ld_blocks -= print_ld_blks((uint64_t *)dummy_bh.b_data +
-						   (sbd.gfs1 ? 0 :
-						    sizeof(struct gfs2_meta_header)),
-						   (dummy_bh.b_data +
-						    sbd.bsize), start_line,
-						   tblk, &tblk_off, 0, rgd,
-						   0, 1, NULL, 0);
+			ld_blocks -= print_ld_blks(b, (buf + sbd.bsize), start_line,
+			                     tblk, &tblk_off, 0, rgd, 0, 1, NULL, 0);
 		} else if (block_type == 0) {
 			continue;
 		}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 15/32] gfs2_edit: Don't use gfs2_buffer_head in display_block_type()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (13 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 14/32] gfs2_edit: get_block_type() improvements Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 16/32] gfs2_edit: Don't use gfs2_buffer_head in display_gfs2() Andrew Price
                   ` (16 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/hexedit.c  | 19 +++++++++----------
 gfs2/edit/hexedit.h  |  2 +-
 gfs2/edit/savemeta.c |  4 ++--
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 357e790a..6019996d 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -298,12 +298,11 @@ const struct lgfs2_metadata *get_block_type(char *buf)
 	return NULL;
 }
 
-/* ------------------------------------------------------------------------ */
-/* display_block_type                                                       */
-/* returns: metatype if block is a GFS2 structure block type                */
-/*          0 if block is not a GFS2 structure                              */
-/* ------------------------------------------------------------------------ */
-int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
+/**
+ * returns: metatype if block is a GFS2 structure block type
+ *          0 if block is not a GFS2 structure
+ */
+int display_block_type(char *buf, uint64_t addr, int from_restore)
 {
 	const struct lgfs2_metadata *mtype;
 	const struct gfs2_meta_header *mh;
@@ -324,7 +323,7 @@ int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 	else if (block == JOURNALS_DUMMY_BLOCK)
 		print_gfs2("Journal Status:      ");
 	else
-		print_gfs2("%"PRIu64"    (0x%"PRIx64")", dbh->b_blocknr, dbh->b_blocknr);
+		print_gfs2("%"PRIu64"    (0x%"PRIx64")", addr, addr);
 	if (termlines) {
 		if (edit_row[dmode] == -1)
 			COLORS_NORMAL;
@@ -340,7 +339,7 @@ int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 		ret_type = GFS2_METATYPE_DI;
 		struct_len = 0;
 	} else {
-		mtype = get_block_type(dbh->b_data);
+		mtype = get_block_type(buf);
 		if (mtype != NULL) {
 			print_gfs2("(%s)", mtype->display);
 			struct_len = mtype->size;
@@ -350,7 +349,7 @@ int display_block_type(struct gfs2_buffer_head *dbh, int from_restore)
 			ret_type = 0;
 		}
 	}
-	mh = dbh->iov.iov_base;
+	mh = (void *)buf;
 	eol(0);
 	if (from_restore)
 		return ret_type;
@@ -1035,7 +1034,7 @@ int display(int identify_only, int trunc_zeros, uint64_t flagref,
 		}
 	}
 	line = 1;
-	gfs2_struct_type = display_block_type(bh, FALSE);
+	gfs2_struct_type = display_block_type(bh->b_data, bh->b_blocknr, FALSE);
 	if (identify_only)
 		return 0;
 	indirect_blocks = 0;
diff --git a/gfs2/edit/hexedit.h b/gfs2/edit/hexedit.h
index a73c150d..1c94c901 100644
--- a/gfs2/edit/hexedit.h
+++ b/gfs2/edit/hexedit.h
@@ -222,7 +222,7 @@ extern int block_is_inum_file(uint64_t blk);
 extern int block_is_statfs_file(uint64_t blk);
 extern int block_is_quota_file(uint64_t blk);
 extern int block_is_per_node(uint64_t blk);
-extern int display_block_type(struct gfs2_buffer_head *bh, int from_restore);
+extern int display_block_type(char *buf, uint64_t addr, int from_restore);
 extern void gfs_jindex_in(struct gfs_jindex *jindex, char *buf);
 extern void gfs_log_header_in(struct gfs_log_header *head, const char *buf);
 extern void gfs_log_header_print(struct gfs_log_header *lh);
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index c42f2da0..f3c85b0d 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -1153,12 +1153,12 @@ static int restore_data(int fd, struct metafd *mfd, int printonly)
 				.b_blocknr = savedata.blk,
 			};
 			if (printonly > 1 && printonly == savedata.blk) {
-				display_block_type(&dummy_bh, TRUE);
+				display_block_type(bp, savedata.blk, TRUE);
 				display_gfs2(&dummy_bh);
 				break;
 			} else if (printonly == 1) {
 				print_gfs2("%"PRId64" (l=0x%x): ", blks_saved, savedata.siglen);
-				display_block_type(&dummy_bh, TRUE);
+				display_block_type(bp, savedata.blk, TRUE);
 			}
 		} else {
 			warm_fuzzy_stuff(savedata.blk, FALSE);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 16/32] gfs2_edit: Don't use gfs2_buffer_head in display_gfs2()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (14 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 15/32] gfs2_edit: Don't use gfs2_buffer_head in display_block_type() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 17/32] gfs2_edit: restore_block() improvements Andrew Price
                   ` (15 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Now we can finally get rid of that dummy_bh in restore_data()

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/gfs2hex.c  | 37 ++++++++++++++++++-------------------
 gfs2/edit/gfs2hex.h  |  2 +-
 gfs2/edit/hexedit.c  |  2 +-
 gfs2/edit/savemeta.c |  6 +-----
 4 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index 7274303c..3204577a 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -351,7 +351,7 @@ uint64_t do_leaf_extended(char *dlebuf, struct iinfo *indir)
 	return indir->ii[0].lf.lf_next;
 }
 
-static void do_eattr_extended(struct gfs2_buffer_head *ebh)
+static void do_eattr_extended(char *buf)
 {
 	struct gfs2_ea_header ea;
 	unsigned int x;
@@ -364,9 +364,8 @@ static void do_eattr_extended(struct gfs2_buffer_head *ebh)
 	     x += ea.ea_rec_len)
 	{
 		eol(0);
-		gfs2_ea_header_in(&ea, ebh->b_data + x);
-		gfs2_ea_header_print(&ea, ebh->b_data + x +
-				     sizeof(struct gfs2_ea_header));
+		gfs2_ea_header_in(&ea, buf + x);
+		gfs2_ea_header_print(&ea, buf + x + sizeof(struct gfs2_ea_header));
 	}
 }
 
@@ -429,11 +428,11 @@ static void gfs2_sb_print2(struct gfs2_sb *sbp2)
 /**
  * gfs1_rgrp_in - read in a gfs1 rgrp
  */
-static void gfs1_rgrp_in(struct gfs_rgrp *rgrp, struct gfs2_buffer_head *rbh)
+static void gfs1_rgrp_in(struct gfs_rgrp *rgrp, const char *buf)
 {
-        struct gfs_rgrp *str = (struct gfs_rgrp *)rbh->b_data;
+        struct gfs_rgrp *str = (struct gfs_rgrp *)buf;
 
-        gfs2_meta_header_in(&rgrp->rg_header, rbh->b_data);
+        gfs2_meta_header_in(&rgrp->rg_header, buf);
         rgrp->rg_flags = be32_to_cpu(str->rg_flags);
         rgrp->rg_free = be32_to_cpu(str->rg_free);
         rgrp->rg_useddi = be32_to_cpu(str->rg_useddi);
@@ -460,7 +459,7 @@ static void gfs1_rgrp_print(struct gfs_rgrp *rg)
         pv(rg, rg_freemeta, "%u", "0x%x");
 }
 
-int display_gfs2(struct gfs2_buffer_head *dbh)
+int display_gfs2(char *buf)
 {
 	struct gfs2_meta_header mh;
 	struct gfs2_rgrp rg;
@@ -472,12 +471,12 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 
 	uint32_t magic;
 
-	magic = be32_to_cpu(*(uint32_t *)dbh->b_data);
+	magic = be32_to_cpu(*(uint32_t *)buf);
 
 	switch (magic)
 	{
 	case GFS2_MAGIC:
-		gfs2_meta_header_in(&mh, dbh->b_data);
+		gfs2_meta_header_in(&mh, buf);
 		if (mh.mh_type > GFS2_METATYPE_QC)
 			print_gfs2("Unknown metadata type");
 		else
@@ -487,7 +486,7 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 		switch (mh.mh_type)
 		{
 		case GFS2_METATYPE_SB:
-			gfs2_sb_in(&sbd.sd_sb, dbh->b_data);
+			gfs2_sb_in(&sbd.sd_sb, buf);
 			gfs2_sb_print2(&sbd.sd_sb);
 			break;
 
@@ -495,10 +494,10 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 			if (sbd.gfs1) {
 				struct gfs_rgrp rg1;
 
-				gfs1_rgrp_in(&rg1, dbh);
+				gfs1_rgrp_in(&rg1, buf);
 				gfs1_rgrp_print(&rg1);
 			} else {
-				gfs2_rgrp_in(&rg, dbh->b_data);
+				gfs2_rgrp_in(&rg, buf);
 				gfs2_rgrp_print(&rg);
 			}
 			break;
@@ -516,7 +515,7 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 			break;
 
 		case GFS2_METATYPE_LF:
-			gfs2_leaf_in(&lf, dbh->b_data);
+			gfs2_leaf_in(&lf, buf);
 			gfs2_leaf_print(&lf);
 			break;
 
@@ -526,21 +525,21 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 
 		case GFS2_METATYPE_LH:
 			if (sbd.gfs1) {
-				gfs_log_header_in(&lh1, dbh->b_data);
+				gfs_log_header_in(&lh1, buf);
 				gfs_log_header_print(&lh1);
 			} else {
-				gfs2_log_header_in(&lh, dbh->b_data);
+				gfs2_log_header_in(&lh, buf);
 				gfs2_log_header_print(&lh);
 			}
 			break;
 
 		case GFS2_METATYPE_LD:
-			gfs2_log_descriptor_in(&ld, dbh->b_data);
+			gfs2_log_descriptor_in(&ld, buf);
 			gfs2_log_descriptor_print(&ld);
 			break;
 
 		case GFS2_METATYPE_EA:
-			do_eattr_extended(dbh);
+			do_eattr_extended(buf);
 			break;
 
 		case GFS2_METATYPE_ED:
@@ -552,7 +551,7 @@ int display_gfs2(struct gfs2_buffer_head *dbh)
 			break;
 
 		case GFS2_METATYPE_QC:
-			gfs2_quota_change_in(&qc, dbh->b_data);
+			gfs2_quota_change_in(&qc, buf);
 			gfs2_quota_change_print(&qc);
 			break;
 
diff --git a/gfs2/edit/gfs2hex.h b/gfs2/edit/gfs2hex.h
index 6c6ddde3..878ab801 100644
--- a/gfs2/edit/gfs2hex.h
+++ b/gfs2/edit/gfs2hex.h
@@ -3,7 +3,7 @@
 
 #include "hexedit.h"
 
-extern int display_gfs2(struct gfs2_buffer_head *dbh);
+extern int display_gfs2(char *buf);
 extern int edit_gfs2(void);
 extern void do_dinode_extended(struct gfs2_dinode *di, char *buf);
 extern void print_gfs2(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 6019996d..f8b6f67c 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1099,7 +1099,7 @@ int display(int identify_only, int trunc_zeros, uint64_t flagref,
 		        flagref, ref_blk);
 	else if (dmode == GFS2_MODE) { /* if structure display */
 		if (block != JOURNALS_DUMMY_BLOCK)
-			display_gfs2(bh);  /* display the gfs2 structure */
+			display_gfs2(bh->b_data);
 	} else
 		display_extended();        /* display extended blocks       */
 	/* No else here because display_extended can switch back to hex mode */
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index f3c85b0d..43ac4251 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -1148,13 +1148,9 @@ static int restore_data(int fd, struct metafd *mfd, int printonly)
 		}
 
 		if (printonly) {
-			struct gfs2_buffer_head dummy_bh = {
-				.b_data = bp,
-				.b_blocknr = savedata.blk,
-			};
 			if (printonly > 1 && printonly == savedata.blk) {
 				display_block_type(bp, savedata.blk, TRUE);
-				display_gfs2(&dummy_bh);
+				display_gfs2(bp);
 				break;
 			} else if (printonly == 1) {
 				print_gfs2("%"PRId64" (l=0x%x): ", blks_saved, savedata.siglen);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 17/32] gfs2_edit: restore_block() improvements
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (15 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 16/32] gfs2_edit: Don't use gfs2_buffer_head in display_gfs2() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 18/32] savemeta: Simplify di_save_len() Andrew Price
                   ` (14 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

No need for maxlen and checklen as the only caller passes in the block
size which is always used as the check length. Return the buffer instead
of an error flag to fix an uninitialised 'bp' warning in restore_data()
and just check mfd->eof again in the caller.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 43ac4251..0c541553 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -1054,15 +1054,15 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 	exit(0);
 }
 
-static int restore_block(struct metafd *mfd, struct saved_metablock *svb, char **buf, uint16_t maxlen)
+static char *restore_block(struct metafd *mfd, struct saved_metablock *svb)
 {
 	struct saved_metablock *svb_be;
 	const char *errstr;
-	uint16_t checklen;
+	char *buf = NULL;
 
 	svb_be = (struct saved_metablock *)(restore_buf_next(mfd, sizeof(*svb)));
 	if (svb_be == NULL)
-		goto read_err;
+		goto nobuffer;
 	svb->blk = be64_to_cpu(svb_be->blk);
 	svb->siglen = be16_to_cpu(svb_be->siglen);
 
@@ -1070,34 +1070,25 @@ static int restore_block(struct metafd *mfd, struct saved_metablock *svb, char *
 		fprintf(stderr, "Error: File system is too small to restore this metadata.\n");
 		fprintf(stderr, "File system is %llu blocks. Restore block = %llu\n",
 		        (unsigned long long)sbd.fssize, (unsigned long long)svb->blk);
-		return -1;
+		return NULL;
 	}
 
-	if (maxlen)
-		checklen = maxlen;
-	else
-		checklen = sbd.bsize;
-
-	if (checklen && svb->siglen > checklen) {
+	if (svb->siglen > sbd.bsize) {
 		fprintf(stderr, "Bad record length: %u for block %"PRIu64" (0x%"PRIx64").\n",
 			svb->siglen, svb->blk, svb->blk);
-		return -1;
-	}
-
-	if (buf != NULL && maxlen != 0) {
-		*buf = restore_buf_next(mfd, svb->siglen);
-		if (*buf == NULL)
-			goto read_err;
+		return NULL;
 	}
-	return 0;
 
-read_err:
+	buf = restore_buf_next(mfd, svb->siglen);
+	if (buf != NULL)
+		return buf;
+nobuffer:
 	if (mfd->eof)
-		return 1;
+		return NULL;
 
 	errstr = mfd->strerr(mfd);
 	fprintf(stderr, "Failed to restore block: %s\n", errstr);
-	return -1;
+	return NULL;
 }
 
 static int restore_super(struct metafd *mfd, char *buf, int printonly)
@@ -1127,7 +1118,6 @@ static int restore_data(int fd, struct metafd *mfd, int printonly)
 	struct saved_metablock savedata = {0};
 	uint64_t writes = 0;
 	char *buf;
-	char *bp;
 
 	buf = calloc(1, sbd.bsize);
 	if (buf == NULL) {
@@ -1137,16 +1127,15 @@ static int restore_data(int fd, struct metafd *mfd, int printonly)
 
 	blks_saved = 0;
 	while (TRUE) {
-		int err;
+		char *bp;
 
-		err = restore_block(mfd, &savedata, &bp, sbd.bsize);
-		if (err == 1)
+		bp = restore_block(mfd, &savedata);
+		if (bp == NULL && mfd->eof)
 			break;
-		if (err != 0) {
+		if (bp == NULL) {
 			free(buf);
 			return -1;
 		}
-
 		if (printonly) {
 			if (printonly > 1 && printonly == savedata.blk) {
 				display_block_type(bp, savedata.blk, TRUE);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 18/32] savemeta: Simplify di_save_len()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (16 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 17/32] gfs2_edit: restore_block() improvements Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 19/32] savemeta: Remove gfs2_buffer_head from get_gfs_struct_info() Andrew Price
                   ` (13 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

We only need to consider 2 or 3 dinode fields to decide how much of a
dinode block needs to be saved, so ditch the ..._inode_get() calls
and just look at the relevant fields. Also accept a char * instead of a
gfs2_buffer_head because that's all we need now.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 41 +++++++++++++++--------------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 0c541553..249b49af 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -296,36 +296,25 @@ static int block_is_systemfile(uint64_t blk)
 		block_is_per_node(blk) || block_is_in_per_node(blk);
 }
 
-static size_t di_save_len(struct gfs2_buffer_head *bh, uint64_t owner)
+static size_t di_save_len(const char *buf, uint64_t owner)
 {
-	struct gfs2_inode *inode;
-	struct gfs2_dinode *dn;
-	size_t len;
-
-	if (sbd.gfs1)
-		inode = lgfs2_gfs_inode_get(&sbd, bh->b_data);
-	else
-		inode = lgfs2_inode_get(&sbd, bh);
+	const struct gfs2_dinode *dn;
+	uint16_t di_height;
+	uint32_t di_mode;
+	int gfs1dir;
 
-	if (inode == NULL) {
-		fprintf(stderr, "Error reading inode at %"PRIu64": %s\n",
-		        bh->b_blocknr, strerror(errno));
-		return 0; /* Skip the block */
-	}
-	dn = &inode->i_di;
-	len = sizeof(struct gfs2_dinode);
+	dn = (void *)buf;
+	di_mode = be32_to_cpu(dn->di_mode);
+	di_height = be16_to_cpu(dn->di_height);
+	/* __pad1 is di_type in gfs1 */
+	gfs1dir = sbd.gfs1 && (be16_to_cpu(dn->__pad1) == GFS_FILE_DIR);
 
 	/* Do not save (user) data from the inode block unless they are
 	   indirect pointers, dirents, symlinks or fs internal data */
-	if (dn->di_height != 0 ||
-	    S_ISDIR(dn->di_mode) ||
-	    S_ISLNK(dn->di_mode) ||
-	    (sbd.gfs1 && dn->__pad1 == GFS_FILE_DIR) ||
-	    block_is_systemfile(owner))
-		len = sbd.bsize;
-
-	inode_put(&inode);
-	return len;
+	if (di_height > 0 || S_ISDIR(di_mode) || S_ISLNK(di_mode) || gfs1dir
+	    || block_is_systemfile(owner))
+		return sbd.bsize;
+	return sizeof(struct gfs2_dinode);
 }
 
 /*
@@ -366,7 +355,7 @@ static int get_gfs_struct_info(struct gfs2_buffer_head *lbh, uint64_t owner,
 		*gstruct_len = sbd.bsize;
 		break;
 	case GFS2_METATYPE_DI:   /* 4 (disk inode) */
-		*gstruct_len = di_save_len(lbh, owner);
+		*gstruct_len = di_save_len(lbh->b_data, owner);
 		break;
 	case GFS2_METATYPE_IN:   /* 5 (indir inode blklst) */
 		*gstruct_len = sbd.bsize; /*sizeof(struct gfs_indirect);*/
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 19/32] savemeta: Remove gfs2_buffer_head from get_gfs_struct_info()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (17 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 18/32] savemeta: Simplify di_save_len() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 20/32] savemeta: Remove gfs2_buffer_head from save_bh() (and rename it) Andrew Price
                   ` (12 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Now that di_save_len() accepts a char* this function can too.

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

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 249b49af..937fed48 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -320,7 +320,7 @@ static size_t di_save_len(const char *buf, uint64_t owner)
 /*
  * get_gfs_struct_info - get block type and structure length
  *
- * @lbh - The block buffer to examine
+ * @buf - The block buffer to examine
  * @owner - The block address of the parent structure
  * @block_type - pointer to integer to hold the block type
  * @gstruct_len - pointer to integer to hold the structure length
@@ -328,8 +328,8 @@ static size_t di_save_len(const char *buf, uint64_t owner)
  * returns: 0 if successful
  *          -1 if this isn't gfs metadata.
  */
-static int get_gfs_struct_info(struct gfs2_buffer_head *lbh, uint64_t owner,
-                               int *block_type, size_t *gstruct_len)
+static int get_gfs_struct_info(const char *buf, uint64_t owner, int *block_type,
+                               size_t *gstruct_len)
 {
 	struct gfs2_meta_header mh;
 
@@ -337,7 +337,7 @@ static int get_gfs_struct_info(struct gfs2_buffer_head *lbh, uint64_t owner,
 		*block_type = 0;
 	*gstruct_len = sbd.bsize;
 
-	gfs2_meta_header_in(&mh, lbh->b_data);
+	gfs2_meta_header_in(&mh, buf);
 	if (mh.mh_magic != GFS2_MAGIC)
 		return -1;
 
@@ -355,7 +355,7 @@ static int get_gfs_struct_info(struct gfs2_buffer_head *lbh, uint64_t owner,
 		*gstruct_len = sbd.bsize;
 		break;
 	case GFS2_METATYPE_DI:   /* 4 (disk inode) */
-		*gstruct_len = di_save_len(lbh->b_data, owner);
+		*gstruct_len = di_save_len(buf, owner);
 		break;
 	case GFS2_METATYPE_IN:   /* 5 (indir inode blklst) */
 		*gstruct_len = sbd.bsize; /*sizeof(struct gfs_indirect);*/
@@ -563,7 +563,7 @@ static int save_bh(struct metafd *mfd, struct gfs2_buffer_head *savebh, uint64_t
 	   because we want to know if the source inode is a system inode
 	   not the block within the inode "blk". They may or may not
 	   be the same thing. */
-	if (get_gfs_struct_info(savebh, owner, blktype, &blklen) &&
+	if (get_gfs_struct_info(savebh->b_data, owner, blktype, &blklen) &&
 	    !block_is_systemfile(owner) && owner != 0)
 		return 0; /* Not metadata, and not system file, so skip it */
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 20/32] savemeta: Remove gfs2_buffer_head from save_bh() (and rename it)
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (18 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 19/32] savemeta: Remove gfs2_buffer_head from get_gfs_struct_info() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 21/32] savemeta: Don't use gfs2_buffer_head in save_leaf_chain() Andrew Price
                   ` (11 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Now that get_gfs_struct_info() accepts a char* we can make save_bh() the
true save_buf().

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

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 937fed48..1d47f46e 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -522,7 +522,7 @@ static int savemetaclose(struct metafd *mfd)
 	return close(mfd->fd);
 }
 
-static int save_buf(struct metafd *mfd, char *buf, uint64_t addr, unsigned blklen)
+static int do_save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned blklen)
 {
 	struct saved_metablock *savedata;
 	size_t outsz;
@@ -554,7 +554,7 @@ static int save_buf(struct metafd *mfd, char *buf, uint64_t addr, unsigned blkle
 	return 0;
 }
 
-static int save_bh(struct metafd *mfd, struct gfs2_buffer_head *savebh, uint64_t owner, int *blktype)
+static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, uint64_t owner, int *blktype)
 {
 	size_t blklen;
 
@@ -563,11 +563,11 @@ static int save_bh(struct metafd *mfd, struct gfs2_buffer_head *savebh, uint64_t
 	   because we want to know if the source inode is a system inode
 	   not the block within the inode "blk". They may or may not
 	   be the same thing. */
-	if (get_gfs_struct_info(savebh->b_data, owner, blktype, &blklen) &&
+	if (get_gfs_struct_info(buf, owner, blktype, &blklen) &&
 	    !block_is_systemfile(owner) && owner != 0)
 		return 0; /* Not metadata, and not system file, so skip it */
 
-	return save_buf(mfd, savebh->b_data, savebh->b_blocknr, blklen);
+	return do_save_buf(mfd, buf, addr, blklen);
 }
 
 static int save_block(int fd, struct metafd *mfd, uint64_t blk, uint64_t owner, int *blktype)
@@ -583,7 +583,7 @@ static int save_block(int fd, struct metafd *mfd, uint64_t blk, uint64_t owner,
 	savebh = bread(&sbd, blk);
 	if (savebh == NULL)
 		return 1;
-	err = save_bh(mfd, savebh, owner, blktype);
+	err = save_buf(mfd, savebh->b_data, blk, owner, blktype);
 	brelse(savebh);
 	return err;
 }
@@ -671,7 +671,7 @@ static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t bl
 		}
 		warm_fuzzy_stuff(blk, FALSE);
 		if (gfs2_check_meta(bh->b_data, GFS2_METATYPE_LF) == 0) {
-			int ret = save_bh(mfd, bh, blk, NULL);
+			int ret = save_buf(mfd, bh->b_data, blk, blk, NULL);
 			if (ret != 0) {
 				brelse(bh);
 				return ret;
@@ -912,7 +912,7 @@ static void save_rgrp(struct gfs2_sbd *sdp, struct metafd *mfd, struct rgrp_tree
 	/* Save the rg and bitmaps */
 	for (unsigned i = 0; i < rgd->ri.ri_length; i++) {
 		warm_fuzzy_stuff(rgd->ri.ri_addr + i, FALSE);
-		save_buf(mfd, buf + (i * sdp->bsize), rgd->ri.ri_addr + i, sdp->bsize);
+		do_save_buf(mfd, buf + (i * sdp->bsize), rgd->ri.ri_addr + i, sdp->bsize);
 	}
 	/* Save the other metadata: inodes, etc. if mode is not 'savergs' */
 	if (withcontents)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 21/32] savemeta: Don't use gfs2_buffer_head in save_leaf_chain()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (19 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 20/32] savemeta: Remove gfs2_buffer_head from save_bh() (and rename it) Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 22/32] savemeta: Remove gfs2_buffer_head from save_block() Andrew Price
                   ` (10 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Now that we can use save_buf() without a bh, we can save some
allocations by using one buffer for every block in the chain.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 1d47f46e..47c998c9 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -658,30 +658,40 @@ static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
 
 static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t blk)
 {
-	struct gfs2_buffer_head *bh;
 	struct gfs2_leaf leaf;
+	char *buf = calloc(1, sdp->bsize);
+
+	if (buf == NULL) {
+		perror("Failed to save leaf blocks");
+		return 1;
+	}
 
 	do {
+		ssize_t r;
+
 		if (gfs2_check_range(sdp, blk) != 0)
 			return 0;
-		bh = bread(sdp, blk);
-		if (bh == NULL) {
-			perror("Failed to read leaf block");
+
+		r = pread(sdp->device_fd, buf, sdp->bsize, sdp->bsize * blk);
+		if (r != sdp->bsize) {
+			fprintf(stderr, "Failed to read leaf block %"PRIx64": %s\n",
+			        blk, strerror(errno));
+			free(buf);
 			return 1;
 		}
 		warm_fuzzy_stuff(blk, FALSE);
-		if (gfs2_check_meta(bh->b_data, GFS2_METATYPE_LF) == 0) {
-			int ret = save_buf(mfd, bh->b_data, blk, blk, NULL);
+		if (gfs2_check_meta(buf, GFS2_METATYPE_LF) == 0) {
+			int ret = save_buf(mfd, buf, blk, blk, NULL);
 			if (ret != 0) {
-				brelse(bh);
+				free(buf);
 				return ret;
 			}
 		}
-		gfs2_leaf_in(&leaf, bh->b_data);
-		brelse(bh);
+		gfs2_leaf_in(&leaf, buf);
 		blk = leaf.lf_next;
 	} while (leaf.lf_next != 0);
 
+	free(buf);
 	return 0;
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 22/32] savemeta: Remove gfs2_buffer_head from save_block()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (20 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 21/32] savemeta: Don't use gfs2_buffer_head in save_leaf_chain() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 23/32] savemeta: Split block reading from saving Andrew Price
                   ` (9 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 47c998c9..8afa8e60 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -572,19 +572,28 @@ static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, uint64_t
 
 static int save_block(int fd, struct metafd *mfd, uint64_t blk, uint64_t owner, int *blktype)
 {
-	struct gfs2_buffer_head *savebh;
+	char *buf;
 	int err;
 
+	buf = calloc(1, sbd.bsize);
+	if (buf == NULL) {
+		fprintf(stderr, "Failed to read block 0x%"PRIx64" (in 0x%"PRIx64"): %s\n",
+		        blk, owner, strerror(errno));
+		return 1;
+	}
 	if (gfs2_check_range(&sbd, blk) && blk != LGFS2_SB_ADDR(&sbd)) {
 		fprintf(stderr, "Warning: bad pointer 0x%"PRIx64" ignored in block 0x%"PRIx64"\n",
 		        blk, owner);
 		return 0;
 	}
-	savebh = bread(&sbd, blk);
-	if (savebh == NULL)
+	if (pread(sbd.device_fd, buf, sbd.bsize, sbd.bsize * blk) != sbd.bsize) {
+		fprintf(stderr, "Failed to read block 0x%"PRIx64" (in 0x%"PRIx64"): %s\n",
+		        blk, owner, strerror(errno));
+		free(buf);
 		return 1;
-	err = save_buf(mfd, savebh->b_data, blk, owner, blktype);
-	brelse(savebh);
+	}
+	err = save_buf(mfd, buf, blk, owner, blktype);
+	free(buf);
 	return err;
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 23/32] savemeta: Split block reading from saving
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (21 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 22/32] savemeta: Remove gfs2_buffer_head from save_block() Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 24/32] savemeta: Call get_struct_info() in the read path Andrew Price
                   ` (8 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

We lose a lot of control by not having access to the block data at the
point where we decide it needs to be saved, so split save_buf() out of
save_block() (which is renamed to check_read_block()) and add the
minimum required buffer propagation at the call points.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 93 +++++++++++++++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 28 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 8afa8e60..cbead772 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -570,31 +570,25 @@ static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, uint64_t
 	return do_save_buf(mfd, buf, addr, blklen);
 }
 
-static int save_block(int fd, struct metafd *mfd, uint64_t blk, uint64_t owner, int *blktype)
+static char *check_read_block(int fd, uint64_t blk)
 {
-	char *buf;
-	int err;
-
-	buf = calloc(1, sbd.bsize);
+	char *buf = calloc(1, sbd.bsize);
 	if (buf == NULL) {
-		fprintf(stderr, "Failed to read block 0x%"PRIx64" (in 0x%"PRIx64"): %s\n",
-		        blk, owner, strerror(errno));
-		return 1;
+		perror("Failed to read block");
+		return NULL;
 	}
 	if (gfs2_check_range(&sbd, blk) && blk != LGFS2_SB_ADDR(&sbd)) {
-		fprintf(stderr, "Warning: bad pointer 0x%"PRIx64" ignored in block 0x%"PRIx64"\n",
-		        blk, owner);
-		return 0;
+		fprintf(stderr, "Warning: bad pointer 0x%"PRIx64" ignored.\n", blk);
+		free(buf);
+		return NULL;
 	}
 	if (pread(sbd.device_fd, buf, sbd.bsize, sbd.bsize * blk) != sbd.bsize) {
-		fprintf(stderr, "Failed to read block 0x%"PRIx64" (in 0x%"PRIx64"): %s\n",
-		        blk, owner, strerror(errno));
+		fprintf(stderr, "Failed to read block 0x%"PRIx64": %s\n",
+		        blk, strerror(errno));
 		free(buf);
-		return 1;
+		return NULL;
 	}
-	err = save_buf(mfd, buf, blk, owner, blktype);
-	free(buf);
-	return err;
+	return buf;
 }
 
 /*
@@ -611,6 +605,8 @@ static void save_ea_block(struct metafd *mfd, char *buf, uint64_t owner)
 
 		gfs2_ea_header_in(&ea, buf + e);
 		for (i = 0; i < ea.ea_num_ptrs; i++) {
+			char *_buf;
+
 			charoff = e + ea.ea_name_len +
 				sizeof(struct gfs2_ea_header) +
 				sizeof(uint64_t) - 1;
@@ -618,7 +614,11 @@ static void save_ea_block(struct metafd *mfd, char *buf, uint64_t owner)
 			b = (uint64_t *)buf;
 			b += charoff + i;
 			blk = be64_to_cpu(*b);
-			save_block(sbd.device_fd, mfd, blk, owner, NULL);
+			_buf = check_read_block(sbd.device_fd, blk);
+			if (_buf != NULL) {
+				save_buf(mfd, _buf, blk, owner, NULL);
+				free(_buf);
+			}
 		}
 		if (!ea.ea_rec_len)
 			break;
@@ -642,13 +642,21 @@ static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
 
 	for (ptr = (uint64_t *)(mybh->b_data + head_size);
 	     (char *)ptr < (mybh->b_data + sbd.bsize); ptr++) {
+		char *buf;
+
 		if (!*ptr)
 			continue;
 		indir_block = be64_to_cpu(*ptr);
 		if (indir_block == old_block)
 			continue;
 		old_block = indir_block;
-		save_block(sbd.device_fd, mfd, indir_block, owner, &blktype);
+
+		buf = check_read_block(sbd.device_fd, indir_block);
+		if (buf != NULL) {
+			save_buf(mfd, buf, indir_block, owner, &blktype);
+			free(buf);
+		}
+
 		if (blktype == GFS2_METATYPE_EA) {
 			nbh = bread(&sbd, indir_block);
 			save_ea_block(mfd, nbh->b_data, owner);
@@ -799,9 +807,14 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 	if (inode->i_di.di_eattr) { /* if this inode has extended attributes */
 		struct gfs2_buffer_head *lbh;
 		int mhtype;
+		char *buf;
 
 		lbh = bread(&sbd, inode->i_di.di_eattr);
-		save_block(sbd.device_fd, mfd, inode->i_di.di_eattr, iblk, &mhtype);
+		buf = check_read_block(sbd.device_fd, inode->i_di.di_eattr);
+		if (buf != NULL) {
+			save_buf(mfd, buf, inode->i_di.di_eattr, iblk, &mhtype);
+			free(buf);
+		}
 		if (mhtype == GFS2_METATYPE_EA)
 			save_ea_block(mfd, lbh->b_data, iblk);
 		else if (mhtype == GFS2_METATYPE_IN)
@@ -871,9 +884,15 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_DINODE);
 
 		for (j = 0; j < m; j++) {
+			char *buf;
+
 			blk = ibuf[j];
 			warm_fuzzy_stuff(blk, FALSE);
-			save_block(sbd.device_fd, mfd, blk, blk, &blktype);
+			buf = check_read_block(sbd.device_fd, blk);
+			if (buf != NULL) {
+				save_buf(mfd, buf, blk, blk, &blktype);
+				free(buf);
+			}
 			if (blktype == GFS2_METATYPE_DI)
 				save_inode_data(mfd, blk);
 		}
@@ -884,7 +903,11 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 		 * If we don't, we may run into metadata allocation issues. */
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_UNLINKED);
 		for (j = 0; j < m; j++) {
-			save_block(sbd.device_fd, mfd, blk, blk, NULL);
+			char *buf = check_read_block(sbd.device_fd, blk);
+			if (buf != NULL) {
+				save_buf(mfd, buf, blk, blk, &blktype);
+				free(buf);
+			}
 		}
 	}
 	free(ibuf);
@@ -984,6 +1007,7 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 	struct metafd mfd;
 	struct osi_node *n;
 	int err = 0;
+	char *buf;
 
 	sbd.md.journals = 1;
 
@@ -1018,7 +1042,11 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		exit(1);
 	}
 	/* Save off the superblock */
-	save_block(sbd.device_fd, &mfd, GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize, 0, NULL);
+	buf = check_read_block(sbd.device_fd, GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize);
+	if (buf != NULL) {
+		save_buf(&mfd, buf, GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize, 0, NULL);
+		free(buf);
+	}
 	/* If this is gfs1, save off the rindex because it's not
 	   part of the file system as it is in gfs2. */
 	if (sbd.gfs1) {
@@ -1026,15 +1054,24 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		int j;
 
 		blk = sbd1->sb_rindex_di.no_addr;
-		save_block(sbd.device_fd, &mfd, blk, blk, NULL);
+		buf = check_read_block(sbd.device_fd, blk);
+		if (buf != NULL) {
+			save_buf(&mfd, buf, blk, blk, NULL);
+			free(buf);
+		}
 		save_inode_data(&mfd, blk);
 		/* In GFS1, journals aren't part of the RG space */
 		for (j = 0; j < journals_found; j++) {
+			uint64_t jb = journal_blocks[j];
+
 			log_debug("Saving journal #%d\n", j + 1);
-			for (blk = journal_blocks[j];
-			     blk < journal_blocks[j] + gfs1_journal_size;
-			     blk++)
-				save_block(sbd.device_fd, &mfd, blk, blk, NULL);
+			for (blk = jb; blk < (jb + gfs1_journal_size); blk++) {
+				buf = check_read_block(sbd.device_fd, blk);
+				if (buf != NULL) {
+					save_buf(&mfd, buf, blk, blk, NULL);
+					free(buf);
+				}
+			}
 		}
 	}
 	/* Walk through the resource groups saving everything within */
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 24/32] savemeta: Call get_struct_info() in the read path
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (22 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 23/32] savemeta: Split block reading from saving Andrew Price
@ 2020-08-06 13:37 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 25/32] savemeta: Introduce multi-block reads Andrew Price
                   ` (7 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Makes no sense to figure out the block type and length in save_buf(),
it's too late to do much with it at that point. Move the
get_gfs_struct_info() call and other checks into check_read_block().
Which means save_buf() can now be merged with do_save_buf().

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 89 +++++++++++++++++++++++---------------------
 1 file changed, 47 insertions(+), 42 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index cbead772..0de7dbf9 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -335,7 +335,9 @@ static int get_gfs_struct_info(const char *buf, uint64_t owner, int *block_type,
 
 	if (block_type != NULL)
 		*block_type = 0;
-	*gstruct_len = sbd.bsize;
+
+	if (gstruct_len != NULL)
+		*gstruct_len = sbd.bsize;
 
 	gfs2_meta_header_in(&mh, buf);
 	if (mh.mh_magic != GFS2_MAGIC)
@@ -344,6 +346,9 @@ static int get_gfs_struct_info(const char *buf, uint64_t owner, int *block_type,
 	if (block_type != NULL)
 		*block_type = mh.mh_type;
 
+	if (gstruct_len == NULL)
+		return 0;
+
 	switch (mh.mh_type) {
 	case GFS2_METATYPE_SB:   /* 1 (superblock) */
 		*gstruct_len = sizeof(struct gfs_sb);
@@ -522,7 +527,7 @@ static int savemetaclose(struct metafd *mfd)
 	return close(mfd->fd);
 }
 
-static int do_save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned blklen)
+static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned blklen)
 {
 	struct saved_metablock *savedata;
 	size_t outsz;
@@ -554,23 +559,7 @@ static int do_save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsig
 	return 0;
 }
 
-static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, uint64_t owner, int *blktype)
-{
-	size_t blklen;
-
-	/* If this isn't metadata and isn't a system file, we don't want it.
-	   Note that we're checking "owner" here rather than blk.  That's
-	   because we want to know if the source inode is a system inode
-	   not the block within the inode "blk". They may or may not
-	   be the same thing. */
-	if (get_gfs_struct_info(buf, owner, blktype, &blklen) &&
-	    !block_is_systemfile(owner) && owner != 0)
-		return 0; /* Not metadata, and not system file, so skip it */
-
-	return do_save_buf(mfd, buf, addr, blklen);
-}
-
-static char *check_read_block(int fd, uint64_t blk)
+static char *check_read_block(int fd, uint64_t blk, uint64_t owner, int *blktype, size_t *blklen)
 {
 	char *buf = calloc(1, sbd.bsize);
 	if (buf == NULL) {
@@ -588,6 +577,11 @@ static char *check_read_block(int fd, uint64_t blk)
 		free(buf);
 		return NULL;
 	}
+	if (get_gfs_struct_info(buf, owner, blktype, blklen) &&
+	    !block_is_systemfile(owner) && owner != 0) {
+		free(buf);
+		return NULL;
+	}
 	return buf;
 }
 
@@ -614,9 +608,9 @@ static void save_ea_block(struct metafd *mfd, char *buf, uint64_t owner)
 			b = (uint64_t *)buf;
 			b += charoff + i;
 			blk = be64_to_cpu(*b);
-			_buf = check_read_block(sbd.device_fd, blk);
+			_buf = check_read_block(sbd.device_fd, blk, owner, NULL, NULL);
 			if (_buf != NULL) {
-				save_buf(mfd, _buf, blk, owner, NULL);
+				save_buf(mfd, _buf, blk, sbd.bsize);
 				free(_buf);
 			}
 		}
@@ -633,7 +627,7 @@ static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
 {
 	uint64_t old_block = 0, indir_block;
 	uint64_t *ptr;
-	int head_size, blktype;
+	int head_size;
 	struct gfs2_buffer_head *nbh;
 
 	head_size = (hgt > 1 ?
@@ -642,6 +636,8 @@ static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
 
 	for (ptr = (uint64_t *)(mybh->b_data + head_size);
 	     (char *)ptr < (mybh->b_data + sbd.bsize); ptr++) {
+		size_t blklen;
+		int blktype;
 		char *buf;
 
 		if (!*ptr)
@@ -651,9 +647,9 @@ static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
 			continue;
 		old_block = indir_block;
 
-		buf = check_read_block(sbd.device_fd, indir_block);
+		buf = check_read_block(sbd.device_fd, indir_block, owner, &blktype, &blklen);
 		if (buf != NULL) {
-			save_buf(mfd, buf, indir_block, owner, &blktype);
+			save_buf(mfd, buf, indir_block, blklen);
 			free(buf);
 		}
 
@@ -698,7 +694,7 @@ static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t bl
 		}
 		warm_fuzzy_stuff(blk, FALSE);
 		if (gfs2_check_meta(buf, GFS2_METATYPE_LF) == 0) {
-			int ret = save_buf(mfd, buf, blk, blk, NULL);
+			int ret = save_buf(mfd, buf, blk, sdp->bsize);
 			if (ret != 0) {
 				free(buf);
 				return ret;
@@ -806,13 +802,16 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 	}
 	if (inode->i_di.di_eattr) { /* if this inode has extended attributes */
 		struct gfs2_buffer_head *lbh;
+		size_t blklen;
+		uint64_t blk;
 		int mhtype;
 		char *buf;
 
-		lbh = bread(&sbd, inode->i_di.di_eattr);
-		buf = check_read_block(sbd.device_fd, inode->i_di.di_eattr);
+		blk = inode->i_di.di_eattr;
+		lbh = bread(&sbd, blk);
+		buf = check_read_block(sbd.device_fd, blk, iblk, &mhtype, &blklen);
 		if (buf != NULL) {
-			save_buf(mfd, buf, inode->i_di.di_eattr, iblk, &mhtype);
+			save_buf(mfd, buf, blk, blklen);
 			free(buf);
 		}
 		if (mhtype == GFS2_METATYPE_EA)
@@ -875,12 +874,14 @@ static void get_journal_inode_blocks(void)
 
 static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 {
-	int blktype;
 	uint64_t blk = 0;
 	unsigned i, j, m;
 	uint64_t *ibuf = malloc(sbd.bsize * GFS2_NBBY * sizeof(uint64_t));
 
 	for (i = 0; i < rgd->ri.ri_length; i++) {
+		size_t blen;
+		int btype;
+
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_DINODE);
 
 		for (j = 0; j < m; j++) {
@@ -888,13 +889,13 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 
 			blk = ibuf[j];
 			warm_fuzzy_stuff(blk, FALSE);
-			buf = check_read_block(sbd.device_fd, blk);
+			buf = check_read_block(sbd.device_fd, blk, blk, &btype, &blen);
 			if (buf != NULL) {
-				save_buf(mfd, buf, blk, blk, &blktype);
+				save_buf(mfd, buf, blk, blen);
 				free(buf);
+				if (btype == GFS2_METATYPE_DI)
+					save_inode_data(mfd, blk);
 			}
-			if (blktype == GFS2_METATYPE_DI)
-				save_inode_data(mfd, blk);
 		}
 		if (!sbd.gfs1)
 			continue;
@@ -903,9 +904,9 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 		 * If we don't, we may run into metadata allocation issues. */
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_UNLINKED);
 		for (j = 0; j < m; j++) {
-			char *buf = check_read_block(sbd.device_fd, blk);
+			char *buf = check_read_block(sbd.device_fd, blk, blk, &btype, &blen);
 			if (buf != NULL) {
-				save_buf(mfd, buf, blk, blk, &blktype);
+				save_buf(mfd, buf, blk, blen);
 				free(buf);
 			}
 		}
@@ -954,7 +955,7 @@ static void save_rgrp(struct gfs2_sbd *sdp, struct metafd *mfd, struct rgrp_tree
 	/* Save the rg and bitmaps */
 	for (unsigned i = 0; i < rgd->ri.ri_length; i++) {
 		warm_fuzzy_stuff(rgd->ri.ri_addr + i, FALSE);
-		do_save_buf(mfd, buf + (i * sdp->bsize), rgd->ri.ri_addr + i, sdp->bsize);
+		save_buf(mfd, buf + (i * sdp->bsize), rgd->ri.ri_addr + i, sdp->bsize);
 	}
 	/* Save the other metadata: inodes, etc. if mode is not 'savergs' */
 	if (withcontents)
@@ -1006,6 +1007,7 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 	struct gfs2_buffer_head *lbh;
 	struct metafd mfd;
 	struct osi_node *n;
+	uint64_t sb_addr;
 	int err = 0;
 	char *buf;
 
@@ -1042,9 +1044,10 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		exit(1);
 	}
 	/* Save off the superblock */
-	buf = check_read_block(sbd.device_fd, GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize);
+	sb_addr = GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize;
+	buf = check_read_block(sbd.device_fd, sb_addr, 0, NULL, NULL);
 	if (buf != NULL) {
-		save_buf(&mfd, buf, GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize, 0, NULL);
+		save_buf(&mfd, buf, sb_addr, sizeof(struct gfs_sb));
 		free(buf);
 	}
 	/* If this is gfs1, save off the rindex because it's not
@@ -1054,9 +1057,9 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		int j;
 
 		blk = sbd1->sb_rindex_di.no_addr;
-		buf = check_read_block(sbd.device_fd, blk);
+		buf = check_read_block(sbd.device_fd, blk, blk, NULL, NULL);
 		if (buf != NULL) {
-			save_buf(&mfd, buf, blk, blk, NULL);
+			save_buf(&mfd, buf, blk, sbd.bsize);
 			free(buf);
 		}
 		save_inode_data(&mfd, blk);
@@ -1066,9 +1069,11 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 
 			log_debug("Saving journal #%d\n", j + 1);
 			for (blk = jb; blk < (jb + gfs1_journal_size); blk++) {
-				buf = check_read_block(sbd.device_fd, blk);
+				size_t blen;
+
+				buf = check_read_block(sbd.device_fd, blk, blk, NULL, &blen);
 				if (buf != NULL) {
-					save_buf(&mfd, buf, blk, blk, NULL);
+					save_buf(&mfd, buf, blk, blen);
 					free(buf);
 				}
 			}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 25/32] savemeta: Introduce multi-block reads
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (23 preceding siblings ...)
  2020-08-06 13:37 ` [Cluster-devel] [PATCH 24/32] savemeta: Call get_struct_info() in the read path Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 26/32] savemeta: Process indirect pointers in chunks Andrew Price
                   ` (6 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

When there are contiguous ranges of inode blocks that we can read
together, read them in one chunk. They still need to be processed
individually as they might have different characteristics but this will
allow us to do larger reads where possible. The new check_read_range() and
save_range() functions are intended to be generic so that we can use
them to read indirect blocks etc. in larger chunks later.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 132 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 99 insertions(+), 33 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 0de7dbf9..50d93b0b 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -328,8 +328,8 @@ static size_t di_save_len(const char *buf, uint64_t owner)
  * returns: 0 if successful
  *          -1 if this isn't gfs metadata.
  */
-static int get_gfs_struct_info(const char *buf, uint64_t owner, int *block_type,
-                               size_t *gstruct_len)
+static int get_gfs_struct_info(const char *buf, uint64_t owner, unsigned *block_type,
+                               unsigned *gstruct_len)
 {
 	struct gfs2_meta_header mh;
 
@@ -559,30 +559,79 @@ static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned
 	return 0;
 }
 
-static char *check_read_block(int fd, uint64_t blk, uint64_t owner, int *blktype, size_t *blklen)
+struct block_range {
+	uint64_t start;
+	unsigned len;
+	unsigned *blktype;
+	unsigned *blklen;
+	char *buf;
+};
+
+static int save_range(struct metafd *mfd, struct block_range *br)
 {
-	char *buf = calloc(1, sbd.bsize);
-	if (buf == NULL) {
-		perror("Failed to read block");
-		return NULL;
+	for (unsigned i = 0; i < br->len; i++) {
+		int err;
+
+		err = save_buf(mfd, br->buf + (i * sbd.bsize), br->start + i, br->blklen[i]);
+		if (err != 0)
+			return err;
 	}
-	if (gfs2_check_range(&sbd, blk) && blk != LGFS2_SB_ADDR(&sbd)) {
-		fprintf(stderr, "Warning: bad pointer 0x%"PRIx64" ignored.\n", blk);
-		free(buf);
-		return NULL;
+	return 0;
+}
+
+static int check_read_range(int fd, struct block_range *br, uint64_t owner)
+{
+	size_t size;
+
+	br->buf = calloc(br->len, sbd.bsize + sizeof(*br->blktype) + sizeof(*br->blklen));
+	if (br->buf == NULL) {
+		perror("Failed to read range");
+		return 1;
 	}
-	if (pread(sbd.device_fd, buf, sbd.bsize, sbd.bsize * blk) != sbd.bsize) {
-		fprintf(stderr, "Failed to read block 0x%"PRIx64": %s\n",
-		        blk, strerror(errno));
-		free(buf);
-		return NULL;
+	br->blktype = (unsigned *)(br->buf + (br->len * sbd.bsize));
+	br->blklen = br->blktype + br->len;
+	if (br->start < LGFS2_SB_ADDR(&sbd) || br->start + br->len > sbd.fssize) {
+		fprintf(stderr, "Warning: bad range 0x%"PRIx64" (%u blocks) ignored.\n",
+		        br->start, br->len);
+		free(br->buf);
+		br->buf = NULL;
+		return 1;
 	}
-	if (get_gfs_struct_info(buf, owner, blktype, blklen) &&
-	    !block_is_systemfile(owner) && owner != 0) {
-		free(buf);
-		return NULL;
+	size = br->len * sbd.bsize;
+	if (pread(sbd.device_fd, br->buf, size, sbd.bsize * br->start) != size) {
+		fprintf(stderr, "Failed to read block range 0x%"PRIx64" (%u blocks): %s\n",
+		        br->start, br->len, strerror(errno));
+		free(br->buf);
+		br->buf = NULL;
+		return 1;
 	}
-	return buf;
+	for (unsigned i = 0; i < br->len; i++) {
+		char *buf = br->buf + (i * sbd.bsize);
+		uint64_t addr = br->start + i;
+		uint64_t _owner = (owner == 0) ? addr : owner;
+
+		if (get_gfs_struct_info(buf, _owner, br->blktype + i, br->blklen + i) &&
+		    !block_is_systemfile(_owner)) {
+			br->blklen[i] = 0;
+		}
+	}
+	return 0;
+}
+
+static char *check_read_block(int fd, uint64_t blk, uint64_t owner, int *blktype, size_t *blklen)
+{
+	struct block_range br = {
+		.start = blk,
+		.len = 1
+	};
+
+	if (check_read_range(fd, &br, owner) != 0)
+		return NULL;
+	if (blklen != NULL)
+		*blklen = *br.blklen;
+	if (blktype != NULL)
+		*blktype = *br.blktype;
+	return br.buf;
 }
 
 /*
@@ -872,6 +921,19 @@ static void get_journal_inode_blocks(void)
 	}
 }
 
+static void save_allocated_range(struct metafd *mfd, struct block_range *br)
+{
+	if (check_read_range(sbd.device_fd, br, 0) != 0)
+		return;
+
+	save_range(mfd, br);
+	for (unsigned i = 0; i < br->len; i++) {
+		if (br->blktype[i] == GFS2_METATYPE_DI)
+			save_inode_data(mfd, br->start + i);
+	}
+	free(br->buf);
+}
+
 static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 {
 	uint64_t blk = 0;
@@ -879,24 +941,27 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 	uint64_t *ibuf = malloc(sbd.bsize * GFS2_NBBY * sizeof(uint64_t));
 
 	for (i = 0; i < rgd->ri.ri_length; i++) {
-		size_t blen;
-		int btype;
+		struct block_range br = {0};
 
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_DINODE);
 
 		for (j = 0; j < m; j++) {
-			char *buf;
-
 			blk = ibuf[j];
-			warm_fuzzy_stuff(blk, FALSE);
-			buf = check_read_block(sbd.device_fd, blk, blk, &btype, &blen);
-			if (buf != NULL) {
-				save_buf(mfd, buf, blk, blen);
-				free(buf);
-				if (btype == GFS2_METATYPE_DI)
-					save_inode_data(mfd, blk);
+			if (br.start == 0) {
+				br.start = blk;
+				br.len = 1;
+			} else if (blk == br.start + br.len) {
+				br.len++;
+			} else {
+				save_allocated_range(mfd, &br);
+				br.start = blk;
+				br.len = 1;
 			}
+			warm_fuzzy_stuff(blk, FALSE);
 		}
+		if (br.start != 0)
+			save_allocated_range(mfd, &br);
+
 		if (!sbd.gfs1)
 			continue;
 
@@ -904,7 +969,8 @@ static void save_allocated(struct rgrp_tree *rgd, struct metafd *mfd)
 		 * If we don't, we may run into metadata allocation issues. */
 		m = lgfs2_bm_scan(rgd, i, ibuf, GFS2_BLKST_UNLINKED);
 		for (j = 0; j < m; j++) {
-			char *buf = check_read_block(sbd.device_fd, blk, blk, &btype, &blen);
+			size_t blen;
+			char *buf = check_read_block(sbd.device_fd, blk, blk, NULL, &blen);
 			if (buf != NULL) {
 				save_buf(mfd, buf, blk, blen);
 				free(buf);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 26/32] savemeta: Process indirect pointers in chunks
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (24 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 25/32] savemeta: Introduce multi-block reads Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 27/32] savemeta: Don't trim off trailing zeroes when compressing Andrew Price
                   ` (5 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Where indirect pointers point to consecutive blocks, use the new block
range structures to read them in one go. The largest read is bounded by
the number of pointers in a block, so just under 2MB for a 4K block
size. Replace the lists of buffer heads for each metadata tree height
with a simpler queue structure.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 192 ++++++++++++++++++++++++-------------------
 1 file changed, 107 insertions(+), 85 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 50d93b0b..76477b79 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -560,6 +560,7 @@ static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned
 }
 
 struct block_range {
+	struct block_range *next;
 	uint64_t start;
 	unsigned len;
 	unsigned *blktype;
@@ -567,6 +568,11 @@ struct block_range {
 	char *buf;
 };
 
+struct block_range_queue {
+	struct block_range *tail;
+	struct block_range **head;
+};
+
 static int save_range(struct metafd *mfd, struct block_range *br)
 {
 	for (unsigned i = 0; i < br->len; i++) {
@@ -668,54 +674,68 @@ static void save_ea_block(struct metafd *mfd, char *buf, uint64_t owner)
 	}
 }
 
-/*
- * save_indirect_blocks - save all indirect blocks for the given buffer
- */
-static void save_indirect_blocks(struct metafd *mfd, osi_list_t *cur_list,
-           struct gfs2_buffer_head *mybh, uint64_t owner, int height, int hgt)
+static void save_indirect_range(struct metafd *mfd, struct block_range **brp, uint64_t owner,
+                                struct block_range_queue *q)
 {
-	uint64_t old_block = 0, indir_block;
-	uint64_t *ptr;
-	int head_size;
-	struct gfs2_buffer_head *nbh;
+	struct block_range *br = *brp;
 
-	head_size = (hgt > 1 ?
-		     sizeof(struct gfs2_meta_header) :
-		     sizeof(struct gfs2_dinode));
+	if (check_read_range(sbd.device_fd, br, owner) != 0)
+		return;
 
-	for (ptr = (uint64_t *)(mybh->b_data + head_size);
-	     (char *)ptr < (mybh->b_data + sbd.bsize); ptr++) {
-		size_t blklen;
-		int blktype;
-		char *buf;
+	save_range(mfd, br);
+	for (unsigned i = 0; i < br->len; i++) {
+		if (br->blktype[i] == GFS2_METATYPE_EA)
+			save_ea_block(mfd, br->buf + (i * sbd.bsize), owner);
+	}
+	if (q) {
+		*q->head = br;
+		q->head = &br->next;
+		*brp = NULL; /* The list now has ownership of it */
+	} else {
+		free(br->buf);
+		br->buf = NULL;
+	}
+}
+
+static void save_indirect_blocks(struct metafd *mfd, char *buf, uint64_t owner,
+                                 struct block_range_queue *q, unsigned headsize)
+{
+	uint64_t old_block = 0, indir_block;
+	struct block_range *br = NULL;
+	uint64_t *ptr;
 
+	for (ptr = (uint64_t *)(buf + headsize);
+	     (char *)ptr < (buf + sbd.bsize); ptr++) {
 		if (!*ptr)
 			continue;
+
 		indir_block = be64_to_cpu(*ptr);
 		if (indir_block == old_block)
 			continue;
 		old_block = indir_block;
 
-		buf = check_read_block(sbd.device_fd, indir_block, owner, &blktype, &blklen);
-		if (buf != NULL) {
-			save_buf(mfd, buf, indir_block, blklen);
-			free(buf);
-		}
-
-		if (blktype == GFS2_METATYPE_EA) {
-			nbh = bread(&sbd, indir_block);
-			save_ea_block(mfd, nbh->b_data, owner);
-			brelse(nbh);
-		}
-		if (height != hgt && /* If not at max height and */
-		    (!gfs2_check_range(&sbd, indir_block))) {
-			nbh = bread(&sbd, indir_block);
-			osi_list_add_prev(&nbh->b_altlist, cur_list);
-			/* The buffer_head needs to be queued ahead, so
-			   don't release it!
-			   brelse(nbh);*/
+		if (br == NULL) {
+new_range:
+			br = calloc(1, sizeof(*br));
+			if (br == NULL) {
+				perror("Failed to save indirect blocks");
+				return;
+			}
+			br->start = indir_block;
+			br->len = 1;
+		} else if (indir_block == br->start + br->len) {
+			br->len++;
+		} else {
+			save_indirect_range(mfd, &br, owner, q);
+			if (br == NULL) /* This one was queued up for later */
+				goto new_range;
+			br->start = indir_block;
+			br->len = 1;
 		}
-	} /* for all data on the indirect block */
+	}
+	if (br != NULL && br->start != 0)
+		save_indirect_range(mfd, &br, owner, q);
+	free(br);
 }
 
 static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t blk)
@@ -761,6 +781,7 @@ static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t bl
  * save_inode_data - save off important data associated with an inode
  *
  * mfd - destination file descriptor
+ * buf - buffer containing the inode block
  * iblk - block number of the inode to save the data for
  *
  * For user files, we don't want anything except all the indirect block
@@ -772,17 +793,55 @@ static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t bl
  * For file system journals, the "data" is a mixture of metadata and
  * journaled data.  We want all the metadata and none of the user data.
  */
-static void save_inode_data(struct metafd *mfd, uint64_t iblk)
+static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 {
+	struct block_range_queue indq[GFS2_MAX_META_HEIGHT] = {{NULL}};
+	struct gfs2_buffer_head *metabh;
 	uint32_t height;
 	struct gfs2_inode *inode;
-	osi_list_t metalist[GFS2_MAX_META_HEIGHT];
-	osi_list_t *prev_list, *cur_list, *tmp;
-	struct gfs2_buffer_head *metabh, *mybh;
-	int i;
+	struct gfs2_dinode _di = {0};
+
+	for (unsigned i = 0; i < GFS2_MAX_META_HEIGHT; i++)
+		indq[i].head = &indq[i].tail;
+
+	gfs2_dinode_in(&_di, ibuf);
+	height = _di.di_height;
+
+	/* If this is a user inode, we don't follow to the file height.
+	   We stop one level less.  That way we save off the indirect
+	   pointer blocks but not the actual file contents. The exception
+	   is directories, where the height represents the level@which
+	   the hash table exists, and we have to save the directory data. */
+
+	if (_di.di_flags & GFS2_DIF_EXHASH && (S_ISDIR(_di.di_mode) ||
+	     (sbd.gfs1 && _di.__pad1 == GFS_FILE_DIR)))
+		height++;
+	else if (height > 0 && !(_di.di_flags & GFS2_DIF_SYSTEM) &&
+		 !block_is_systemfile(iblk) && !S_ISDIR(_di.di_mode))
+		height--;
+
+	if (height > 0)
+		save_indirect_blocks(mfd, ibuf, iblk, height == 1 ? NULL : &indq[0], sizeof(_di));
+	for (unsigned i = 1; i < height; i++) {
+		struct block_range_queue *nextq = &indq[i];
+
+		if (i == height - 1)
+			nextq = NULL;
+
+		while (indq[i - 1].tail != NULL) {
+			struct block_range *q = indq[i - 1].tail;
 
-	for (i = 0; i < GFS2_MAX_META_HEIGHT; i++)
-		osi_list_init(&metalist[i]);
+			for (unsigned j = 0; j < q->len; j++) {
+				char *_buf = q->buf + (j * sbd.bsize);
+
+				save_indirect_blocks(mfd, _buf, iblk, nextq, sizeof(_di.di_header));
+			}
+			warm_fuzzy_stuff(q->start + q->len, 0);
+			indq[i - 1].tail = q->next;
+			free(q->buf);
+			free(q);
+		}
+	}
 	metabh = bread(&sbd, iblk);
 	if (sbd.gfs1) {
 		inode = lgfs2_gfs_inode_get(&sbd, metabh->b_data);
@@ -793,45 +852,6 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 		perror("Failed to read inode");
 		exit(-1);
 	}
-	height = inode->i_di.di_height;
-	/* If this is a user inode, we don't follow to the file height.
-	   We stop one level less.  That way we save off the indirect
-	   pointer blocks but not the actual file contents. The exception
-	   is directories, where the height represents the level at which
-	   the hash table exists, and we have to save the directory data. */
-	if (inode->i_di.di_flags & GFS2_DIF_EXHASH &&
-	    (S_ISDIR(inode->i_di.di_mode) ||
-	     (sbd.gfs1 && inode->i_di.__pad1 == GFS_FILE_DIR)))
-		height++;
-	else if (height && !(inode->i_di.di_flags & GFS2_DIF_SYSTEM) &&
-		 !block_is_systemfile(iblk) && !S_ISDIR(inode->i_di.di_mode))
-		height--;
-	osi_list_add(&metabh->b_altlist, &metalist[0]);
-        for (i = 1; i <= height; i++){
-		prev_list = &metalist[i - 1];
-		cur_list = &metalist[i];
-
-		for (tmp = prev_list->next; tmp != prev_list; tmp = tmp->next){
-			mybh = osi_list_entry(tmp, struct gfs2_buffer_head,
-					      b_altlist);
-			warm_fuzzy_stuff(iblk, FALSE);
-			save_indirect_blocks(mfd, cur_list, mybh, iblk,
-					     height, i);
-		} /* for blocks at that height */
-	} /* for height */
-	/* free metalists */
-	for (i = 0; i < GFS2_MAX_META_HEIGHT; i++) {
-		cur_list = &metalist[i];
-		while (!osi_list_empty(cur_list)) {
-			mybh = osi_list_entry(cur_list->next,
-					    struct gfs2_buffer_head,
-					    b_altlist);
-			if (mybh == inode->i_bh)
-				osi_list_del(&mybh->b_altlist);
-			else
-				brelse(mybh);
-		}
-	}
 	/* Process directory exhash inodes */
 	if (S_ISDIR(inode->i_di.di_mode) &&
 	    inode->i_di.di_flags & GFS2_DIF_EXHASH) {
@@ -866,7 +886,7 @@ static void save_inode_data(struct metafd *mfd, uint64_t iblk)
 		if (mhtype == GFS2_METATYPE_EA)
 			save_ea_block(mfd, lbh->b_data, iblk);
 		else if (mhtype == GFS2_METATYPE_IN)
-			save_indirect_blocks(mfd, cur_list, lbh, iblk, 2, 2);
+			save_indirect_blocks(mfd, lbh->b_data, iblk, NULL, sizeof(di.di_header));
 		brelse(lbh);
 	}
 	inode_put(&inode);
@@ -928,8 +948,10 @@ static void save_allocated_range(struct metafd *mfd, struct block_range *br)
 
 	save_range(mfd, br);
 	for (unsigned i = 0; i < br->len; i++) {
+		char *buf = br->buf + (i * sbd.bsize);
+
 		if (br->blktype[i] == GFS2_METATYPE_DI)
-			save_inode_data(mfd, br->start + i);
+			save_inode_data(mfd, buf, br->start + i);
 	}
 	free(br->buf);
 }
@@ -1126,9 +1148,9 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		buf = check_read_block(sbd.device_fd, blk, blk, NULL, NULL);
 		if (buf != NULL) {
 			save_buf(&mfd, buf, blk, sbd.bsize);
+			save_inode_data(&mfd, buf, blk);
 			free(buf);
 		}
-		save_inode_data(&mfd, blk);
 		/* In GFS1, journals aren't part of the RG space */
 		for (j = 0; j < journals_found; j++) {
 			uint64_t jb = journal_blocks[j];
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 27/32] savemeta: Don't trim off trailing zeroes when compressing
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (25 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 26/32] savemeta: Process indirect pointers in chunks Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 28/32] savemeta: Leaf block processing improvements Andrew Price
                   ` (4 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

The code that trims the trailing zeroes from blocks before saving them
always features as one of the slow points in performance reports. When
we're compressing the file anyway, the compression algos should do this
much more efficiently than we can, so short circuit it when compression
is enabled.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 76477b79..55f64922 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -532,8 +532,10 @@ static int save_buf(struct metafd *mfd, const char *buf, uint64_t addr, unsigned
 	struct saved_metablock *savedata;
 	size_t outsz;
 
-	/* No need to save trailing zeroes */
-	for (; blklen > 0 && buf[blklen - 1] == '\0'; blklen--);
+	/* No need to save trailing zeroes, but leave that for compression to
+	   deal with when enabled as this adds a significant overhead */
+	if (mfd->gziplevel == 0)
+		for (; blklen > 0 && buf[blklen - 1] == '\0'; blklen--);
 
 	if (blklen == 0) /* No significant data; skip. */
 		return 0;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 28/32] savemeta: Leaf block processing improvements
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (26 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 27/32] savemeta: Don't trim off trailing zeroes when compressing Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 29/32] savemeta: Remove some unnecessary reads from save_inode_data() Andrew Price
                   ` (3 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

We've already walked the indirect pointer tree and read the leaf blocks
earlier, so hold onto those and use them to read down the leaf chains,
instead of doing a gfs2_readi() for every leaf block.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 59 +++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 55f64922..ea09d2bf 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -740,17 +740,14 @@ new_range:
 	free(br);
 }
 
-static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t blk)
+static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, char *buf)
 {
 	struct gfs2_leaf leaf;
-	char *buf = calloc(1, sdp->bsize);
 
-	if (buf == NULL) {
-		perror("Failed to save leaf blocks");
-		return 1;
-	}
+	gfs2_leaf_in(&leaf, buf);
 
-	do {
+	while (leaf.lf_next != 0) {
+		uint64_t blk = leaf.lf_next;
 		ssize_t r;
 
 		if (gfs2_check_range(sdp, blk) != 0)
@@ -772,13 +769,26 @@ static int save_leaf_chain(struct metafd *mfd, struct gfs2_sbd *sdp, uint64_t bl
 			}
 		}
 		gfs2_leaf_in(&leaf, buf);
-		blk = leaf.lf_next;
-	} while (leaf.lf_next != 0);
-
-	free(buf);
+	}
 	return 0;
 }
 
+static void save_leaf_blocks(struct metafd *mfd, struct block_range_queue *q)
+{
+	while (q->tail != NULL) {
+		struct block_range *br = q->tail;
+
+		for (unsigned i = 0; i < br->len; i++) {
+			char *buf = br->buf + (i * sbd.bsize);
+
+			save_leaf_chain(mfd, &sbd, buf);
+		}
+		q->tail = br->next;
+		free(br->buf);
+		free(br);
+	}
+}
+
 /*
  * save_inode_data - save off important data associated with an inode
  *
@@ -802,6 +812,7 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 	uint32_t height;
 	struct gfs2_inode *inode;
 	struct gfs2_dinode _di = {0};
+	int is_exhash;
 
 	for (unsigned i = 0; i < GFS2_MAX_META_HEIGHT; i++)
 		indq[i].head = &indq[i].tail;
@@ -815,8 +826,9 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 	   is directories, where the height represents the level@which
 	   the hash table exists, and we have to save the directory data. */
 
-	if (_di.di_flags & GFS2_DIF_EXHASH && (S_ISDIR(_di.di_mode) ||
-	     (sbd.gfs1 && _di.__pad1 == GFS_FILE_DIR)))
+	is_exhash = (S_ISDIR(_di.di_mode) || (sbd.gfs1 && _di.__pad1 == GFS_FILE_DIR)) &&
+	             _di.di_flags & GFS2_DIF_EXHASH;
+	if (is_exhash)
 		height++;
 	else if (height > 0 && !(_di.di_flags & GFS2_DIF_SYSTEM) &&
 		 !block_is_systemfile(iblk) && !S_ISDIR(_di.di_mode))
@@ -827,7 +839,7 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 	for (unsigned i = 1; i < height; i++) {
 		struct block_range_queue *nextq = &indq[i];
 
-		if (i == height - 1)
+		if (!is_exhash && i == height - 1)
 			nextq = NULL;
 
 		while (indq[i - 1].tail != NULL) {
@@ -844,6 +856,8 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 			free(q);
 		}
 	}
+	if (is_exhash)
+		save_leaf_blocks(mfd, &indq[height - 1]);
 	metabh = bread(&sbd, iblk);
 	if (sbd.gfs1) {
 		inode = lgfs2_gfs_inode_get(&sbd, metabh->b_data);
@@ -854,23 +868,6 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 		perror("Failed to read inode");
 		exit(-1);
 	}
-	/* Process directory exhash inodes */
-	if (S_ISDIR(inode->i_di.di_mode) &&
-	    inode->i_di.di_flags & GFS2_DIF_EXHASH) {
-		uint64_t  leaf_no, old_leaf = -1;
-		int li;
-
-		for (li = 0; li < (1 << inode->i_di.di_depth); li++) {
-			if (lgfs2_get_leaf_ptr(inode, li, &leaf_no)) {
-				fprintf(stderr, "Could not read leaf index %d in dinode %"PRIu64"\n", li,
-				        (uint64_t)inode->i_di.di_num.no_addr);
-				exit(-1);
-			}
-			if (leaf_no != old_leaf && save_leaf_chain(mfd, &sbd, leaf_no) != 0)
-				exit(-1);
-			old_leaf = leaf_no;
-		}
-	}
 	if (inode->i_di.di_eattr) { /* if this inode has extended attributes */
 		struct gfs2_buffer_head *lbh;
 		size_t blklen;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 29/32] savemeta: Remove some unnecessary reads from save_inode_data()
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (27 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 28/32] savemeta: Leaf block processing improvements Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 30/32] savemeta: Remove some unnecessary jindex reading code Andrew Price
                   ` (2 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

We've already read the inode block so there's no need to do it again,
and we shouldn't be reading the xattr block twice.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index ea09d2bf..272f796c 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -808,9 +808,7 @@ static void save_leaf_blocks(struct metafd *mfd, struct block_range_queue *q)
 static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 {
 	struct block_range_queue indq[GFS2_MAX_META_HEIGHT] = {{NULL}};
-	struct gfs2_buffer_head *metabh;
 	uint32_t height;
-	struct gfs2_inode *inode;
 	struct gfs2_dinode _di = {0};
 	int is_exhash;
 
@@ -858,38 +856,23 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 	}
 	if (is_exhash)
 		save_leaf_blocks(mfd, &indq[height - 1]);
-	metabh = bread(&sbd, iblk);
-	if (sbd.gfs1) {
-		inode = lgfs2_gfs_inode_get(&sbd, metabh->b_data);
-	} else {
-		inode = lgfs2_inode_get(&sbd, metabh);
-	}
-	if (inode == NULL) {
-		perror("Failed to read inode");
-		exit(-1);
-	}
-	if (inode->i_di.di_eattr) { /* if this inode has extended attributes */
-		struct gfs2_buffer_head *lbh;
+	if (_di.di_eattr) { /* if this inode has extended attributes */
 		size_t blklen;
 		uint64_t blk;
 		int mhtype;
 		char *buf;
 
-		blk = inode->i_di.di_eattr;
-		lbh = bread(&sbd, blk);
+		blk = _di.di_eattr;
 		buf = check_read_block(sbd.device_fd, blk, iblk, &mhtype, &blklen);
 		if (buf != NULL) {
 			save_buf(mfd, buf, blk, blklen);
+			if (mhtype == GFS2_METATYPE_EA)
+				save_ea_block(mfd, buf, iblk);
+			else if (mhtype == GFS2_METATYPE_IN)
+				save_indirect_blocks(mfd, buf, iblk, NULL, sizeof(_di.di_header));
 			free(buf);
 		}
-		if (mhtype == GFS2_METATYPE_EA)
-			save_ea_block(mfd, lbh->b_data, iblk);
-		else if (mhtype == GFS2_METATYPE_IN)
-			save_indirect_blocks(mfd, lbh->b_data, iblk, NULL, sizeof(di.di_header));
-		brelse(lbh);
 	}
-	inode_put(&inode);
-	brelse(metabh);
 }
 
 static void get_journal_inode_blocks(void)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 30/32] savemeta: Remove some unnecessary jindex reading code
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (28 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 29/32] savemeta: Remove some unnecessary reads from save_inode_data() Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 31/32] savemeta: Move block range queue ops into functions Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 32/32] restoremeta: Fix up superblock processing Andrew Price
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This section reads the jindex and walks its tree but the data it gathers
is not used subsequently. The jindex and journals will be picked up by
the bitmap walk later.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 272f796c..fe7a6d78 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -1073,8 +1073,6 @@ static int parse_header(char *buf, struct savemeta_header *smh)
 
 void savemeta(char *out_fn, int saveoption, int gziplevel)
 {
-	uint64_t jindex_block;
-	struct gfs2_buffer_head *lbh;
 	struct metafd mfd;
 	struct osi_node *n;
 	uint64_t sb_addr;
@@ -1090,15 +1088,6 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 		sbd.bsize = sbd.sd_sb.sb_bsize;
 	printf("There are %llu blocks of %u bytes in the filesystem.\n",
 	                     (unsigned long long)sbd.fssize, sbd.bsize);
-	if (sbd.gfs1)
-		jindex_block = sbd1->sb_jindex_di.no_addr;
-	else
-		jindex_block = masterblock("jindex");
-	lbh = bread(&sbd, jindex_block);
-	gfs2_dinode_in(&di, lbh->b_data);
-	if (!sbd.gfs1)
-		do_dinode_extended(&di, lbh->b_data);
-	brelse(lbh);
 
 	printf("Filesystem size: %.2fGB\n", (sbd.fssize * sbd.bsize) / ((float)(1 << 30)));
 	get_journal_inode_blocks();
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 31/32] savemeta: Move block range queue ops into functions
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (29 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 30/32] savemeta: Remove some unnecessary jindex reading code Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 32/32] restoremeta: Fix up superblock processing Andrew Price
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Makes things a bit less cluttered and easier to debug.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 103 +++++++++++++++++++++++++++++++------------
 1 file changed, 74 insertions(+), 29 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index fe7a6d78..759113fd 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -570,11 +570,76 @@ struct block_range {
 	char *buf;
 };
 
+static int block_range_prepare(struct block_range *br)
+{
+	br->buf = calloc(br->len, sbd.bsize + sizeof(*br->blktype) + sizeof(*br->blklen));
+	if (br->buf == NULL) {
+		perror("Failed to allocate block range buffer");
+		return 1;
+	}
+	br->blktype = (unsigned *)(br->buf + (br->len * sbd.bsize));
+	br->blklen = br->blktype + br->len;
+	return 0;
+}
+
+static int block_range_check(struct block_range *br)
+{
+	if (br->start >= LGFS2_SB_ADDR(&sbd) && br->start + br->len <= sbd.fssize)
+		return 0;
+
+	fprintf(stderr, "Warning: bad range 0x%"PRIx64" (%u blocks) ignored.\n",
+		br->start, br->len);
+	free(br->buf);
+	br->buf = NULL;
+	return 1;
+}
+
+static void block_range_setinfo(struct block_range *br, uint64_t owner)
+{
+	for (unsigned i = 0; i < br->len; i++) {
+		char *buf = br->buf + (i * sbd.bsize);
+		uint64_t addr = br->start + i;
+		uint64_t _owner = (owner == 0) ? addr : owner;
+
+		if (get_gfs_struct_info(buf, _owner, br->blktype + i, br->blklen + i) &&
+		    !block_is_systemfile(_owner)) {
+			br->blklen[i] = 0;
+		}
+	}
+}
+
+static void block_range_free(struct block_range **brp)
+{
+	free((*brp)->buf);
+	free(*brp);
+	*brp = NULL;
+}
+
 struct block_range_queue {
 	struct block_range *tail;
 	struct block_range **head;
 };
 
+static void block_range_queue_init(struct block_range_queue *q)
+{
+	q->head = &q->tail;
+}
+
+static void block_range_queue_insert(struct block_range_queue *q, struct block_range *br)
+{
+	*q->head = br;
+	q->head = &br->next;
+}
+
+static struct block_range *block_range_queue_pop(struct block_range_queue *q)
+{
+	struct block_range *br = q->tail;
+
+	q->tail = br->next;
+	br->next = NULL;
+	return br;
+}
+
 static int save_range(struct metafd *mfd, struct block_range *br)
 {
 	for (unsigned i = 0; i < br->len; i++) {
@@ -591,20 +656,12 @@ static int check_read_range(int fd, struct block_range *br, uint64_t owner)
 {
 	size_t size;
 
-	br->buf = calloc(br->len, sbd.bsize + sizeof(*br->blktype) + sizeof(*br->blklen));
-	if (br->buf == NULL) {
-		perror("Failed to read range");
+	if (block_range_prepare(br) != 0)
 		return 1;
-	}
-	br->blktype = (unsigned *)(br->buf + (br->len * sbd.bsize));
-	br->blklen = br->blktype + br->len;
-	if (br->start < LGFS2_SB_ADDR(&sbd) || br->start + br->len > sbd.fssize) {
-		fprintf(stderr, "Warning: bad range 0x%"PRIx64" (%u blocks) ignored.\n",
-		        br->start, br->len);
-		free(br->buf);
-		br->buf = NULL;
+
+	if (block_range_check(br) != 0)
 		return 1;
-	}
+
 	size = br->len * sbd.bsize;
 	if (pread(sbd.device_fd, br->buf, size, sbd.bsize * br->start) != size) {
 		fprintf(stderr, "Failed to read block range 0x%"PRIx64" (%u blocks): %s\n",
@@ -613,16 +670,7 @@ static int check_read_range(int fd, struct block_range *br, uint64_t owner)
 		br->buf = NULL;
 		return 1;
 	}
-	for (unsigned i = 0; i < br->len; i++) {
-		char *buf = br->buf + (i * sbd.bsize);
-		uint64_t addr = br->start + i;
-		uint64_t _owner = (owner == 0) ? addr : owner;
-
-		if (get_gfs_struct_info(buf, _owner, br->blktype + i, br->blklen + i) &&
-		    !block_is_systemfile(_owner)) {
-			br->blklen[i] = 0;
-		}
-	}
+	block_range_setinfo(br, owner);
 	return 0;
 }
 
@@ -690,8 +738,7 @@ static void save_indirect_range(struct metafd *mfd, struct block_range **brp, ui
 			save_ea_block(mfd, br->buf + (i * sbd.bsize), owner);
 	}
 	if (q) {
-		*q->head = br;
-		q->head = &br->next;
+		block_range_queue_insert(q, br);
 		*brp = NULL; /* The list now has ownership of it */
 	} else {
 		free(br->buf);
@@ -813,7 +860,7 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 	int is_exhash;
 
 	for (unsigned i = 0; i < GFS2_MAX_META_HEIGHT; i++)
-		indq[i].head = &indq[i].tail;
+		block_range_queue_init(&indq[i]);
 
 	gfs2_dinode_in(&_di, ibuf);
 	height = _di.di_height;
@@ -841,7 +888,7 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 			nextq = NULL;
 
 		while (indq[i - 1].tail != NULL) {
-			struct block_range *q = indq[i - 1].tail;
+			struct block_range *q = block_range_queue_pop(&indq[i - 1]);
 
 			for (unsigned j = 0; j < q->len; j++) {
 				char *_buf = q->buf + (j * sbd.bsize);
@@ -849,9 +896,7 @@ static void save_inode_data(struct metafd *mfd, char *ibuf, uint64_t iblk)
 				save_indirect_blocks(mfd, _buf, iblk, nextq, sizeof(_di.di_header));
 			}
 			warm_fuzzy_stuff(q->start + q->len, 0);
-			indq[i - 1].tail = q->next;
-			free(q->buf);
-			free(q);
+			block_range_free(&q);
 		}
 	}
 	if (is_exhash)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Cluster-devel] [PATCH 32/32] restoremeta: Fix up superblock processing
  2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
                   ` (30 preceding siblings ...)
  2020-08-06 13:38 ` [Cluster-devel] [PATCH 31/32] savemeta: Move block range queue ops into functions Andrew Price
@ 2020-08-06 13:38 ` Andrew Price
  31 siblings, 0 replies; 33+ messages in thread
From: Andrew Price @ 2020-08-06 13:38 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Prior to 933c03b 'restoremeta: Metadata file reading overhaul'
restoremeta restored the superblock to gather information, did a seek
back to before the superblock and then carried on to restore the
superblock properly. When that commit removed the seeks it didn't add
new printing code for the superblock (for gfs2_edit printsavedmeta).
Add that missing printing code and make sure the filesystem information
is printed in the right order.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/savemeta.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index 759113fd..80c11c90 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -351,7 +351,10 @@ static int get_gfs_struct_info(const char *buf, uint64_t owner, unsigned *block_
 
 	switch (mh.mh_type) {
 	case GFS2_METATYPE_SB:   /* 1 (superblock) */
-		*gstruct_len = sizeof(struct gfs_sb);
+		if (sbd.gfs1)
+			*gstruct_len = sizeof(struct gfs_sb);
+		else
+			*gstruct_len = sizeof(struct gfs2_sb);
 		break;
 	case GFS2_METATYPE_RG:   /* 2 (rsrc grp hdr) */
 		*gstruct_len = sbd.bsize; /*sizeof(struct gfs_rgrp);*/
@@ -1151,7 +1154,10 @@ void savemeta(char *out_fn, int saveoption, int gziplevel)
 	sb_addr = GFS2_SB_ADDR * GFS2_BASIC_BLOCK / sbd.bsize;
 	buf = check_read_block(sbd.device_fd, sb_addr, 0, NULL, NULL);
 	if (buf != NULL) {
-		save_buf(&mfd, buf, sb_addr, sizeof(struct gfs_sb));
+		if (sbd.gfs1)
+			save_buf(&mfd, buf, sb_addr, sizeof(struct gfs_sb));
+		else
+			save_buf(&mfd, buf, sb_addr, sizeof(struct gfs2_sb));
 		free(buf);
 	}
 	/* If this is gfs1, save off the rindex because it's not
@@ -1263,7 +1269,7 @@ static int restore_super(struct metafd *mfd, char *buf, int printonly)
 		fprintf(stderr, "Failed to write superblock\n");
 		return -1;
 	}
-	printf("Block size is %uB\n", sbd.bsize);
+	blks_saved++;
 	return 0;
 }
 
@@ -1279,7 +1285,6 @@ static int restore_data(int fd, struct metafd *mfd, int printonly)
 		exit(1);
 	}
 
-	blks_saved = 0;
 	while (TRUE) {
 		char *bp;
 
@@ -1338,6 +1343,7 @@ static int restore_init(const char *path, struct metafd *mfd, struct savemeta_he
 	char *bp;
 	int ret;
 
+	blks_saved = 0;
 	restore_buf = malloc(RESTORE_BUF_SIZE);
 	if (restore_buf == NULL) {
 		perror("Restore failed");
@@ -1379,11 +1385,26 @@ static int restore_init(const char *path, struct metafd *mfd, struct savemeta_he
 		fprintf(stderr, "No superblock found in metadata file\n");
 		return -1;
 	}
-	ret = restore_super(mfd, bp + sizeof(struct saved_metablock), printonly);
+	bp += sizeof(struct saved_metablock);
+	ret = restore_super(mfd, bp, printonly);
 	if (ret != 0)
 		return ret;
 
-	bp += sizeof(struct saved_metablock) + sb_siglen;
+	if (smh->sh_fs_bytes > 0) {
+		sbd.fssize = smh->sh_fs_bytes / sbd.bsize;
+		printf("Saved file system size is %"PRIu64" blocks, %.2fGB\n",
+		       sbd.fssize, smh->sh_fs_bytes / ((float)(1 << 30)));
+	}
+	printf("Block size is %uB\n", sbd.bsize);
+	printf("This is gfs%c metadata.\n", sbd.gfs1 ? '1': '2');
+	if (printonly > 1 && printonly == LGFS2_SB_ADDR(&sbd)) {
+		display_block_type(bp, LGFS2_SB_ADDR(&sbd), TRUE);
+		display_gfs2(bp);
+	} else if (printonly == 1) {
+		print_gfs2("0 (l=0x%x): ", sb_siglen);
+		display_block_type(bp, LGFS2_SB_ADDR(&sbd), TRUE);
+	}
+	bp += sb_siglen;
 	restore_off = bp - restore_buf;
 	restore_left -= restore_off;
 	return 0;
@@ -1414,14 +1435,6 @@ void restoremeta(const char *in_fn, const char *out_device, uint64_t printonly)
 	if (error != 0)
 		exit(error);
 
-	if (smh.sh_fs_bytes > 0) {
-		sbd.fssize = smh.sh_fs_bytes / sbd.bsize;
-		printf("Saved file system size is %"PRIu64" blocks, %.2fGB\n",
-		       sbd.fssize, smh.sh_fs_bytes / ((float)(1 << 30)));
-	}
-
-	printf("This is gfs%c metadata.\n", sbd.gfs1 ? '1': '2');
-
 	if (!printonly) {
 		uint64_t space = lseek(sbd.device_fd, 0, SEEK_END) / sbd.bsize;
 		printf("There are %"PRIu64" free blocks on the destination device.\n", space);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2020-08-06 13:38 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 13:37 [Cluster-devel] [PATCH 00/32] gfs2-utils: savemeta improvements Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 01/32] savemeta: Allow saving to /dev/null Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 02/32] mkfs.gfs2: Fix strncpy warnings Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 03/32] libgfs2: Separate out gfs2l's language API Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 04/32] glocktop: Improve mount info handling Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 05/32] savemeta: Don't save bad xattr blocks twice Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 06/32] libgfs2: Remove gfs2_buffer_head from gfs_dinode_in() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 07/32] libgfs2: Remove gfs2_buffer_head from lgfs2_gfs_inode_get() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 08/32] libgfs2: Remove gfs2_buffer_head from lgfs2_write_journal_data() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 09/32] libgfs2: Move get_file_buf() into structures.c Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 10/32] gfs2l: Remove uses of gfs2_buffer_heads Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 11/32] libgfs2: No need to use gfs2_buffer_head in metapointer() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 12/32] gfs2_edit: Don't use gfs2_buffer_head in do_dinode_extended() args Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 13/32] libgfs2: Add a display name field to struct lgfs2_metadata Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 14/32] gfs2_edit: get_block_type() improvements Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 15/32] gfs2_edit: Don't use gfs2_buffer_head in display_block_type() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 16/32] gfs2_edit: Don't use gfs2_buffer_head in display_gfs2() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 17/32] gfs2_edit: restore_block() improvements Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 18/32] savemeta: Simplify di_save_len() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 19/32] savemeta: Remove gfs2_buffer_head from get_gfs_struct_info() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 20/32] savemeta: Remove gfs2_buffer_head from save_bh() (and rename it) Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 21/32] savemeta: Don't use gfs2_buffer_head in save_leaf_chain() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 22/32] savemeta: Remove gfs2_buffer_head from save_block() Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 23/32] savemeta: Split block reading from saving Andrew Price
2020-08-06 13:37 ` [Cluster-devel] [PATCH 24/32] savemeta: Call get_struct_info() in the read path Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 25/32] savemeta: Introduce multi-block reads Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 26/32] savemeta: Process indirect pointers in chunks Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 27/32] savemeta: Don't trim off trailing zeroes when compressing Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 28/32] savemeta: Leaf block processing improvements Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 29/32] savemeta: Remove some unnecessary reads from save_inode_data() Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 30/32] savemeta: Remove some unnecessary jindex reading code Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 31/32] savemeta: Move block range queue ops into functions Andrew Price
2020-08-06 13:38 ` [Cluster-devel] [PATCH 32/32] restoremeta: Fix up superblock processing Andrew Price

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.