thr3ads.net - Btrfs devel - [RFC] Move btrfsck in to the btrfs command [Jan 2013]

If this information is useful, please help other people find it:
Share via:

Ian Kumlien

2013-Jan-29 23:03 UTC

[RFC] Move btrfsck in to the btrfs command

NOTE: in order to apply this patch you should:
	git mv btrfsck.c cmd-fsck.c

This patch moves btrfsck in to "btrfs fsck".

It also adds support for symlinks to the btrfs binary to
retain compablity, =)

I think something should be done to the help description but i''m not
sure what... Anyway, feedback is welcome.

--
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

Ian Kumlien

2013-Jan-29 23:03 UTC

head link

[PATCH] [RFC] include btrfsck in btrfs - including "name check"

This patch includes fsck as a subcommand of btrfs, but if you rename
the binary to btrfsck (or, preferably, use a symlink) it will act like
the old btrfs command.

It will also handle fsck.btrfs which currently is a noop.
---
 Makefile    |  4 ++--
 btrfs.c     | 68 +++++++++++++++++++++++++++++++++++++++++++++----------------
 cmds-fsck.c | 38 +++++++++++++++++++---------------
 commands.h  |  3 +++
 4 files changed, 77 insertions(+), 36 deletions(-)

diff --git a/Makefile b/Makefile
index 4894903..8467530 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
 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-quota.o cmds-qgroup.o cmds-fsck.o
 
 CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
 	    -Wuninitialized -Wshadow -Wundef
@@ -20,7 +20,7 @@ bindir = $(prefix)/bin
 LIBS=-luuid -lm
 RESTORE_LIBS=-lz
 
-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
 
diff --git a/btrfs.c b/btrfs.c
index 687acec..5c1220e 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 },
+		{ "fsck", cmd_fsck, cmd_fsck_usage, NULL, 0 },
 		{ "inspect-internal", cmd_inspect, NULL, &inspect_cmd_group, 0
},
 		{ "send", cmd_send, NULL, &send_cmd_group, 0 },
 		{ "receive", cmd_receive, NULL, &receive_cmd_group, 0 },
@@ -257,24 +266,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_fsck, 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 *func = strrchr(argv[0], ''/'');
+	if (func)
+		argv[0] = ++func;
 
 	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-fsck.c b/cmds-fsck.c
index 67f4a9d..bb5d81f 100644
--- a/cmds-fsck.c
+++ b/cmds-fsck.c
@@ -34,6 +34,7 @@
 #include "list.h"
 #include "version.h"
 #include "utils.h"
+#include "commands.h"
 
 static u64 bytes_used = 0;
 static u64 total_csum_bytes = 0;
@@ -3470,13 +3471,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 },
@@ -3485,7 +3479,18 @@ static struct option long_options[] = {
 	{ 0, 0, 0, 0}
 };
 
-int main(int ac, char **av)
+const char * const cmd_fsck_usage[] = {
+	"btrfs fsck [-s <superblock>] [--repair] [--init-csum-tree]
[--init-extent-tree] <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_fsck(int argc, char **argv)
 {
 	struct cache_tree root_cache;
 	struct btrfs_root *root;
@@ -3501,7 +3506,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt_long(ac, av, "s:", long_options,
+		c = getopt_long(argc, argv, "s:", long_options,
 				&option_index);
 		if (c < 0)
 			break;
@@ -3513,7 +3518,8 @@ int main(int ac, char **av)
 				       (unsigned long long)bytenr);
 				break;
 			case ''?'':
-				print_usage();
+			case ''h'':
+				usage(cmd_fsck_usage);
 		}
 		if (option_index == 1) {
 			printf("enabling repair mode\n");
@@ -3526,23 +3532,23 @@ int main(int ac, char **av)
 		}
 
 	}
-	ac = ac - optind;
+	argc = argc - optind;
 
-	if (ac != 1)
-		print_usage();
+	if (argc != 1)
+		usage(cmd_fsck_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);
 
 	if (info == NULL)
 		return 1;
