[PATCH v3 0/3] btrfs-progs: provide command to dump checksums

[PATCH v3 0/3] btrfs-progs: provide command to dump checksums

[Johannes Thumshirn]

Provide a command to dump checksums from 'btrfs inspect-internal'. This
command does a lookup for the given file's extents via FIEMAP (which is
handy as it already skips holes) and walks the checksum tree printing all
checksums of an extent.

It is tested against different layouts of files with and without holes,
but further testing done by others is highly appreciated.

It is also available on github @ https://github.com/morbidrsa/btrfs-progs/tree/inspect-csums

As this feature set will be used in upcomming fstests test cases, which will
serve as regression tests around the checksum tree I, I decided that no
standalone tests in btrfs-progs are needed.

Changes since v2:
* Cleanups per Nikolay

Changes since v1:
* Reworked error path's per Sun's comments
* Misc cleanups per Nikolay's comments
* Factored out super block reading from load_and_dump_sb() to be re-usable,
  also per Nikolay

Changes since RFC:
* Complete re-write using ioctl()s (FIEMAP and BTRFS_TREE_SEARCH_V2) to
  gather the extent and checksum informtion, as per David's (private)
  comment.
* Re-named btrfs_lookup_csum() as per Qu's comment.

Johannes Thumshirn (3):
  btrfs-progs: factor out super_block reading from load_and_dump_sb
  btrfs-progs: add 'btrfs inspect-internal csum-dump' command
  btrfs-progs: completion: wire-up dump-csum

 Makefile                  |   3 +-
 btrfs-completion          |   4 +-
 cmds-inspect-dump-csum.c  | 238 ++++++++++++++++++++++++++++++++++++++++++++++
 cmds-inspect-dump-super.c |  15 ++-
 cmds-inspect.c            |   2 +
 commands.h                |   2 +
 utils.c                   |  15 +++
 utils.h                   |   2 +
 8 files changed, 269 insertions(+), 12 deletions(-)
 create mode 100644 cmds-inspect-dump-csum.c

-- 
2.16.4

[PATCH v3 1/3] btrfs-progs: factor out super_block reading from load_and_dump_sb

[Johannes Thumshirn]

inspect-internal dump-superblock's load_and_dump_sb() already reads a
super block from a file descriptor and places it into a 'struct
btrfs_super_block'.

For inspect-internal dump-csum we need this super block as well but don't
care about printing it.

