Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in to btrfs

Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in to btrfs

[Ian Kumlien]

Hi,

This patch series moves some of the commands around to reflect that
they are now subcommands of btrfs.

As a stage in this we also add support for btrfs being called as 
btrfsck which yeilds the, now(?), starnard "btrfs check" or 
fsck.btrfs which is a noop to avoid complications with distributions.

We also merge Josef Baciks btrfs-dump-super in patch number 4.

This patchset has been rebased on top of David Sterbas latest WIP changes.

Comments? Ideas? Suggestions?

Please help me with all the help text and verify that it's correct enough ;)

[PATCH 1/6] Btrfs-progs: Rename btrfsck.c -> cmds-check.c

[Ian Kumlien]

In preparation for merging btrfsck functionality in to btrfs.

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 btrfsck.c => cmds-check.c | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename btrfsck.c => cmds-check.c (100%)

diff --git a/btrfsck.c b/cmds-check.c
similarity index 100%
rename from btrfsck.c
rename to cmds-check.c
-- 
1.8.1.2

[PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Ian Kumlien]

This patch includes the functionality of btrfs, it's
found as "btrfs check" however it makes the binary
behave differently depending on what it's run as.

	btrfsck -> will act like normal btrfsck
	fsck.btrfs -> noop

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 Makefile     |  8 ++-----
 btrfs.c      | 68 ++++++++++++++++++++++++++++++++++++++++++++----------------
 cmds-check.c | 40 ++++++++++++++++++++---------------
 commands.h   |  3 +++
 4 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/Makefile b/Makefile
index 2c2a500..94541b2 100644
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,7 @@ objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  send-stream.o send-utils.o qgroup.o raid6.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
-	       cmds-quota.o cmds-qgroup.o cmds-replace.o
+	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o
 
 CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
 	    -Wuninitialized -Wshadow -Wundef
@@ -35,7 +35,7 @@ endif
 
 MAKEOPTS = --no-print-directory Q=$(Q)
 
-progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol btrfsck \
+progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol \
 	btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
 	btrfs-find-root btrfs-restore btrfstune btrfs-show-super \
 	btrfs-dump-super
@@ -111,10 +111,6 @@ btrfs-show: $(objects) btrfs-show.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfs-show btrfs-show.o $(objects) $(LDFLAGS) $(LIBS)
 
-btrfsck: $(objects) btrfsck.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o btrfsck btrfsck.o $(objects) $(LDFLAGS) $(LIBS)
-
 mkfs.btrfs: $(objects) mkfs.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid
diff --git a/btrfs.c b/btrfs.c
index 7b0e50f..cf2f320 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -48,8 +48,13 @@ int prefixcmp(const char *str, const char *prefix)
 			return (unsigned char)*prefix - (unsigned char)*str;
 }
 