diff --git a/commands.h b/commands.h
index bb6d2dd..308eb61 100644
--- a/commands.h
+++ b/commands.h
@@ -93,11 +93,14 @@ 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 char * const cmd_fsck_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_fsck(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

--
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

Filipe Brandenburger

2013-Jan-30 20:33 UTC

head link

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Hi Ian,

On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien <pomac@demius.net>
wrote:> This patch includes fsck as a subcommand of btrfs, but if you rename
> the binary to btrfsck (or, preferably, use a symlink) it will act like
> the old btrfs command.
You can rename files in your git (there''s "git mv" for that),
only
thing is when you generate the patch with format-patch (or "git show",
"git diff" etc.) pass it the -M option to detect moves and act
appropriately.

Regarding your patches, I really like the idea of "btrfs fsck" but I
think I''d prefer to keep the external commands as wrapper scripts
instead of adding busybox-style name detection to btrfs... But then,
that''s just my opinion.

I guess I would have a "btrfsck" that would simply contain:

    #! /bin/sh
    exec btrfs fsck "$@"

Downside is that error reporting (e.g. invalid syntax, etc.) would
show "btrfs fsck" instead of the command the user actually typed...

Cheers,
Filipe
--
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

Ian Kumlien

2013-Jan-30 21:11 UTC

head link

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

On Wed, Jan 30, 2013 at 12:33:42PM -0800, Filipe Brandenburger
wrote:> Hi Ian,
> 
> On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien <pomac@demius.net>
wrote:
> > This patch includes fsck as a subcommand of btrfs, but if you rename
> > the binary to btrfsck (or, preferably, use a symlink) it will act like
> > the old btrfs command.
> 
> You can rename files in your git (there''s "git mv" for
that), only
> thing is when you generate the patch with format-patch (or "git
show",
> "git diff" etc.) pass it the -M option to detect moves and act
> appropriately.
git send-email seems to send the full diff, diffing against /dev/null =P
This is why i skipped that part.
> Regarding your patches, I really like the idea of "btrfs fsck"
but I
> think I''d prefer to keep the external commands as wrapper scripts
> instead of adding busybox-style name detection to btrfs... But then,
> that''s just my opinion.
Well, now both works.
> I guess I would have a "btrfsck" that would simply contain:
> 
>     #! /bin/sh
>     exec btrfs fsck "$@"
> 
> Downside is that error reporting (e.g. invalid syntax, etc.) would
> show "btrfs fsck" instead of the command the user actually
typed...
Actually it still does, due to how btrfs handles things... It''s a
simple
enough fix and it will make rescue cd''s or dracut images, or just about
anything.

I understand your point, but i think this is a simpler solution =)
> Cheers,
> Filipe--
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

Ilya Dryomov

2013-Jan-30 21:59 UTC

head link

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

On Wed, Jan 30, 2013 at 10:11:44PM +0100, Ian Kumlien
wrote:> On Wed, Jan 30, 2013 at 12:33:42PM -0800, Filipe Brandenburger wrote:
> > Hi Ian,
> > 
> > On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien <pomac@demius.net>
wrote:
> > > This patch includes fsck as a subcommand of btrfs, but if you
rename
> > > the binary to btrfsck (or, preferably, use a symlink) it will act
like
> > > the old btrfs command.
> > 
> > You can rename files in your git (there''s "git mv"
for that), only
> > thing is when you generate the patch with format-patch (or "git
show",
> > "git diff" etc.) pass it the -M option to detect moves and
act
> > appropriately.
> 
> git send-email seems to send the full diff, diffing against /dev/null =P
> This is why i skipped that part.
> 
> > Regarding your patches, I really like the idea of "btrfs
fsck" but I
> > think I''d prefer to keep the external commands as wrapper
scripts
> > instead of adding busybox-style name detection to btrfs... But then,
> > that''s just my opinion.
> 
> Well, now both works.
> 
> > I guess I would have a "btrfsck" that would simply contain:
> > 
> >     #! /bin/sh
> >     exec btrfs fsck "$@"
> > 
> > Downside is that error reporting (e.g. invalid syntax, etc.) would
> > show "btrfs fsck" instead of the command the user actually
typed...
> 
> Actually it still does, due to how btrfs handles things... It''s a
simple
> enough fix and it will make rescue cd''s or dracut images, or just
about
> anything.
> 
> I understand your point, but i think this is a simpler solution =)
FWIW I agree with Filipe, this name detection thing looks ugly to me.
The merge itself is a good idea, but I think we should stick with shell
wrappers for everything else.

Thanks,

		Ilya
--
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

Ian Kumlien

2013-Jan-30 23:00 UTC

head link

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

On Wed, Jan 30, 2013 at 11:59:05PM +0200, Ilya Dryomov
wrote:> On Wed, Jan 30, 2013 at 10:11:44PM +0100, Ian Kumlien wrote:
> > On Wed, Jan 30, 2013 at 12:33:42PM -0800, Filipe Brandenburger wrote:
> > > Hi Ian,
> > > 
> > > On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien
<pomac@demius.net> wrote:
> > > > This patch includes fsck as a subcommand of btrfs, but if
you rename
> > > > the binary to btrfsck (or, preferably, use a symlink) it
will act like
> > > > the old btrfs command.
> > > 
> > > You can rename files in your git (there''s "git
mv" for that), only
> > > thing is when you generate the patch with format-patch (or
"git show",
> > > "git diff" etc.) pass it the -M option to detect moves
and act
> > > appropriately.
> > 
> > git send-email seems to send the full diff, diffing against /dev/null
=P
> > This is why i skipped that part.
> > 
> > > Regarding your patches, I really like the idea of "btrfs
fsck" but I
> > > think I''d prefer to keep the external commands as
wrapper scripts
> > > instead of adding busybox-style name detection to btrfs... But
then,
> > > that''s just my opinion.
> > 
> > Well, now both works.
> > 
> > > I guess I would have a "btrfsck" that would simply
contain:
> > > 
> > >     #! /bin/sh
> > >     exec btrfs fsck "$@"
> > > 
> > > Downside is that error reporting (e.g. invalid syntax, etc.)
would
> > > show "btrfs fsck" instead of the command the user
actually typed...
> > 
> > Actually it still does, due to how btrfs handles things...
It''s a simple
> > enough fix and it will make rescue cd''s or dracut images, or
just about
> > anything.
> > 
> > I understand your point, but i think this is a simpler solution =)
> 
> FWIW I agree with Filipe, this name detection thing looks ugly to me.
> The merge itself is a good idea, but I think we should stick with shell
> wrappers for everything else.
Which part of it?

        char *func = strrchr(argv[0], ''/'');
        if (func)
                argv[0] = ++func;