Separate the read from the dump phase so we can re-use it elsewhere.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 cmds-inspect-dump-super.c | 15 ++++++---------
 utils.c                   | 15 +++++++++++++++
 utils.h                   |  2 ++
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index 7815c863f2ed..879f979f526a 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -477,16 +477,13 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
 static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
 		int force)
 {
-	u8 super_block_data[BTRFS_SUPER_INFO_SIZE];
-	struct btrfs_super_block *sb;
+	struct btrfs_super_block sb;
 	u64 ret;
 
-	sb = (struct btrfs_super_block *)super_block_data;
-
-	ret = pread64(fd, super_block_data, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
-	if (ret != BTRFS_SUPER_INFO_SIZE) {
+	ret = load_sb(fd, sb_bytenr, &sb);
+	if (ret) {
 		/* check if the disk if too short for further superblock */
-		if (ret == 0 && errno == 0)
+		if (ret == -ENOSPC)
 			return 0;
 
 		error("failed to read the superblock on %s at %llu",
@@ -496,11 +493,11 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
 	}
 	printf("superblock: bytenr=%llu, device=%s\n", sb_bytenr, filename);
 	printf("---------------------------------------------------------\n");
-	if (btrfs_super_magic(sb) != BTRFS_MAGIC && !force) {
+	if (btrfs_super_magic(&sb) != BTRFS_MAGIC && !force) {
 		error("bad magic on superblock on %s at %llu",
 				filename, (unsigned long long)sb_bytenr);
 	} else {
-		dump_superblock(sb, full);
+		dump_superblock(&sb, full);
 	}
 	return 0;
 }
diff --git a/utils.c b/utils.c
index 9e26c884cc6c..04908b174708 100644
--- a/utils.c
+++ b/utils.c
@@ -2593,3 +2593,18 @@ void print_all_devices(struct list_head *devices)
 		print_device_info(dev, "\t");
 	printf("\n");
 }
+
+int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb)
+{
+	size_t size = sizeof(sizeof(struct btrfs_super_block));
+	int ret;
+
+	ret = pread64(fd, sb, size, bytenr);
+	if (ret != size) {
+		if (ret == 0 && errno == 0)
+			return -EINVAL;
+
+		return -errno;
+	}
+	return 0;
+}
diff --git a/utils.h b/utils.h
index 7c5eb798557d..f0b9ec372893 100644
--- a/utils.h
+++ b/utils.h
@@ -171,6 +171,8 @@ unsigned long total_memory(void);
 void print_device_info(struct btrfs_device *device, char *prefix);
 void print_all_devices(struct list_head *devices);
 
+int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb);
+
 /*
  * Global program state, configurable by command line and available to
  * functions without extra context passing.
-- 
2.16.4

[PATCH v3 2/3] btrfs-progs: add 'btrfs inspect-internal csum-dump' command

[Johannes Thumshirn]

Add a 'btrfs inspect-internal csum-dump' command to dump the on-disk
checksums of a file.

The dump command first uses the FIEMAP ioctl() to get a map of the file's
extents and then uses the BTRFS_TREE_SEARCH_V2 ioctl() to get the
checksums for these extents.

Using FIEMAP instead of the BTRFS_TREE_SEARCH_V2 ioctl() to get the
extents allows us to quickly filter out any holes in the file, as this is
already done for us in the kernel.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 Makefile                 |   3 +-
 cmds-inspect-dump-csum.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++
 cmds-inspect.c           |   2 +
 commands.h               |   2 +
 4 files changed, 244 insertions(+), 1 deletion(-)
 create mode 100644 cmds-inspect-dump-csum.c

diff --git a/Makefile b/Makefile
index e25e256f96af..f5d0c0532faf 100644
--- a/Makefile
+++ b/Makefile
@@ -130,7 +130,8 @@ cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-restore.o cmds-rescue.o chunk-recover.o super-recover.o \
 	       cmds-property.o cmds-fi-usage.o cmds-inspect-dump-tree.o \
 	       cmds-inspect-dump-super.o cmds-inspect-tree-stats.o cmds-fi-du.o \
-	       mkfs/common.o check/mode-common.o check/mode-lowmem.o
+	       cmds-inspect-dump-csum.o mkfs/common.o check/mode-common.o \
+	       check/mode-lowmem.o
 libbtrfs_objects = send-stream.o send-utils.o kernel-lib/rbtree.o btrfs-list.o \
 		   kernel-lib/crc32c.o messages.o \
 		   uuid-tree.o utils-lib.o rbtree-utils.o
diff --git a/cmds-inspect-dump-csum.c b/cmds-inspect-dump-csum.c
new file mode 100644
index 000000000000..02ebd462041e
--- /dev/null
+++ b/cmds-inspect-dump-csum.c
@@ -0,0 +1,238 @@
+/*
+ * Copyright (C) 2019 SUSE. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <linux/fiemap.h>
+#include <linux/fs.h>
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <fcntl.h>
+#include <getopt.h>
+
+#include "kerncompat.h"
+#include "ctree.h"
+#include "messages.h"
+#include "help.h"
+#include "ioctl.h"
+#include "utils.h"
+#include "disk-io.h"
+
+static bool debug = false;
+
+static int btrfs_lookup_csum_for_extent(int fd, struct btrfs_super_block *sb,
+					struct fiemap_extent *fe)
+{
+	struct btrfs_ioctl_search_args_v2 *search;
+	struct btrfs_ioctl_search_key *sk;
+	const int bufsz = 1024;
+	char buf[bufsz], *bp;
+	unsigned int off = 0;
+	const int csum_size = btrfs_super_csum_size(sb);
+	const int sector_size = btrfs_super_sectorsize(sb);
+	int ret, i, j;
+	u64 phys = fe->fe_physical;
+	u64 needle = phys;
+	u64 pending_csum_count = fe->fe_length / sector_size;
+
+	memset(buf, 0, sizeof(buf));
+	search = (struct btrfs_ioctl_search_args_v2 *)buf;
+	sk = &search->key;
+
+again:
+	if (debug)
+		printf(
+"Looking up checksums for extent at physial offset: %llu (searching at %llu), looking for %llu csums\n",
+		       phys, needle, pending_csum_count);
+
+	sk->tree_id = BTRFS_CSUM_TREE_OBJECTID;
+	sk->min_objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	sk->max_objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	sk->max_type = BTRFS_EXTENT_CSUM_KEY;
+	sk->min_type = BTRFS_EXTENT_CSUM_KEY;
+	sk->min_offset = needle;
+	sk->max_offset = (u64)-1;
+	sk->max_transid = (u64)-1;
+	sk->nr_items = 1;
+	search->buf_size = bufsz - sizeof(*search);
+
+	ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH_V2, search);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * If we don't find the csum item at @needle go back by @sector_size and
+	 * retry until we've found it.
+	 */
+	if (sk->nr_items == 0) {
+		needle -= sector_size;
+		goto again;
+	}
+
+
+	bp = (char *) search->buf;
+
+	for (i = 0; i < sk->nr_items; i++) {
+		struct btrfs_ioctl_search_header *sh;
+		u64 csums_in_item;
+
+		sh = (struct btrfs_ioctl_search_header *) (bp + off);
+		off += sizeof(*sh);
+
+		csums_in_item = btrfs_search_header_len(sh) / csum_size;
+		csums_in_item = min(csums_in_item, pending_csum_count);
+
+		for (j = 0; j < csums_in_item; j++) {
+			struct btrfs_csum_item *csum_item;
+
+			csum_item = (struct btrfs_csum_item *)
+						(bp + off + j * csum_size);
+
+			printf("Offset: %llu, checksum: 0x%08x\n",
+			       phys + j * sector_size, *(u32 *)csum_item);
+		}
+
+		off += btrfs_search_header_len(sh);
+		pending_csum_count -= csums_in_item;
+
+	}
+
+	return ret;
+}
+
+static int btrfs_get_extent_csum(int fd, struct btrfs_super_block *sb)
+{
+	struct fiemap *fiemap, *tmp;
+	size_t ext_size;
+	int ret, i;
+
+	fiemap = calloc(1, sizeof(*fiemap));
+	if (!fiemap)
+		return -ENOMEM;
+
+	fiemap->fm_length = ~0;
+
+	ret = ioctl(fd, FS_IOC_FIEMAP, fiemap);
+	if (ret)
+		goto free_fiemap;
+
+	ext_size = fiemap->fm_mapped_extents * sizeof(struct fiemap_extent);
+
+	tmp = realloc(fiemap, sizeof(*fiemap) + ext_size);
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto free_fiemap;
+	}
+
+	fiemap = tmp;
+	fiemap->fm_extent_count = fiemap->fm_mapped_extents;
+	fiemap->fm_mapped_extents = 0;
+
+	ret = ioctl(fd, FS_IOC_FIEMAP, fiemap);
+	if (ret)
+		goto free_fiemap;
+
+	for (i = 0; i < fiemap->fm_mapped_extents; i++) {
+
+		ret = btrfs_lookup_csum_for_extent(fd, sb,
+						   &fiemap->fm_extents[i]);
+		if (ret)
+			break;
+	}
+
+
+free_fiemap:
+	free(fiemap);
+	return ret;
+}
+
+const char * const cmd_inspect_dump_csum_usage[] = {
+	"btrfs inspect-internal dump-csum <path> <device>",
+	"Get Checksums for a given file",
+	"-d|--debug    Be more verbose",
+	NULL
+};
+
+int cmd_inspect_dump_csum(int argc, char **argv)
+{
+	struct btrfs_super_block sb;
+	char *filename;
+	char *device;
+	int fd;
+	int devfd;
+	int ret;
+
+	optind = 0;
+
+	while (1) {
+		static const struct option longopts[] = {
+			{ "debug", no_argument, NULL, 'd' },
+			{ NULL, 0, NULL, 0 }
+		};
+
+		int opt = getopt_long(argc, argv, "d", longopts, NULL);
+		if (opt < 0)
+			break;
+
+		switch (opt) {
+		case 'd':
+			debug = true;
+			break;
+		default:
+			usage(cmd_inspect_dump_csum_usage);
+		}
+	}
+
+	if (check_argc_exact(argc - optind, 2))
+		usage(cmd_inspect_dump_csum_usage);
+
+	filename = argv[optind];
+	device = argv[optind + 1];
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		error("cannot open file %s:%m", filename);
+		return -errno;
+	}
+
+	devfd = open(device, O_RDONLY);
+	if (devfd < 0) {
+		ret = -errno;
+		goto out_close;
+	}
+	load_sb(devfd, btrfs_sb_offset(0), &sb);
+	close(devfd);
+
+	if (btrfs_super_magic(&sb) != BTRFS_MAGIC) {
+		ret = -EINVAL;
+		error("bad magic on superblock on %s", device);
+		goto out_close;
+	}
+
+	ret = btrfs_get_extent_csum(fd, &sb);
+	if (ret)
+		error("checsum lookup for file %s failed", filename);
+out_close:
+	close(fd);
+	return ret;
+}
diff --git a/cmds-inspect.c b/cmds-inspect.c
index efea0331b7aa..c20decbf6fac 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -654,6 +654,8 @@ const struct cmd_group inspect_cmd_group = {
 				cmd_inspect_dump_super_usage, NULL, 0 },
 		{ "tree-stats", cmd_inspect_tree_stats,
 				cmd_inspect_tree_stats_usage, NULL, 0 },