-static int parse_one_token(const char *arg, const struct cmd_group *grp,
-			   const struct cmd_struct **cmd_ret)
+#define parse_one_token(arg, grp, cmd_ret) \
+	_parse_one_token((arg), (grp), (cmd_ret), 0)
+#define parse_one_exact_token(arg, grp, cmd_ret) \
+	_parse_one_token((arg), (grp), (cmd_ret), 1)
+
+static int _parse_one_token(const char *arg, const struct cmd_group *grp,
+			   const struct cmd_struct **cmd_ret, int exact)
 {
 	const struct cmd_struct *cmd = grp->commands;
 	const struct cmd_struct *abbrev_cmd = NULL, *ambiguous_cmd = NULL;
@@ -80,12 +85,15 @@ static int parse_one_token(const char *arg, const struct cmd_group *grp,
 		return 0;
 	}
 
-	if (ambiguous_cmd)
-		return -2;
+	if (!exact)
+	{
+		if (ambiguous_cmd)
+			return -2;
 
-	if (abbrev_cmd) {
-		*cmd_ret = abbrev_cmd;
-		return 0;
+		if (abbrev_cmd) {
+			*cmd_ret = abbrev_cmd;
+			return 0;
+		}
 	}
 
 	return -1;
@@ -246,6 +254,7 @@ const struct cmd_group btrfs_cmd_group = {
 		{ "balance", cmd_balance, NULL, &balance_cmd_group, 0 },
 		{ "device", cmd_device, NULL, &device_cmd_group, 0 },
 		{ "scrub", cmd_scrub, NULL, &scrub_cmd_group, 0 },
+		{ "check", cmd_check, cmd_check_usage, NULL, 0 },
 		{ "inspect-internal", cmd_inspect, NULL, &inspect_cmd_group, 0 },
 		{ "send", cmd_send, cmd_send_usage, NULL, 0 },
 		{ "receive", cmd_receive, cmd_receive_usage, NULL, 0 },
@@ -258,24 +267,47 @@ const struct cmd_group btrfs_cmd_group = {
 	},
 };
 
+static int cmd_dummy(int argc, char **argv)
+{
+	return 0;
+}
+
+/* change behaviour depending on what we're called */
+const struct cmd_group function_cmd_group = {
+	NULL, NULL,
+	{
+		{ "btrfsck", cmd_check, NULL, NULL, 0 },
+		{ "fsck.btrfs", cmd_dummy, NULL, NULL, 0 },
+		{ 0, 0, 0, 0, 0 }
+	},
+};
+
 int main(int argc, char **argv)
 {
 	const struct cmd_struct *cmd;
+	char *called_as = strrchr(argv[0], '/');
+	if (called_as)
+		argv[0] = ++called_as;
 
 	crc32c_optimization_init();
 
-	argc--;
-	argv++;
-	handle_options(&argc, &argv);
-	if (argc > 0) {
-		if (!prefixcmp(argv[0], "--"))
-			argv[0] += 2;
-	} else {
-		usage_command_group(&btrfs_cmd_group, 0, 0);
-		exit(1);
-	}
+	/* if we have cmd, we're started as a sub command */
+	if (parse_one_exact_token(argv[0], &function_cmd_group, &cmd) < 0)
+	{
+		argc--;
+		argv++;
 
-	cmd = parse_command_token(argv[0], &btrfs_cmd_group);
+		handle_options(&argc, &argv);
+		if (argc > 0) {
+			if (!prefixcmp(argv[0], "--"))
+				argv[0] += 2;
+		} else {
+			usage_command_group(&btrfs_cmd_group, 0, 0);
+			exit(1);
+		}
+
+		cmd = parse_command_token(argv[0], &btrfs_cmd_group);
+	}
 
 	handle_help_options_next_level(cmd, argc, argv);
 
diff --git a/cmds-check.c b/cmds-check.c
index 71e98de..8e4cce0 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -37,6 +37,7 @@
 #include "list.h"
 #include "version.h"
 #include "utils.h"
+#include "commands.h"
 
 static u64 bytes_used = 0;
 static u64 total_csum_bytes = 0;
@@ -3529,13 +3530,6 @@ static int check_extents(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static void print_usage(void)
-{
-	fprintf(stderr, "usage: btrfsck dev\n");
-	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
-	exit(1);
-}
-
 static struct option long_options[] = {
 	{ "super", 1, NULL, 's' },
 	{ "repair", 0, NULL, 0 },
@@ -3544,7 +3538,18 @@ static struct option long_options[] = {
 	{ 0, 0, 0, 0}
 };
 
-int main(int ac, char **av)
+const char * const cmd_check_usage[] = {
+	"btrfs check [options] <device>",
+	"check a btrfs filesystem",
+	"",
+	"-s|--super <superblock>     use this superblock copy",
+	"--repair                    try to repair the filesystem",
+	"--init-csum-tree            create a new CRC tree",
+	"--init-extent-tree          create a new extent tree",
+	NULL
+};
+
+int cmd_check(int argc, char **argv)
 {
 	struct cache_tree root_cache;
 	struct btrfs_root *root;
@@ -3561,7 +3566,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt_long(ac, av, "as:", long_options,
+		c = getopt_long(argc, argv, "as:", long_options,
 				&option_index);
 		if (c < 0)
 			break;
@@ -3574,7 +3579,8 @@ int main(int ac, char **av)
 				       (unsigned long long)bytenr);
 				break;
 			case '?':
-				print_usage();
+			case 'h':
+				usage(cmd_check_usage);
 		}
 		if (option_index == 1) {
 			printf("enabling repair mode\n");
@@ -3587,25 +3593,25 @@ int main(int ac, char **av)
 		}
 
 	}
-	ac = ac - optind;
+	argc = argc - optind;
 
-	if (ac != 1)
-		print_usage();
+	if (argc != 1)
+		usage(cmd_check_usage);
 
 	radix_tree_init();
 	cache_tree_init(&root_cache);
 
-	if((ret = check_mounted(av[optind])) < 0) {
+	if((ret = check_mounted(argv[optind])) < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n", strerror(-ret));
 		return ret;
 	} else if(ret) {
-		fprintf(stderr, "%s is currently mounted. Aborting.\n", av[optind]);
+		fprintf(stderr, "%s is currently mounted. Aborting.\n", argv[optind]);
 		return -EBUSY;
 	}
 
-	info = open_ctree_fs_info(av[optind], bytenr, rw, 1);
+	info = open_ctree_fs_info(argv[optind], bytenr, rw, 1);
 	uuid_unparse(info->super_copy.fsid, uuidbuf);
-	printf("Checking filesystem on %s\nUUID: %s\n", av[optind], uuidbuf);
+	printf("Checking filesystem on %s\nUUID: %s\n", argv[optind], uuidbuf);
 
 	if (info == NULL)
 		return 1;
diff --git a/commands.h b/commands.h
index 33eb99a..ddb636f 100644
--- a/commands.h
+++ b/commands.h
@@ -94,11 +94,14 @@ extern const struct cmd_group replace_cmd_group;
 extern const char * const cmd_send_usage[];
 extern const char * const cmd_receive_usage[];
 
+extern const char * const cmd_check_usage[];
+
 int cmd_subvolume(int argc, char **argv);
 int cmd_filesystem(int argc, char **argv);
 int cmd_balance(int argc, char **argv);
 int cmd_device(int argc, char **argv);
 int cmd_scrub(int argc, char **argv);
+int cmd_check(int argc, char **argv);
 int cmd_inspect(int argc, char **argv);
 int cmd_send(int argc, char **argv);
 int cmd_receive(int argc, char **argv);
-- 
1.8.1.2

[PATCH 3/6] Btrfs-progs: move btrfs-select-super.c

[Ian Kumlien]

This patch moves
	btrfs-select-super.c -> cmds-rescue-super-ops.c

This is done in preparation to merge the select-super
functionality to btrfs.

The naming is because we will also merge
btrfs-dump-super.c from Josef Bacik

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 btrfs-select-super.c => cmds-rescue-super-ops.c | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename btrfs-select-super.c => cmds-rescue-super-ops.c (100%)

diff --git a/btrfs-select-super.c b/cmds-rescue-super-ops.c
similarity index 100%
rename from btrfs-select-super.c
rename to cmds-rescue-super-ops.c
-- 
1.8.1.2

[PATCH 4/6] Btrfs-progs: move debug-tree.c -> cmds-rescue-debug-tree.c

[Ian Kumlien]

The btrfs-debug-tree functionality will be integrated
in to btrfs as "btrfs rescue debug-tree"

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 debug-tree.c => cmds-rescue-debug-tree.c | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename debug-tree.c => cmds-rescue-debug-tree.c (100%)

diff --git a/debug-tree.c b/cmds-rescue-debug-tree.c
similarity index 100%
rename from debug-tree.c
rename to cmds-rescue-debug-tree.c
-- 
1.8.1.2

[PATCH 5/6] Btrfs-progs: restore.c -> cmds-restore.c

[Ian Kumlien]

The btrfs-restore functionality will be integrated in
btrs as "btrfs restore"

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 restore.c => cmds-restore.c | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename restore.c => cmds-restore.c (100%)

diff --git a/restore.c b/cmds-restore.c
similarity index 100%
rename from restore.c
rename to cmds-restore.c
-- 
1.8.1.2

[PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[Ian Kumlien]

the btrfs command now lists:
    btrfs rescue select-super -s <number> <device>
        Select a superblock
    btrfs rescue dump-super <device>
        Dump a superblock to disk
    btrfs rescue debug-tree [options] <device>
        Debug the filesystem

btrfs-dump-super.c was imported in to cmds-rescue-super-ops.c

This patch integrates all the functionality...

cmds-rescue.c is used to glue cmds-rescue-debug-tree.c,
cmds-rescue-restore.c and cmds-rescue-super-ops.c together to
make the source files more managable.

Signed-off-by: Ian Kumlien <pomac@demius.net>
---
 Makefile                 |  34 +++++-----------
 btrfs-dump-super.c       |  87 ---------------------------------------
 btrfs.c                  |   2 +
 cmds-rescue-debug-tree.c |  44 ++++++++++----------
 cmds-rescue-super-ops.c  | 103 +++++++++++++++++++++++++++++++++++++----------
 cmds-rescue.c            |  48 ++++++++++++++++++++++
 cmds-restore.c           |  42 +++++++++++--------
 commands.h               |   8 +++-
 8 files changed, 195 insertions(+), 173 deletions(-)
 delete mode 100644 btrfs-dump-super.c
 create mode 100644 cmds-rescue.c

diff --git a/Makefile b/Makefile
index 94541b2..7e2db76 100644
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,9 @@ objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  send-stream.o send-utils.o qgroup.o raid6.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
-	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o
+	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o \
+	       cmds-rescue.o cmds-rescue-super-ops.o \
+	       cmds-rescue-debug-tree.o cmds-restore.o
 
 CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
 	    -Wuninitialized -Wshadow -Wundef
@@ -17,8 +19,7 @@ DEPFLAGS = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
-LIBS=-luuid -lm
-RESTORE_LIBS=-lz -llzo2
+LIBS=-luuid -lm -lz -llzo2
 
 ifeq ("$(origin V)", "command line")
   BUILD_VERBOSE = $(V)
@@ -35,10 +36,9 @@ endif
 
 MAKEOPTS = --no-print-directory Q=$(Q)
 
-progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol \
-	btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
-	btrfs-find-root btrfs-restore btrfstune btrfs-show-super \
-	btrfs-dump-super
+progs = btrfsctl mkfs.btrfs btrfs-show btrfs-vol btrfs \
+	btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
+	btrfs-find-root btrfstune \
 
 # Create all the static targets
 static_objects = $(patsubst %.o, %.static.o, $(objects))
@@ -95,10 +95,6 @@ btrfs-find-root: $(objects) find-root.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfs-find-root find-root.o $(objects) $(LDFLAGS) $(LIBS)
 
-btrfs-restore: $(objects) restore.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o btrfs-restore restore.o $(objects) $(LDFLAGS) $(LIBS) $(RESTORE_LIBS)
-
 btrfsctl: $(objects) btrfsctl.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfsctl btrfsctl.o $(objects) $(LDFLAGS) $(LIBS)
@@ -115,10 +111,6 @@ mkfs.btrfs: $(objects) mkfs.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid
 
-btrfs-debug-tree: $(objects) debug-tree.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o btrfs-debug-tree $(objects) debug-tree.o $(LDFLAGS) $(LIBS)
-
 btrfs-zero-log: $(objects) btrfs-zero-log.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfs-zero-log $(objects) btrfs-zero-log.o $(LDFLAGS) $(LIBS)
@@ -127,14 +119,6 @@ btrfs-show-super: $(objects) btrfs-show-super.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfs-show-super $(objects) btrfs-show-super.o $(LDFLAGS) $(LIBS)
 
-btrfs-select-super: $(objects) btrfs-select-super.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o btrfs-select-super $(objects) btrfs-select-super.o $(LDFLAGS) $(LIBS)
-
-btrfs-dump-super: $(objects) btrfs-dump-super.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o btrfs-dump-super $(objects) btrfs-dump-super.o $(LDFLAGS) $(LIBS)
-
 btrfstune: $(objects) btrfstune.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o btrfstune $(objects) btrfstune.o $(LDFLAGS) $(LIBS)
@@ -175,8 +159,8 @@ install-man:
 
 clean :
 	@echo "Cleaning"
-	$(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \
-	      btrfs-zero-log btrfstune btrfs-dump-super dir-test ioctl-test quick-test \
+	$(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image \
+	      btrfs-zero-log btrfstune dir-test ioctl-test quick-test \
 	      version.h
 	$(Q)$(MAKE) $(MAKEOPTS) -C man $@
 
diff --git a/btrfs-dump-super.c b/btrfs-dump-super.c
deleted file mode 100644
index 033140c..0000000
--- a/btrfs-dump-super.c
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * Copyright (C) 2011 Google.  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.
- */
-
-#define _XOPEN_SOURCE 500
-#define _GNU_SOURCE 1
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/stat.h>
-#include "kerncompat.h"
-#include "ctree.h"
-#include "disk-io.h"
-#include "version.h"
-
-static void print_usage(void)
-{
-	fprintf(stderr, "usage: btrfs-dump-super dev\n");
-	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
-	exit(1);
-}
-
-static int read_block(const char* filename,  u64 bytenr, struct btrfs_super_block* sb) {
-	int fd = open(filename, O_RDONLY, 0600);
-	int block_size = sizeof(struct btrfs_super_block);
-	int bytes_read = 0;
-
-	if (fd < 0) {
-		fprintf(stderr, "Could not open %s\n", filename);
-		return -1;
-	}
-
-	bytes_read = pread(fd, sb, block_size, bytenr);
-	if (bytes_read < block_size) {
-		fprintf(stderr, "Only read %d bytes of %d.\n", bytes_read, block_size);
-	}
-
-	close(fd);
-	return bytes_read;
-}
-
-int main(int ac, char **av)
-{
-	int i;
-
-	if (ac != 2)
-		print_usage();
-
-	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
-		u64 bytenr = btrfs_sb_offset(i);
-		int fd;
-		struct btrfs_super_block sb;
-		int block_size = sizeof(struct btrfs_super_block);
-		char filename[1024];
-		int bytes_read = read_block(av[optind], bytenr, &sb);
-		if (bytes_read < block_size)
-			continue;
-
-		sprintf(filename, "/tmp/block.%s.%llu",
-			strrchr(av[optind], '/') + 1, bytenr);
-		fd = open(filename, O_CREAT|O_WRONLY, 0644);
-		if (block_size != pwrite(fd, &sb, block_size, 0)) {
-			fprintf(stderr, "Failed to dump superblock %d", i);
-			continue;
-		}
-		fprintf(stderr, "Dumped superblock %s:%d, gen %llu to %s.\n",
-			av[optind], i, sb.generation, filename);
-		close(fd);
-	}
-
-	return 0;
-}
diff --git a/btrfs.c b/btrfs.c
index cf2f320..459108d 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -261,6 +261,8 @@ const struct cmd_group btrfs_cmd_group = {
 		{ "quota", cmd_quota, NULL, &quota_cmd_group, 0 },
 		{ "qgroup", cmd_qgroup, NULL, &qgroup_cmd_group, 0 },
 		{ "replace", cmd_replace, NULL, &replace_cmd_group, 0 },
+		{ "rescue", cmd_rescue, NULL, &rescue_cmd_group, 0 },
+		{ "restore", cmd_restore, cmd_restore_usage, NULL, 0 },
 		{ "help", cmd_help, cmd_help_usage, NULL, 0 },
 		{ "version", cmd_version, cmd_version_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }
diff --git a/cmds-rescue-debug-tree.c b/cmds-rescue-debug-tree.c
index f6bd5d8..24703cb 100644
--- a/cmds-rescue-debug-tree.c
+++ b/cmds-rescue-debug-tree.c
@@ -27,21 +27,19 @@
 #include "print-tree.h"
 #include "transaction.h"
 #include "version.h"
+#include "commands.h"
 
-static int print_usage(void)
-{
-	fprintf(stderr, "usage: btrfs-debug-tree [ -e ] [ -d ] [ -r ] [ -R ]\n");
-	fprintf(stderr, "                        [-b block_num ] device\n");
-	fprintf(stderr, "\t-e : print detailed extents info\n");
-	fprintf(stderr, "\t-d : print info of btrfs device and root tree dirs"
-                    " only\n");
-	fprintf(stderr, "\t-r : print info of roots only\n");
-	fprintf(stderr, "\t-R : print info of roots and root backups\n");
-	fprintf(stderr, "\t-b block_num : print info of the specified block"
-                    " only\n");
-	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
-	exit(1);
-}
+const char * const cmd_debug_tree_usage[] = {
+	"btrfs rescue debug-tree [options] <device>",
+	"Debug the filesystem",
+	"",
+	"-e                print detailed extents info",
+	"-d                print info about btrfs device and root tree dirs",
+	"-r                print info about roots only",
+	"-R                print info about roots and root backups",
+	"-b <block_num>    print info about the specified block only",
+	NULL
+};
 
 static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
 {
@@ -109,7 +107,7 @@ static void print_old_roots(struct btrfs_super_block *super)
 	}
 }
 
-int main(int ac, char **av)
+int cmd_debug_tree(int argc, char **argv)
 {
 	struct btrfs_root *root;
 	struct btrfs_fs_info *info;
@@ -133,7 +131,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt(ac, av, "deb:rR");
+		c = getopt(argc, argv, "deb:rR");
 		if (c < 0)
 			break;
 		switch(c) {
@@ -154,23 +152,23 @@ int main(int ac, char **av)
 				block_only = atoll(optarg);
 				break;
 			default:
-				print_usage();
+				usage(cmd_debug_tree_usage);
 		}
 	}
-	ac = ac - optind;
-	if (ac != 1)
-		print_usage();
+	argc = argc - optind;
+	if (argc != 1)
+		usage(cmd_debug_tree_usage);
 
-	info = open_ctree_fs_info(av[optind], 0, 0, 1);
+	info = open_ctree_fs_info(argv[optind], 0, 0, 1);
 	if (!info) {
-		fprintf(stderr, "unable to open %s\n", av[optind]);
+		fprintf(stderr, "unable to open %s\n", argv[optind]);
 		exit(1);
 	}
 	root = info->fs_root;
 
 	if (block_only) {
 		if (!root) {
-			fprintf(stderr, "unable to open %s\n", av[optind]);
+			fprintf(stderr, "unable to open %s\n", argv[optind]);
 			exit(1);
 		}
 		leaf = read_tree_block(root,
diff --git a/cmds-rescue-super-ops.c b/cmds-rescue-super-ops.c
index 5eb4e6e..a5e25f6 100644
--- a/cmds-rescue-super-ops.c
+++ b/cmds-rescue-super-ops.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2007 Oracle.  All rights reserved.
+ * Copyright (C) 2011 Google.  All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public
@@ -31,17 +32,25 @@
 #include "list.h"
 #include "version.h"
 #include "utils.h"
+#include "commands.h"
 
-static void print_usage(void)
-{
-	fprintf(stderr, "usage: btrfs-select-super [-c] [-e] -s number dev\n");
-	fprintf(stderr, "       -c Commit changes to disk [IRREVERSIBLE]\n");
-	fprintf(stderr, "       -e Use the earliest super found, may help recover transid verify problems\n");
-	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
-	exit(1);
-}
+const char * const cmd_select_super_usage[] = {
+	"btrfs rescue select-super [options] -s <number> <device>",
+	"Select a superblock",
+	"",
+	"-s <offset>     specify superblock",
+	"-c              commit changes to disk [IRREVERSIBLE]",
+	"-e              Use the earliest super found, may help recover transid verify problems",
+	NULL
+};
 
-int main(int ac, char **av)
+const char * const cmd_dump_super_usage[] = {
+	"btrfs rescue dump-super <device>",
+	"Dump a superblock to disk",
+	NULL
+};
+
+int cmd_select_super(int argc, char **argv)
 {
 	struct btrfs_root *root;
 	int ret;
@@ -53,7 +62,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt(ac, av, "s:ce");
+		c = getopt(argc, argv, "s:ce");
 		if (c < 0)
 			break;
 		switch(c) {
@@ -70,35 +79,35 @@ int main(int ac, char **av)
 				use_earliest_bdev = 1;
 				break;
 			default:
-				print_usage();
+				usage(cmd_select_super_usage);
 		}
 	}
-	ac = ac - optind;
+	argc = argc - optind;
 
-	if (ac != 1)
-		print_usage();
+	if (argc != 1)
+		usage(cmd_select_super_usage);
 
 	if (bytenr == 0) {
-		fprintf(stderr, "Please select the super copy with -s\n");
-		print_usage();
+		fprintf(stderr, "Please select the superblock copy with -s\n");
+		usage(cmd_select_super_usage);
 	}
 
 	radix_tree_init();
 
-	if ((ret = check_mounted(av[optind])) < 0) {
+	if ((ret = check_mounted(argv[optind])) < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n", strerror(-ret));
 		return ret;
 	} else if (ret) {
-		fprintf(stderr, "%s is currently mounted. Aborting.\n", av[optind]);
+		fprintf(stderr, "%s is currently mounted. Aborting.\n", argv[optind]);
 		return -EBUSY;
 	}
 
-	fp = open(av[optind], O_CREAT|O_RDWR, 0600);
+	fp = open(argv[optind], O_CREAT|O_RDWR, 0600);
 	if (fp < 0) {
-		fprintf(stderr, "Could not open %s\n", av[optind]);
+		fprintf(stderr, "Could not open %s\n", argv[optind]);
 		return 1;
 	}
-	root = open_ctree_fd(fp, av[optind], bytenr, 1, 0);
+	root = open_ctree_fd(fp, argv[optind], bytenr, 1, 0);
 
 	if (!root) {
 		fprintf(stderr, "Open ctree failed\n");
@@ -121,3 +130,55 @@ int main(int ac, char **av)
 	 */ 
 	return ret;
 }
+
+static int read_block(const char* filename,  u64 bytenr, struct btrfs_super_block* sb) {
+        int fd = open(filename, O_RDONLY, 0600);
+        int block_size = sizeof(struct btrfs_super_block);
+        int bytes_read = 0;
+
+        if (fd < 0) {
+                fprintf(stderr, "Could not open %s\n", filename);
+                return -1;
+        }
+
+        bytes_read = pread(fd, sb, block_size, bytenr);
+        if (bytes_read < block_size) {
+                fprintf(stderr, "Only read %d bytes of %d.\n", bytes_read, block_size);
+        }
+
+        close(fd);
+        return bytes_read;
+}
+
+int cmd_dump_super(int argc, char **argv)
+{
+        int i;
+
+        if (argc != 2)
+                usage(cmd_dump_super_usage);
+
+        for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+                u64 bytenr = btrfs_sb_offset(i);
+                int fd;
+                struct btrfs_super_block sb;
+                int block_size = sizeof(struct btrfs_super_block);
+                char filename[1024];
+                int bytes_read = read_block(argv[optind], bytenr, &sb);
+                if (bytes_read < block_size)
+                        continue;
+
+                sprintf(filename, "/tmp/block.%s.%llu",
+                        strrchr(argv[optind], '/') + 1, bytenr);
+                fd = open(filename, O_CREAT|O_WRONLY, 0644);
+                if (block_size != pwrite(fd, &sb, block_size, 0)) {
+                        fprintf(stderr, "Failed to dump superblock %d", i);
+                        continue;
+                }
+                fprintf(stderr, "Dumped superblock %s:%d, gen %llu to %s.\n",
+                        argv[optind], i, sb.generation, filename);
+                close(fd);
+        }
+
+        return 0;
+}
+
diff --git a/cmds-rescue.c b/cmds-rescue.c
new file mode 100644
index 0000000..d3e0bd4
--- /dev/null
+++ b/cmds-rescue.c
@@ -0,0 +1,48 @@
+/*
+ * 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 <stdio.h>
+#include "commands.h"
+
+static const char * const rescue_cmd_group_usage[] = {
+	"btrfs rescue <command> [<args>]",
+	NULL
+};
+
+extern const char * const cmd_restore_usage[];
+extern const char * const cmd_select_super_usage[];
+extern const char * const cmd_dump_super_usage[];
+extern const char * const cmd_debug_tree_usage[];
+
+int cmd_restore(int argc, char **argv);
+int cmd_select_super(int argc, char **argv);
+int cmd_dump_super(int argc, char **argv);
+int cmd_debug_tree(int argc, char **argv);
+
+const struct cmd_group rescue_cmd_group = {
+        rescue_cmd_group_usage, NULL, {
+		{ "select-super", cmd_select_super, cmd_select_super_usage, NULL, 0 },
+		{ "dump-super", cmd_dump_super, cmd_dump_super_usage, NULL, 0 },
+		{ "debug-tree", cmd_debug_tree, cmd_debug_tree_usage, NULL, 0 },
+		{ 0, 0, 0, 0, 0 },
+	}
+};
+
+int cmd_rescue(int argc, char **argv)
+{
+	return handle_command_group(&rescue_cmd_group, argc, argv);
+}
+
diff --git a/cmds-restore.c b/cmds-restore.c
index 0fe81b3..103a352 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -41,6 +41,7 @@
 #include "version.h"
 #include "volumes.h"
 #include "utils.h"
+#include "commands.h"
 
 static char fs_name[4096];
 static char path_name[4096];
@@ -775,12 +776,6 @@ next:
 	return 0;
 }
 
-static void usage()
-{
-	fprintf(stderr, "Usage: restore [-sviocl] [-t disk offset] "
-		"[-m regex] <device> <directory>\n");
-}
-
 static int do_list_roots(struct btrfs_root *root)
 {
 	struct btrfs_key key;
@@ -1004,7 +999,26 @@ out:
 	return ret;
 }
 
-int main(int argc, char **argv)
+const char * const cmd_restore_usage[] = {
+	"btrfs rescue restore [options] <device>",
+	"Restore filesystem",
+	"",
+	"-s              get snapshots",
+	"-v              verbose",
+	"-i              ignore errors",
+	"-o              overwrite",
+	"-t              tree location",
+	"-f <offset>     filesystem location",
+	"-u <block>      super mirror",
+	"-d              find dir",
+	"-r <num>        root objectid",
+	"-c              ignore case in regular expression",
+	"-m <regexp>     regular expression to match",
+	"-l              list roots",
+	NULL
+};
+
+int cmd_restore(int argc, char **argv)
 {
 	struct btrfs_root *root;
 	struct btrfs_key key;
@@ -1084,18 +1098,14 @@ int main(int argc, char **argv)
 				list_roots = 1;
 				break;
 			default:
-				usage();
-				exit(1);
+				usage(cmd_restore_usage);
 		}
 	}
 
-	if (!list_roots && optind + 1 >= argc) {
-		usage();
-		exit(1);
-	} else if (list_roots && optind >= argc) {
-		usage();
-		exit(1);
-	}
+	if (!list_roots && optind + 1 >= argc)
+		usage(cmd_restore_usage);
+	else if (list_roots && optind >= argc)
+		usage(cmd_restore_usage);
 
 	if ((ret = check_mounted(argv[optind])) < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n",
diff --git a/commands.h b/commands.h
index ddb636f..06b78d1 100644
--- a/commands.h
+++ b/commands.h
@@ -90,11 +90,12 @@ extern const struct cmd_group receive_cmd_group;
 extern const struct cmd_group quota_cmd_group;
 extern const struct cmd_group qgroup_cmd_group;
 extern const struct cmd_group replace_cmd_group;
+extern const struct cmd_group rescue_cmd_group;
 
 extern const char * const cmd_send_usage[];
 extern const char * const cmd_receive_usage[];
-
 extern const char * const cmd_check_usage[];
+extern const char * const cmd_restore_usage[];
 
 int cmd_subvolume(int argc, char **argv);
 int cmd_filesystem(int argc, char **argv);
@@ -108,6 +109,11 @@ int cmd_receive(int argc, char **argv);
 int cmd_quota(int argc, char **argv);
 int cmd_qgroup(int argc, char **argv);
 int cmd_replace(int argc, char **argv);
+int cmd_restore(int argc, char **argv);
+int cmd_select_super(int argc, char **argv);
+int cmd_dump_super(int argc, char **argv);
+int cmd_debug_tree(int argc, char **argv);
+int cmd_rescue(int argc, char **argv);
 
 /* subvolume exported functions */
 int test_issubvolume(char *path);
-- 
1.8.1.2

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[Goffredo Baroncelli]

On 02/08/2013 01:37 AM, Ian Kumlien wrote:
> the btrfs command now lists:
>     btrfs rescue select-super -s <number> <device>
>         Select a superblock
>     btrfs rescue dump-super <device>
>         Dump a superblock to disk
>     btrfs rescue debug-tree [options] <device>
>         Debug the filesystem
> 
> btrfs-dump-super.c was imported in to cmds-rescue-super-ops.c
> 
> This patch integrates all the functionality...
> 
> cmds-rescue.c is used to glue cmds-rescue-debug-tree.c,
> cmds-rescue-restore.c and cmds-rescue-super-ops.c together to
> make the source files more managable.
[...]
> -int main(int ac, char **av)
> +const char * const cmd_dump_super_usage[] = {
> +	"btrfs rescue dump-super <device>",
> +	"Dump a superblock to disk",
> +	NULL
> +};
[...]
> +int cmd_dump_super(int argc, char **argv)
> +{
> +        int i;
> +
> +        if (argc != 2)
> +                usage(cmd_dump_super_usage);
> +
> +        for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
> +                u64 bytenr = btrfs_sb_offset(i);
> +                int fd;
> +                struct btrfs_super_block sb;
> +                int block_size = sizeof(struct btrfs_super_block);
> +                char filename[1024];
> +                int bytes_read = read_block(argv[optind], bytenr, &sb);
> +                if (bytes_read < block_size)
> +                        continue;
I think that we should also implement a check of the superblock checksum.

> +
> +                sprintf(filename, "/tmp/block.%s.%llu",
> +                        strrchr(argv[optind], '/') + 1, bytenr);
> +                fd = open(filename, O_CREAT|O_WRONLY, 0644);

This is the part that I don't like. The output file name should be
passed via the command line. It should not be defined by some obscure
logic. The user is able to understand where the sb is written only after.

> +                if (block_size != pwrite(fd, &sb, block_size, 0)) {
> +                        fprintf(stderr, "Failed to dump superblock %d", i);
> +                        continue;
> +                }
> +                fprintf(stderr, "Dumped superblock %s:%d, gen %llu to %s.\n",
> +                        argv[optind], i, sb.generation, filename);
> +                close(fd);
> +        }
[...]

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Goffredo Baroncelli]

H Iam,

On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> This patch includes the functionality of btrfs, it's
> found as "btrfs check" however it makes the binary
> behave differently depending on what it's run as.
[...]

>  
> +static int cmd_dummy(int argc, char **argv)
> +{
> +	return 0;

I think we should warn that fsck.btrfs does nothing. Something like:
 +	fprintf(stderr, "WARNING: fsck.btrfs does nothing. "
		"Try 'btrfs check'\n");



> +}
> +
> +/* change behaviour depending on what we're called */
> +const struct cmd_group function_cmd_group = {
> +	NULL, NULL,
> +	{
> +		{ "btrfsck", cmd_check, NULL, NULL, 0 },
> +		{ "fsck.btrfs", cmd_dummy, NULL, NULL, 0 },
> +		{ 0, 0, 0, 0, 0 }
> +	},
> +};
> +
>  int main(int argc, char **argv)
>  {
>  	const struct cmd_struct *cmd;
> +	char *called_as = strrchr(argv[0], '/');
> +	if (called_as)
> +		argv[0] = ++called_as;
>  
>  	crc32c_optimization_init();
>  
> -	argc--;
> -	argv++;
> -	handle_options(&argc, &argv);
> -	if (argc > 0) {
> -		if (!prefixcmp(argv[0], "--"))
> -			argv[0] += 2;
> -	} else {
> -		usage_command_group(&btrfs_cmd_group, 0, 0);
> -		exit(1);
> -	}
> +	/* if we have cmd, we're started as a sub command */
> +	if (parse_one_exact_token(argv[0], &function_cmd_group, &cmd) < 0)
> +	{
> +		argc--;
> +		argv++;
>  
> -	cmd = parse_command_token(argv[0], &btrfs_cmd_group);
> +		handle_options(&argc, &argv);
> +		if (argc > 0) {
> +			if (!prefixcmp(argv[0], "--"))
> +				argv[0] += 2;

I can't understand the reason to skip '--' ? But this question is not
related to your patch...

> +		} else {
> +			usage_command_group(&btrfs_cmd_group, 0, 0);
[...]

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in to btrfs

[Goffredo Baroncelli]

On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> Hi,
> 
> This patch series moves some of the commands around to reflect that
> they are now subcommands of btrfs.
> 
> As a stage in this we also add support for btrfs being called as 
> btrfsck which yeilds the, now(?), starnard "btrfs check" or 
> fsck.btrfs which is a noop to avoid complications with distributions.
> 
> We also merge Josef Baciks btrfs-dump-super in patch number 4.
> 
> This patchset has been rebased on top of David Sterbas latest WIP changes.
> 
> Comments? Ideas? Suggestions?

Se my other two email for the specific comment. However I can ask you to
update the man page with these new commands. Any way I really appreciate
the improved help for these commands.
> 
> Please help me with all the help text and verify that it's correct enough ;)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in to btrfs

[Ian Kumlien]

On Fri, Feb 08, 2013 at 06:40:34PM +0100, Goffredo Baroncelli wrote:
> On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> > Hi,
> > 
> > This patch series moves some of the commands around to reflect that
> > they are now subcommands of btrfs.
> > 
> > As a stage in this we also add support for btrfs being called as 
> > btrfsck which yeilds the, now(?), starnard "btrfs check" or 
> > fsck.btrfs which is a noop to avoid complications with distributions.
> > 
> > We also merge Josef Baciks btrfs-dump-super in patch number 4.
> > 
> > This patchset has been rebased on top of David Sterbas latest WIP changes.
> > 
> > Comments? Ideas? Suggestions?
> 
> Se my other two email for the specific comment. However I can ask you to
> update the man page with these new commands. Any way I really appreciate
> the improved help for these commands.

I'll most definetly look in to it - =)

> > 
> > Please help me with all the help text and verify that it's correct enough ;)
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[Ian Kumlien]

On Fri, Feb 08, 2013 at 06:39:12PM +0100, Goffredo Baroncelli wrote:
> On 02/08/2013 01:37 AM, Ian Kumlien wrote:
> > the btrfs command now lists:
> >     btrfs rescue select-super -s <number> <device>
> >         Select a superblock
> >     btrfs rescue dump-super <device>
> >         Dump a superblock to disk
> >     btrfs rescue debug-tree [options] <device>
> >         Debug the filesystem
> > 
> > btrfs-dump-super.c was imported in to cmds-rescue-super-ops.c
> > 
> > This patch integrates all the functionality...
> > 
> > cmds-rescue.c is used to glue cmds-rescue-debug-tree.c,
> > cmds-rescue-restore.c and cmds-rescue-super-ops.c together to
> > make the source files more managable.
> [...]
> > -int main(int ac, char **av)
> > +const char * const cmd_dump_super_usage[] = {
> > +	"btrfs rescue dump-super <device>",
> > +	"Dump a superblock to disk",
> > +	NULL
> > +};
> [...]
> > +int cmd_dump_super(int argc, char **argv)
> > +{
> > +        int i;
> > +
> > +        if (argc != 2)
> > +                usage(cmd_dump_super_usage);
> > +
> > +        for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
> > +                u64 bytenr = btrfs_sb_offset(i);
> > +                int fd;
> > +                struct btrfs_super_block sb;
> > +                int block_size = sizeof(struct btrfs_super_block);
> > +                char filename[1024];
> > +                int bytes_read = read_block(argv[optind], bytenr, &sb);
> > +                if (bytes_read < block_size)
> > +                        continue;
> I think that we should also implement a check of the superblock checksum.
> 
> > +
> > +                sprintf(filename, "/tmp/block.%s.%llu",
> > +                        strrchr(argv[optind], '/') + 1, bytenr);
> > +                fd = open(filename, O_CREAT|O_WRONLY, 0644);
> 
> This is the part that I don't like. The output file name should be
> passed via the command line. It should not be defined by some obscure
> logic. The user is able to understand where the sb is written only after.

This is only the import of the command, we can add anything we want
after that.

Josef also said that it could use some better usability - right now it's
only there since the functionality is good to have when <something> hits
the fan =)

> > +                if (block_size != pwrite(fd, &sb, block_size, 0)) {
> > +                        fprintf(stderr, "Failed to dump superblock %d", i);
> > +                        continue;
> > +                }
> > +                fprintf(stderr, "Dumped superblock %s:%d, gen %llu to %s.\n",
> > +                        argv[optind], i, sb.generation, filename);
> > +                close(fd);
> > +        }
> [...]
> 
> -- 
> gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Ian Kumlien]

On Fri, Feb 08, 2013 at 06:39:18PM +0100, Goffredo Baroncelli wrote:
> H Iam,
> 
> On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> > This patch includes the functionality of btrfs, it's
> > found as "btrfs check" however it makes the binary
> > behave differently depending on what it's run as.
> [...]
> 
> >  
> > +static int cmd_dummy(int argc, char **argv)
> > +{
> > +	return 0;
> 
> I think we should warn that fsck.btrfs does nothing. Something like:
>  +	fprintf(stderr, "WARNING: fsck.btrfs does nothing. "
> 		"Try 'btrfs check'\n");

Yes, will do, perhaps not a big warning but atleast alert the user to
the fact.

> > +}
> > +
> > +/* change behaviour depending on what we're called */
> > +const struct cmd_group function_cmd_group = {
> > +	NULL, NULL,
> > +	{
> > +		{ "btrfsck", cmd_check, NULL, NULL, 0 },
> > +		{ "fsck.btrfs", cmd_dummy, NULL, NULL, 0 },
> > +		{ 0, 0, 0, 0, 0 }
> > +	},
> > +};
> > +
> >  int main(int argc, char **argv)
> >  {
> >  	const struct cmd_struct *cmd;
> > +	char *called_as = strrchr(argv[0], '/');
> > +	if (called_as)
> > +		argv[0] = ++called_as;
> >  
> >  	crc32c_optimization_init();
> >  
> > -	argc--;
> > -	argv++;
> > -	handle_options(&argc, &argv);
> > -	if (argc > 0) {
> > -		if (!prefixcmp(argv[0], "--"))
> > -			argv[0] += 2;
> > -	} else {
> > -		usage_command_group(&btrfs_cmd_group, 0, 0);
> > -		exit(1);
> > -	}
> > +	/* if we have cmd, we're started as a sub command */
> > +	if (parse_one_exact_token(argv[0], &function_cmd_group, &cmd) < 0)
> > +	{
> > +		argc--;
> > +		argv++;
> >  
> > -	cmd = parse_command_token(argv[0], &btrfs_cmd_group);
> > +		handle_options(&argc, &argv);
> > +		if (argc > 0) {
> > +			if (!prefixcmp(argv[0], "--"))
> > +				argv[0] += 2;
> 
> I can't understand the reason to skip '--' ? But this question is not
> related to your patch...

it handles, it seems, btrfs --filesystem or so, if users are unsure of
usage, more like mdadm in that respect

> > +		} else {
> > +			usage_command_group(&btrfs_cmd_group, 0, 0);
> [...]
> 
> -- 
> gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
> Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[Ilya Dryomov]

On Fri, Feb 08, 2013 at 01:37:02AM +0100, Ian Kumlien wrote:
> the btrfs command now lists:
>     btrfs rescue select-super -s <number> <device>
>         Select a superblock
>     btrfs rescue dump-super <device>
>         Dump a superblock to disk
>     btrfs rescue debug-tree [options] <device>
>         Debug the filesystem
> 
> btrfs-dump-super.c was imported in to cmds-rescue-super-ops.c
> 
> This patch integrates all the functionality...
> 
> cmds-rescue.c is used to glue cmds-rescue-debug-tree.c,
> cmds-rescue-restore.c and cmds-rescue-super-ops.c together to
> make the source files more managable.

I think btrfs-debug-tree should go under debug -- btrfs debug dump-tree.
Same goes for btrfs-dump-super -- btrfs debug dump-super.  These
commands won't help an average user to rescue a single byte, they are
there to help developers.  I suppose you can also import btrfs-image
under btrfs debug dump-image.

This leaves the question of whether we want select-super under rescue.
Given that it can easily do more harm than good, it might not be the
best place for it..

Thanks,

		Ilya

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]

On Fri, Feb 08, 2013 at 09:57:17PM +0200, Ilya Dryomov wrote:
> On Fri, Feb 08, 2013 at 01:37:02AM +0100, Ian Kumlien wrote:
> > the btrfs command now lists:
> >     btrfs rescue select-super -s <number> <device>
> >         Select a superblock
> >     btrfs rescue dump-super <device>
> >         Dump a superblock to disk
> >     btrfs rescue debug-tree [options] <device>
> >         Debug the filesystem
> > 
> > btrfs-dump-super.c was imported in to cmds-rescue-super-ops.c
> > 
> > This patch integrates all the functionality...
> > 
> > cmds-rescue.c is used to glue cmds-rescue-debug-tree.c,
> > cmds-rescue-restore.c and cmds-rescue-super-ops.c together to
> > make the source files more managable.
> 
> I think btrfs-debug-tree should go under debug -- btrfs debug dump-tree.
> Same goes for btrfs-dump-super -- btrfs debug dump-super.  These
> commands won't help an average user to rescue a single byte, they are
> there to help developers.  I suppose you can also import btrfs-image
> under btrfs debug dump-image.

   Can I put in a suggestion of using inspect-internal instead? Since
we've already got that one already? (But I'm not bothered about debug;
either is good).

   Hugo.

> This leaves the question of whether we want select-super under rescue.
> Given that it can easily do more harm than good, it might not be the
> best place for it..
> 
> Thanks,
> 
> 		Ilya

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
       --- The early bird gets the worm,  but the second mouse ---       
                            gets the cheese.                             

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[David Sterba]

On Fri, Feb 08, 2013 at 01:37:02AM +0100, Ian Kumlien wrote:
> the btrfs command now lists:
>     btrfs rescue select-super -s <number> <device>
>         Select a superblock
>     btrfs rescue dump-super <device>
>         Dump a superblock to disk
>     btrfs rescue debug-tree [options] <device>
>         Debug the filesystem

Let me summarize here the comments I've read so far and what we disussed
with Ilya on irc:

* btrfs check -- seems to be accepted
* btrfs restore -- seems to be accepted

* btrfs rescue
  * select-super -- it is a command that can make
    things bad and should be used with caution, thus not very suitable
    to put under 'rescue' command, let's keep it as a standalone utitly
    for now

  * show-super +
  * dump-super -- they do basically the same thing, the output is
    different, Ilya suggests to keep only 'dump-super' with 2 output
    modes, and I agree with him

  * debug-tree -- suggested name is 'dump-tree'

Then, the namespace 'rescue' does not fit the purpose, these are
debugging commands, so the proposed name is 'debug'.

They do not seem to fit into 'inspect-internal' which is supposed to
work on a mounted filesystem, and  I think this should contain commands
for infrequent but useful things.

Let's focus on the above commands for start, we can add more later (eg.
the repair subcommand).


thanks,
david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[David Sterba]

On Fri, Feb 08, 2013 at 07:17:13PM +0100, Ian Kumlien wrote:
> On Fri, Feb 08, 2013 at 06:39:18PM +0100, Goffredo Baroncelli wrote:
> > H Iam,
> > 
> > On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> > > This patch includes the functionality of btrfs, it's
> > > found as "btrfs check" however it makes the binary
> > > behave differently depending on what it's run as.
> > [...]
> > 
> > >  
> > > +static int cmd_dummy(int argc, char **argv)
> > > +{
> > > +	return 0;
> > 
> > I think we should warn that fsck.btrfs does nothing. Something like:
> >  +	fprintf(stderr, "WARNING: fsck.btrfs does nothing. "
> > 		"Try 'btrfs check'\n");
> 
> Yes, will do, perhaps not a big warning but atleast alert the user to
> the fact.

I'm not yet decided if I like the no-op functionality merged or if
fsck.btrfs should be a script like fsck.xfs
(http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfsprogs.git;a=blob;f=fsck/xfs_fsck.sh;h=c5a96e688b994c36d9ab1b0206225f2f5e7b12e8;hb=HEAD)

Your version of cmd_dummy is too simple, the mentioned fsck.xfs at least
checks if the device exists, handles the automatic check options and
prints a sensible message what to do if the user runs the utility
expecting it to actually do something.

I think that a binary named 'btrfsck' should be equvalent to 'btrfs
check' for backward compatibility, so I'd take this patch without the
fsck.btrfs bits. Ok?

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Ian Kumlien]

On Sat, Feb 09, 2013 at 12:07:50AM +0100, David Sterba wrote:
> On Fri, Feb 08, 2013 at 07:17:13PM +0100, Ian Kumlien wrote:
> > On Fri, Feb 08, 2013 at 06:39:18PM +0100, Goffredo Baroncelli wrote:
> > > H Iam,
> > > 
> > > On 02/08/2013 01:36 AM, Ian Kumlien wrote:
> > > > This patch includes the functionality of btrfs, it's
> > > > found as "btrfs check" however it makes the binary
> > > > behave differently depending on what it's run as.
> > > [...]
> > > 
> > > >  
> > > > +static int cmd_dummy(int argc, char **argv)
> > > > +{
> > > > +	return 0;
> > > 
> > > I think we should warn that fsck.btrfs does nothing. Something like:
> > >  +	fprintf(stderr, "WARNING: fsck.btrfs does nothing. "
> > > 		"Try 'btrfs check'\n");
> > 
> > Yes, will do, perhaps not a big warning but atleast alert the user to
> > the fact.
> 
> I'm not yet decided if I like the no-op functionality merged or if
> fsck.btrfs should be a script like fsck.xfs
> (http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfsprogs.git;a=blob;f=fsck/xfs_fsck.sh;h=c5a96e688b994c36d9ab1b0206225f2f5e7b12e8;hb=HEAD)
> 
> Your version of cmd_dummy is too simple, the mentioned fsck.xfs at least
> checks if the device exists, handles the automatic check options and
> prints a sensible message what to do if the user runs the utility
> expecting it to actually do something.
> 
> I think that a binary named 'btrfsck' should be equvalent to 'btrfs
> check' for backward compatibility, so I'd take this patch without the
> fsck.btrfs bits. Ok?

Thats fine by me, =)

I didn't know that fsck.xfs did that and i agree that it's a much saner
approach, should we do something similar already or just put it in tha
backlock for when it might be... or, when it would actually be usefull?

> david

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

Re: [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs

[David Sterba]

On Fri, Feb 08, 2013 at 01:37:02AM +0100, Ian Kumlien wrote:
> the btrfs command now lists:
>     btrfs rescue select-super -s <number> <device>
>         Select a superblock
>     btrfs rescue dump-super <device>
>         Dump a superblock to disk
>     btrfs rescue debug-tree [options] <device>
>         Debug the filesystem

The only candidate into rescue section I currently see is 'zero-log'.

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[David Sterba]

On Fri, Feb 08, 2013 at 01:36:58AM +0100, Ian Kumlien wrote:
> -btrfsck: $(objects) btrfsck.o
> -	@echo "    [LD]     $@"
> -	$(Q)$(CC) $(CFLAGS) -o btrfsck btrfsck.o $(objects) $(LDFLAGS) $(LIBS)
> -

Do we want to get rid of the btrfsck binary completely? Using the term
'btrsfck' is common and anybody compiling sources from git needs to cp
or ln btrfs -> btrfsck. Let's make it automatic, I'll add a makefile
rule:

--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,7 @@ endif

 MAKEOPTS = --no-print-directory Q=$(Q)

-progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol \
+progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol btrfsck \
        btrfs btrfs-map-logical btrfs-image btrfs-zero-log btrfs-convert \
        btrfs-find-root btrfstune btrfs-show-super

@@ -125,6 +125,11 @@ btrfs-show: $(objects) $(libs) btrfs-show.o
        @echo "    [LD]     $@"
        $(Q)$(CC) $(CFLAGS) -o btrfs-show btrfs-show.o $(objects) $(LDFLAGS) $(LIBS)

+# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
+btrfsck: btrfs
+       @echo "    [CP]     $@"
+       $(Q)cp btrfs btrfsck
+
 mkfs.btrfs: $(objects) $(libs) mkfs.o
        @echo "    [LD]     $@"
        $(Q)$(CC) $(CFLAGS) -o mkfs.btrfs $(objects) mkfs.o $(LDFLAGS) $(LIBS) -lblkid
@@ -186,7 +191,7 @@ install-man:
 clean :
        @echo "Cleaning"
        $(Q)rm -f $(progs) cscope.out *.o .*.d btrfs-convert btrfs-image btrfs-select-super \
-             btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static \
+             btrfs-zero-log btrfstune dir-test ioctl-test quick-test btrfs.static btrfsck \
              version.h \
              $(libs) libbtrfs.so libbtrfs.so.0 libbtrfs.so.0.1
        $(Q)$(MAKE) $(MAKEOPTS) -C man $@
---

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Filipe Brandenburger]

Hi,

On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote:
> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
> +btrfsck: btrfs
> +       @echo "    [CP]     $@"
> +       $(Q)cp btrfs btrfsck
> +

I think the idea was that btrfsck becomes a link (either symbolic or
hardlink works) to btrfs...

Maybe just replace cp with ln?

Cheers,
Filipe

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Goffredo Baroncelli]

On 02/12/2013 06:37 PM, Filipe Brandenburger wrote:
> Hi,
> 
> On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote:
>> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
>> +btrfsck: btrfs
>> +       @echo "    [CP]     $@"
>> +       $(Q)cp btrfs btrfsck
>> +
> 
> I think the idea was that btrfsck becomes a link (either symbolic or
> hardlink works) to btrfs...
> 
> Maybe just replace cp with ln?

I agree with Filipe, or even a script is reasonable. So we have only one
binary to update, and we avoid the risk to have a version mismatch
between btrfsck and btrfs. This could lead to a different behaviour
when the user call btrfsck instead btrfs. Finally this could save some
bytes of space.

Anyway my opinion would be to left this kind to decision to the
distribution. We (as upstream) should only remove the old btrfsck and
issue an WARNING/REMARK in the release note to notify this change.
Unfortunately btrfsck is old; now we must provide an alternative file to
overwrite this binary in order to avoid the mismatch above when the user
is used to recompile the binary from the source.

BR
Goffredo
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[David Sterba]

On Tue, Feb 12, 2013 at 07:01:33PM +0100, Goffredo Baroncelli wrote:
> On 02/12/2013 06:37 PM, Filipe Brandenburger wrote:
> > Hi,
> > 
> > On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote:
> >> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
> >> +btrfsck: btrfs
> >> +       @echo "    [CP]     $@"
> >> +       $(Q)cp btrfs btrfsck
> >> +
> > 
> > I think the idea was that btrfsck becomes a link (either symbolic or
> > hardlink works) to btrfs...
> > 
> > Maybe just replace cp with ln?
> 
> I agree with Filipe, or even a script is reasonable. So we have only one
> binary to update, and we avoid the risk to have a version mismatch
> between btrfsck and btrfs. This could lead to a different behaviour
> when the user call btrfsck instead btrfs. Finally this could save some
> bytes of space.

Ok, I'll replace it with a hardlink. A symlink is not reliable (cannot
be copied without breaking the path).

> Anyway my opinion would be to left this kind to decision to the
> distribution. We (as upstream) should only remove the old btrfsck and
> issue an WARNING/REMARK in the release note to notify this change.
> Unfortunately btrfsck is old; now we must provide an alternative file to
> overwrite this binary in order to avoid the mismatch above when the user
> is used to recompile the binary from the source.

A warning is a good idea and it will start a deprecation period of
btrfsck as separate utility. Unil some point in future, I'd rather stay
conservative and let it exist.

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Goffredo Baroncelli]

On 02/12/2013 11:52 PM, David Sterba wrote:
> On Tue, Feb 12, 2013 at 07:01:33PM +0100, Goffredo Baroncelli wrote:
>> On 02/12/2013 06:37 PM, Filipe Brandenburger wrote:
>>> Hi,
>>>
>>> On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote:
>>>> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
>>>> +btrfsck: btrfs
>>>> +       @echo "    [CP]     $@"
>>>> +       $(Q)cp btrfs btrfsck
>>>> +
>>>
>>> I think the idea was that btrfsck becomes a link (either symbolic or
>>> hardlink works) to btrfs...
>>>
>>> Maybe just replace cp with ln?
>>
>> I agree with Filipe, or even a script is reasonable. So we have only one
>> binary to update, and we avoid the risk to have a version mismatch
>> between btrfsck and btrfs. This could lead to a different behaviour
>> when the user call btrfsck instead btrfs. Finally this could save some
>> bytes of space.
> 
> Ok, I'll replace it with a hardlink. A symlink is not reliable (cannot
> be copied without breaking the path).

...mmm... the install command (invoked by the Makefile during the
installation phase) doesn't seem to preserve both the hard-link and the
soft-link:

$ touch test
$ ln test test2
$ ln -sf test lntest
$ mkdir t3
$ install test2 t3/
$ install lntest t3/
$ ls -li lntest test test2 t3/test2 t3/lntest
3005857 lrwxrwxrwx 1 ghigo ghigo 4 Feb 13 00:03 lntest -> test
3005858 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:03 t3/lntest
3005854 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:00 t3/test2
3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test
3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test2

I think that a bash script is a better choice.

>> Anyway my opinion would be to left this kind to decision to the
>> distribution. We (as upstream) should only remove the old btrfsck and
>> issue an WARNING/REMARK in the release note to notify this change.
>> Unfortunately btrfsck is old; now we must provide an alternative file to
>> overwrite this binary in order to avoid the mismatch above when the user
>> is used to recompile the binary from the source.
> 
> A warning is a good idea and it will start a deprecation period of
> btrfsck as separate utility. Unil some point in future, I'd rather stay
> conservative and let it exist.
> 
> david
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Dieter Ries]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

Hi everybody,

Am 08.02.2013 01:36, schrieb Ian Kumlien:
> diff --git a/cmds-check.c b/cmds-check.c
> index 71e98de..8e4cce0 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c

[...]

> @@ -3574,7 +3579,8 @@ int main(int ac, char **av)
>  				       (unsigned long long)bytenr);
>  				break;
>  			case '?':
> -				print_usage();
> +			case 'h':
> +				usage(cmd_check_usage);
>  		}
>  		if (option_index == 1) {
>  			printf("enabling repair mode\n");

For this to have any effect, 'h' must be added to getopt_long(), see
attached patch 1.

However, this results in btrfsck -h and --help doing different things:

--help prints the usage message to stdout and exits with exit(0).
-h prints the usage message to stderr and exits with exit(129).

I made a patch to fix this, see attached patch 2.
What it doesn't fix though is, that -h/--help and -? don't do the same
thing. This is more complicated, as getop_long returns '?' for unknown
options.

Cheers,

Dieter

[-- Attachment #2: 0001-Btrfs-progs-Fix-btrfsck-btrfs-check-h.patch --]
[-- Type: text/x-patch, Size: 758 bytes --]

>From 11aabdb018aed3c5b6a1616178883fd879152856 Mon Sep 17 00:00:00 2001
From: Dieter Ries <mail@dieterries.net>
Date: Sun, 2 Jun 2013 17:30:09 +0200
Subject: [PATCH 1/2] Btrfs-progs: Fix 'btrfsck/btrfs check -h'

For the '-h' option to be usable, getopts_long() has to know it.

Signed-off-by: Dieter Ries <mail@dieterries.net>
---
 cmds-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index 1e5e005..ff9298d 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4065,7 +4065,7 @@ int cmd_check(int argc, char **argv)
 
 	while(1) {
 		int c;
-		c = getopt_long(argc, argv, "as:", long_options,
+		c = getopt_long(argc, argv, "ahs:", long_options,
 				&option_index);
 		if (c < 0)
 			break;
-- 
1.8.1.3


[-- Attachment #3: 0002-Btrfs-progs-Fix-help-to-h-inconsistency-in-btrfsck-b.patch --]
[-- Type: text/x-patch, Size: 2207 bytes --]

>From 52d9e47bfa0936a14baa48e8ad6ecdd820295809 Mon Sep 17 00:00:00 2001
From: Dieter Ries <mail@dieterries.net>
Date: Sun, 2 Jun 2013 17:32:15 +0200
Subject: [PATCH 2/2] Btrfs-progs: Fix '--help' to '-h' inconsistency in
 btrfsck/btrfs check

This patch fixes the following inconsistency between calling
btrfsck/btrfs check with the -h or --help options:
--help prints the usage message to stdout and exits with exit(0).
-h prints the usage message to stderr and exits with exit(129).

To achieve this, usage_command_usagestr() is made avalilable via
commands.h.

Signed-off-by: Dieter Ries <mail@dieterries.net>
---
 cmds-check.c | 5 ++++-
 commands.h   | 2 ++
 help.c       | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index ff9298d..093c859 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4078,8 +4078,11 @@ int cmd_check(int argc, char **argv)
 				       (unsigned long long)bytenr);
 				break;
 			case '?':
-			case 'h':
 				usage(cmd_check_usage);
+				break;
+			case 'h':
+				usage_command_usagestr(cmd_check_usage, "check", 1, 0);
+				exit(0);
 		}
 		if (option_index == 1) {
 			printf("enabling repair mode\n");
diff --git a/commands.h b/commands.h
index 15c616d..814452f 100644
--- a/commands.h
+++ b/commands.h
@@ -73,6 +73,8 @@ extern const char * const generic_cmd_help_usage[];
 void usage(const char * const *usagestr);
 void usage_command(const struct cmd_struct *cmd, int full, int err);
 void usage_command_group(const struct cmd_group *grp, int all, int err);
+void usage_command_usagestr(const char * const *usagestr,
+				const char *token, int full, int err);
 
 void help_unknown_token(const char *arg, const struct cmd_group *grp);
 void help_ambiguous_token(const char *arg, const struct cmd_group *grp);
diff --git a/help.c b/help.c
index 6d04293..effb72e 100644
--- a/help.c
+++ b/help.c
@@ -102,7 +102,7 @@ static int usage_command_internal(const char * const *usagestr,
 	return ret;
 }
 
-static void usage_command_usagestr(const char * const *usagestr,
+void usage_command_usagestr(const char * const *usagestr,
 				   const char *token, int full, int err)
 {
 	FILE *outf = err ? stderr : stdout;
-- 
1.8.1.3

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[David Sterba]

Hi,

On Sun, Jun 02, 2013 at 05:47:38PM +0200, Dieter Ries wrote:
> For this to have any effect, 'h' must be added to getopt_long(), see
> attached patch 1.
> 
> However, this results in btrfsck -h and --help doing different things:
> 
> --help prints the usage message to stdout and exits with exit(0).
> -h prints the usage message to stderr and exits with exit(129).
> 
> I made a patch to fix this, see attached patch 2.
> What it doesn't fix though is, that -h/--help and -? don't do the same
> thing. This is more complicated, as getop_long returns '?' for unknown
> options.

FYI, both patchess added to integration.

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Ilya Dryomov]

On Wed, Nov 13, 2013 at 7:13 PM, David Sterba <dsterba@suse.cz> wrote:
> Hi,
>
> On Sun, Jun 02, 2013 at 05:47:38PM +0200, Dieter Ries wrote:
>> For this to have any effect, 'h' must be added to getopt_long(), see
>> attached patch 1.
>>
>> However, this results in btrfsck -h and --help doing different things:
>>
>> --help prints the usage message to stdout and exits with exit(0).
>> -h prints the usage message to stderr and exits with exit(129).
>>
>> I made a patch to fix this, see attached patch 2.
>> What it doesn't fix though is, that -h/--help and -? don't do the same
>> thing. This is more complicated, as getop_long returns '?' for unknown
>> options.
>
> FYI, both patchess added to integration.

Hi David,

FWIW, I think none of the btrfs sub-commands treat -h as a help option.
(This is an artifact that was inherited from the the old btrfs-progs
utility.)  -h vs --help is actually consistent: -h results in a "btrfs
check: invalid option -- 'h'" message, and therefore exits with 129.
Since 'btrfs check -h' has clearly never worked we might want to keep
the status quo.

Thanks,

                Ilya

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[David Sterba]

On Thu, Nov 14, 2013 at 11:25:55AM +0200, Ilya Dryomov wrote:
> On Wed, Nov 13, 2013 at 7:13 PM, David Sterba <dsterba@suse.cz> wrote:
> >> For this to have any effect, 'h' must be added to getopt_long(), see
> >> attached patch 1.
> >>
> >> However, this results in btrfsck -h and --help doing different things:
> >>
> >> --help prints the usage message to stdout and exits with exit(0).
> >> -h prints the usage message to stderr and exits with exit(129).
> >>
> >> I made a patch to fix this, see attached patch 2.
> >> What it doesn't fix though is, that -h/--help and -? don't do the same
> >> thing. This is more complicated, as getop_long returns '?' for unknown
> >> options.
> >
> > FYI, both patchess added to integration.
> 
> FWIW, I think none of the btrfs sub-commands treat -h as a help option.
> (This is an artifact that was inherited from the the old btrfs-progs
> utility.)  -h vs --help is actually consistent: -h results in a "btrfs
> check: invalid option -- 'h'" message, and therefore exits with 129.
> Since 'btrfs check -h' has clearly never worked we might want to keep
> the status quo.

Good point, I'll drop the patches.

david

Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs

[Ian Kumlien]

On Thu, Nov 14, 2013 at 01:49:21PM +0100, David Sterba wrote:
> On Thu, Nov 14, 2013 at 11:25:55AM +0200, Ilya Dryomov wrote:
> > On Wed, Nov 13, 2013 at 7:13 PM, David Sterba <dsterba@suse.cz> wrote:
> > >> For this to have any effect, 'h' must be added to getopt_long(), see
> > >> attached patch 1.
> > >>
> > >> However, this results in btrfsck -h and --help doing different things:
> > >>
> > >> --help prints the usage message to stdout and exits with exit(0).
> > >> -h prints the usage message to stderr and exits with exit(129).
> > >>
> > >> I made a patch to fix this, see attached patch 2.
> > >> What it doesn't fix though is, that -h/--help and -? don't do the same
> > >> thing. This is more complicated, as getop_long returns '?' for unknown
> > >> options.
> > >
> > > FYI, both patchess added to integration.
> > 
> > FWIW, I think none of the btrfs sub-commands treat -h as a help option.
> > (This is an artifact that was inherited from the the old btrfs-progs
> > utility.)  -h vs --help is actually consistent: -h results in a "btrfs
> > check: invalid option -- 'h'" message, and therefore exits with 129.
> > Since 'btrfs check -h' has clearly never worked we might want to keep
> > the status quo.
> 
> Good point, I'll drop the patches.

This should be really easy to add, i don't know if i have the time
currently thuogh... 

> david