Would you prefer i rewrote it as:
	...
	char *func = strrchr(argv[0], ''/'');
	if (func)
		++func;
	else
		func = argv[0]
	...
	if (parse_one_exact_token(func, &function_cmd_group, &cmd) < 0)
---

Would that be better?

The only thing it actually does is remove any path if present....

I have now added:
    btrfs rescue restore [options] <device>
        Restore filesystem
    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

And i''m waiting to rebase my patch series since i need to rewrite the
commit messages if this is wanted changes.
> Thanks,
> 
> 		Ilya--
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

Roman Mamedov

2013-Jan-31 04:10 UTC

head link

Re: [RFC] Move btrfsck in to the btrfs command

On Wed, 30 Jan 2013 00:03:52 +0100
Ian Kumlien <pomac@demius.net> wrote:
> This patch moves btrfsck in to "btrfs fsck".
Does the "...fs fs.." combination look less than ideally beautiful to
anyone
else? That''s "Filesystem" abbreviated two times right there.
Who can use an ATM Machine? Someone who knows a PIN Number.

Also it''s a complete clstrfck of nine consonants, with no vowels.

How about "btrfs check"?

-- 
With respect,
Roman

~~~~~~~~~~~~~~~~~~~~~~~~~~~
"Stallman had a printer,
with code he could not see.
So he began to tinker,
and set the software free."

Chris Murphy

2013-Jan-31 04:16 UTC

head link

Re: [RFC] Move btrfsck in to the btrfs command

On Jan 30, 2013, at 9:10 PM, Roman Mamedov <rm@romanrm.ru>
wrote:> 
> How about "btrfs check"?
For that matter, separate out check and repair. Is there a potential for a
btrfsck repair to make things worse? And if so, could this be determined with a
check?


Chris Murphy--
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

Chris Mason

2013-Jan-31 18:30 UTC

head link

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

On Wed, Jan 30, 2013 at 02:59:05PM -0700, Ilya Dryomov
wrote:> On Wed, Jan 30, 2013 at 10:11:44PM +0100, Ian Kumlien wrote:
> > On Wed, Jan 30, 2013 at 12:33:42PM -0800, Filipe Brandenburger wrote:
> > > Hi Ian,
> > > 
> > > On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien
<pomac@demius.net> wrote:
> > > > This patch includes fsck as a subcommand of btrfs, but if
you rename
> > > > the binary to btrfsck (or, preferably, use a symlink) it
will act like
> > > > the old btrfs command.
> > > 
> > > You can rename files in your git (there''s "git
mv" for that), only
> > > thing is when you generate the patch with format-patch (or
"git show",
> > > "git diff" etc.) pass it the -M option to detect moves
and act
> > > appropriately.
> > 
> > git send-email seems to send the full diff, diffing against /dev/null
=P
> > This is why i skipped that part.
> > 
> > > Regarding your patches, I really like the idea of "btrfs
fsck" but I
> > > think I''d prefer to keep the external commands as
wrapper scripts
> > > instead of adding busybox-style name detection to btrfs... But
then,
> > > that''s just my opinion.
> > 
> > Well, now both works.
> > 
> > > I guess I would have a "btrfsck" that would simply
contain:
> > > 
> > >     #! /bin/sh
> > >     exec btrfs fsck "$@"
> > > 
> > > Downside is that error reporting (e.g. invalid syntax, etc.)
would
> > > show "btrfs fsck" instead of the command the user
actually typed...
> > 
> > Actually it still does, due to how btrfs handles things...
It''s a simple
> > enough fix and it will make rescue cd''s or dracut images, or
just about
> > anything.
> > 
> > I understand your point, but i think this is a simpler solution =)
> 
> FWIW I agree with Filipe, this name detection thing looks ugly to me.
> The merge itself is a good idea, but I think we should stick with shell
> wrappers for everything else.
I like the shell scripts for the things we expect to eventually go away.
But we''re probably stuck with btrfsck and fsck.btrfs forever, so
I''d
prefer these get the name detection treatment.

At the end of the day, I''ll defer to our distro friends on what it
easiest for them to package and include.

-chris

--
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

Maybe Matching Threads

Search for more apparently analagous threads

Btrfs devel - Jan 2013 - [RFC] Move btrfsck in to the btrfs command

[RFC] Move btrfsck in to the btrfs command

[PATCH] [RFC] include btrfsck in btrfs - including "name check"

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Re: [RFC] Move btrfsck in to the btrfs command

Re: [RFC] Move btrfsck in to the btrfs command

Re: [PATCH] [RFC] include btrfsck in btrfs - including "name check"

Maybe Matching Threads