+		{ "dump-csum",  cmd_inspect_dump_csum,
+				cmd_inspect_dump_csum_usage, NULL, 0 },
 		NULL_CMD_STRUCT
 	}
 };
diff --git a/commands.h b/commands.h
index 76991f2b28d5..698ae532b2b8 100644
--- a/commands.h
+++ b/commands.h
@@ -92,6 +92,7 @@ extern const char * const cmd_rescue_usage[];
 extern const char * const cmd_inspect_dump_super_usage[];
 extern const char * const cmd_inspect_dump_tree_usage[];
 extern const char * const cmd_inspect_tree_stats_usage[];
+extern const char * const cmd_inspect_dump_csum_usage[];
 extern const char * const cmd_filesystem_du_usage[];
 extern const char * const cmd_filesystem_usage_usage[];
 
@@ -108,6 +109,7 @@ int cmd_super_recover(int argc, char **argv);
 int cmd_inspect(int argc, char **argv);
 int cmd_inspect_dump_super(int argc, char **argv);
 int cmd_inspect_dump_tree(int argc, char **argv);
+int cmd_inspect_dump_csum(int argc, char **argv);
 int cmd_inspect_tree_stats(int argc, char **argv);
 int cmd_property(int argc, char **argv);
 int cmd_send(int argc, char **argv);
