Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs
On Wed, Nov 13, 2013 at 7:13 PM, David Sterba dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
On Thu, Nov 14, 2013 at 11:25:55AM +0200, Ilya Dryomov wrote: On Wed, Nov 13, 2013 at 7:13 PM, David Sterba dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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
From 11aabdb018aed3c5b6a1616178883fd879152856 Mon Sep 17 00:00:00 2001
From: Dieter Ries m...@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 m...@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
From 52d9e47bfa0936a14baa48e8ad6ecdd820295809 Mon Sep 17 00:00:00 2001
From: Dieter Ries m...@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 m...@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
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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
Hi, On Tue, Feb 12, 2013 at 8:39 AM, David Sterba dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
On 02/12/2013 06:37 PM, Filipe Brandenburger wrote: Hi, On Tue, Feb 12, 2013 at 8:39 AM, David Sterba dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 dste...@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 -- 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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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
--
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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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
--
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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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
--
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 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs
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 po...@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
