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 po...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] include btrfsck in btrfs - including name check
Hi Ian, On Tue, Jan 29, 2013 at 3:03 PM, Ian Kumlien po...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 po...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 po...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 po...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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,