-- 
2.16.4

[PATCH v3 3/3] btrfs-progs: completion: wire-up dump-csum

[Johannes Thumshirn]

Add the new 'dump-csum' command to inspect-internal.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 btrfs-completion | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/btrfs-completion b/btrfs-completion
index 6ae57d1b752b..a381036886f0 100644
--- a/btrfs-completion
+++ b/btrfs-completion
@@ -29,7 +29,7 @@ _btrfs()
 	commands_device='scan add delete remove ready stats usage'
 	commands_scrub='start cancel resume status'
 	commands_rescue='chunk-recover super-recover zero-log'
-	commands_inspect_internal='inode-resolve logical-resolve subvolid-resolve rootid min-dev-size dump-tree dump-super tree-stats'
+	commands_inspect_internal='inode-resolve logical-resolve subvolid-resolve rootid min-dev-size dump-tree dump-super tree-stats dump-csum'
 	commands_property='get set list'
 	commands_quota='enable disable rescan'
 	commands_qgroup='assign remove create destroy show limit'
@@ -128,7 +128,7 @@ _btrfs()
 						_btrfs_mnts
 						return 0
 						;;
-					dump-tree|dump-super|rootid|inode-resolve)
+					dump-tree|dump-super|rootid|inode-resolve|dump-csum)
 						_filedir
 						return 0
 						;;
-- 
2.16.4

[PATCH v3.1 1/3] btrfs-progs: factor out super_block reading from load_and_dump_sb

[Johannes Thumshirn]

inspect-internal dump-superblock's load_and_dump_sb() already reads a
super block from a file descriptor and places it into a 'struct
btrfs_super_block'.

For inspect-internal dump-csum we need this super block as well but don't
care about printing it.

Separate the read from the dump phase so we can re-use it elsewhere.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
- Fix double sizeof() @!$#$ (Nikolay)

 cmds-inspect-dump-super.c | 15 ++++++---------
 utils.c                   | 15 +++++++++++++++
 utils.h                   |  2 ++
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index 7815c863f2ed..879f979f526a 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -477,16 +477,13 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
 static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
 		int force)
 {
-	u8 super_block_data[BTRFS_SUPER_INFO_SIZE];
-	struct btrfs_super_block *sb;
+	struct btrfs_super_block sb;
 	u64 ret;
 
-	sb = (struct btrfs_super_block *)super_block_data;
-
-	ret = pread64(fd, super_block_data, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
-	if (ret != BTRFS_SUPER_INFO_SIZE) {
+	ret = load_sb(fd, sb_bytenr, &sb);
+	if (ret) {
 		/* check if the disk if too short for further superblock */
-		if (ret == 0 && errno == 0)
+		if (ret == -ENOSPC)
 			return 0;
 
 		error("failed to read the superblock on %s at %llu",
@@ -496,11 +493,11 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
 	}
 	printf("superblock: bytenr=%llu, device=%s\n", sb_bytenr, filename);
 	printf("---------------------------------------------------------\n");
-	if (btrfs_super_magic(sb) != BTRFS_MAGIC && !force) {
+	if (btrfs_super_magic(&sb) != BTRFS_MAGIC && !force) {
 		error("bad magic on superblock on %s at %llu",
 				filename, (unsigned long long)sb_bytenr);
 	} else {
-		dump_superblock(sb, full);
+		dump_superblock(&sb, full);
 	}
 	return 0;
 }
diff --git a/utils.c b/utils.c
index 9e26c884cc6c..f0ae53ded315 100644
--- a/utils.c
+++ b/utils.c
@@ -2593,3 +2593,18 @@ void print_all_devices(struct list_head *devices)
 		print_device_info(dev, "\t");
 	printf("\n");
 }
+
+int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb)
+{
+	size_t size = sizeof(struct btrfs_super_block);
+	int ret;
+
+	ret = pread64(fd, sb, size, bytenr);
+	if (ret != size) {
+		if (ret == 0 && errno == 0)
+			return -EINVAL;
+
+		return -errno;
+	}
+	return 0;
+}
diff --git a/utils.h b/utils.h
index 7c5eb798557d..f0b9ec372893 100644
--- a/utils.h
+++ b/utils.h
@@ -171,6 +171,8 @@ unsigned long total_memory(void);
 void print_device_info(struct btrfs_device *device, char *prefix);
 void print_all_devices(struct list_head *devices);
 
+int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb);
+
 /*
  * Global program state, configurable by command line and available to
  * functions without extra context passing.
-- 
2.16.4

Re: [PATCH v3.1 1/3] btrfs-progs: factor out super_block reading from load_and_dump_sb

[Nikolay Borisov]

On 14.05.19 г. 16:58 ч., Johannes Thumshirn wrote:
> inspect-internal dump-superblock's load_and_dump_sb() already reads a
> super block from a file descriptor and places it into a 'struct
> btrfs_super_block'.
> 
> For inspect-internal dump-csum we need this super block as well but don't
> care about printing it.
> 
> Separate the read from the dump phase so we can re-use it elsewhere.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>


> ---
> - Fix double sizeof() @!$#$ (Nikolay)
> 
>  cmds-inspect-dump-super.c | 15 ++++++---------
>  utils.c                   | 15 +++++++++++++++
>  utils.h                   |  2 ++
>  3 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
> index 7815c863f2ed..879f979f526a 100644
> --- a/cmds-inspect-dump-super.c
> +++ b/cmds-inspect-dump-super.c
> @@ -477,16 +477,13 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
>  static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
>  		int force)
>  {
> -	u8 super_block_data[BTRFS_SUPER_INFO_SIZE];
> -	struct btrfs_super_block *sb;
> +	struct btrfs_super_block sb;
>  	u64 ret;
>  
> -	sb = (struct btrfs_super_block *)super_block_data;
> -
> -	ret = pread64(fd, super_block_data, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
> -	if (ret != BTRFS_SUPER_INFO_SIZE) {
> +	ret = load_sb(fd, sb_bytenr, &sb);
> +	if (ret) {
>  		/* check if the disk if too short for further superblock */
> -		if (ret == 0 && errno == 0)
> +		if (ret == -ENOSPC)
>  			return 0;
>  
>  		error("failed to read the superblock on %s at %llu",
> @@ -496,11 +493,11 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
>  	}
>  	printf("superblock: bytenr=%llu, device=%s\n", sb_bytenr, filename);
>  	printf("---------------------------------------------------------\n");
> -	if (btrfs_super_magic(sb) != BTRFS_MAGIC && !force) {
> +	if (btrfs_super_magic(&sb) != BTRFS_MAGIC && !force) {
>  		error("bad magic on superblock on %s at %llu",
>  				filename, (unsigned long long)sb_bytenr);
>  	} else {
> -		dump_superblock(sb, full);
> +		dump_superblock(&sb, full);
>  	}
>  	return 0;
>  }
> diff --git a/utils.c b/utils.c
> index 9e26c884cc6c..f0ae53ded315 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2593,3 +2593,18 @@ void print_all_devices(struct list_head *devices)
>  		print_device_info(dev, "\t");
>  	printf("\n");
>  }
> +
> +int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb)
> +{
> +	size_t size = sizeof(struct btrfs_super_block);
> +	int ret;
> +
> +	ret = pread64(fd, sb, size, bytenr);
> +	if (ret != size) {
> +		if (ret == 0 && errno == 0)
> +			return -EINVAL;
> +
> +		return -errno;
> +	}
> +	return 0;
> +}
> diff --git a/utils.h b/utils.h
> index 7c5eb798557d..f0b9ec372893 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -171,6 +171,8 @@ unsigned long total_memory(void);
>  void print_device_info(struct btrfs_device *device, char *prefix);
>  void print_all_devices(struct list_head *devices);
>  
> +int load_sb(int fd, u64 bytenr, struct btrfs_super_block *sb);
> +
>  /*
>   * Global program state, configurable by command line and available to
>   * functions without extra context passing.
> 

Re: [PATCH v3.1 1/3] btrfs-progs: factor out super_block reading from load_and_dump_sb

[Johannes Thumshirn]

On Tue, May 14, 2019 at 03:58:04PM +0200, Johannes Thumshirn wrote:
> inspect-internal dump-superblock's load_and_dump_sb() already reads a
> super block from a file descriptor and places it into a 'struct
> btrfs_super_block'.
> 
> For inspect-internal dump-csum we need this super block as well but don't
> care about printing it.
> 
> Separate the read from the dump phase so we can re-use it elsewhere.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
> - Fix double sizeof() @!$#$ (Nikolay)
> 
>  cmds-inspect-dump-super.c | 15 ++++++---------
>  utils.c                   | 15 +++++++++++++++
>  utils.h                   |  2 ++
>  3 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
> index 7815c863f2ed..879f979f526a 100644
> --- a/cmds-inspect-dump-super.c
> +++ b/cmds-inspect-dump-super.c
> @@ -477,16 +477,13 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
>  static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full,
>  		int force)
>  {
> -	u8 super_block_data[BTRFS_SUPER_INFO_SIZE];
> -	struct btrfs_super_block *sb;
> +	struct btrfs_super_block sb;
>  	u64 ret;
>  
> -	sb = (struct btrfs_super_block *)super_block_data;
> -
> -	ret = pread64(fd, super_block_data, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
> -	if (ret != BTRFS_SUPER_INFO_SIZE) {
> +	ret = load_sb(fd, sb_bytenr, &sb);
> +	if (ret) {

This is buggy. We need to copy the whole BTRFS_SUPER_INFO_SIZE and calculate
the checksums over this area.

Sorry,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850