[PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Dan McGrath]

Hi,

I thought that I would attempt a quick little patch that will make btrfsck into
a No-op when called as fsck.btrfsck.

The reasoning is that the FAQ states that it is recommended and safe to do so,
and the current 12.04 version of Ubuntu just symlinks fsck.btrfsck to btrfsck
instead of /bin/true.

PS - Apologies if I mess this git send-email up!



Dan McGrath (1):
  btrfs-progs: No-op when called as fsck.btrfsck

 btrfs.c |    2 ++
 1 file changed, 2 insertions(+)

-- 
1.7.9.5

[PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Dan McGrath]

As per FAQ: It is safe to and recommended to turn fsck.btrfs into a no-op

Signed-off-by: Dan McGrath <danmcgrath.ca@gmail.com>
---
 btrfs.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/btrfs.c b/btrfs.c
index 691adef..78161a9 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -272,6 +272,8 @@ int main(int argc, char **argv)
 
 	if (!strcmp(bname, "btrfsck")) {
 		argv[0] = "check";
+	} else if (!strcmp(bname, "fsck.btrfsck")) {
+		exit(0);
 	} else {
 		argc--;
 		argv++;
-- 
1.7.9.5

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Jan Alexander Steffens]

On Mon, Apr 15, 2013 at 3:47 PM, Dan McGrath <danmcgrath.ca@gmail.com> wrote:
> As per FAQ: It is safe to and recommended to turn fsck.btrfs into a no-op
>
> Signed-off-by: Dan McGrath <danmcgrath.ca@gmail.com>
> ---
>  btrfs.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/btrfs.c b/btrfs.c
> index 691adef..78161a9 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -272,6 +272,8 @@ int main(int argc, char **argv)
>
>         if (!strcmp(bname, "btrfsck")) {
>                 argv[0] = "check";
> +       } else if (!strcmp(bname, "fsck.btrfsck")) {
> +               exit(0);
>         } else {
>                 argc--;
>                 argv++;
> --
> 1.7.9.5
>
> --
> 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

Shouldn't it be fsck.btrfs, not fsck.btrfsck?

Also, fsck.xfs does a bit more than just an exit(0), maybe there's
some merit to what it does. It's a simple shellscript, so check it
out.

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Dan McGrath]

My sincere apologies. It would appear that I was overly careful about
checking the binary functioned when called as a symlink, but not the
correct filename:

  # ls -l `which fsck.btrfs`
  lrwxrwxrwx 1 root root 7 Aug 25  2011 /sbin/fsck.btrfs -> btrfsck

So yes, the patch incorrectly assumed a symlink name of
'fsck.btrfsck', instead of 'fsck.btrfs'.

As for fsck.xfs, I will take a look, thanks!


On Mon, Apr 15, 2013 at 10:03 AM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
>
> On Mon, Apr 15, 2013 at 3:47 PM, Dan McGrath <danmcgrath.ca@gmail.com> wrote:
> > As per FAQ: It is safe to and recommended to turn fsck.btrfs into a no-op
> >
> > Signed-off-by: Dan McGrath <danmcgrath.ca@gmail.com>
> > ---
> >  btrfs.c |    2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/btrfs.c b/btrfs.c
> > index 691adef..78161a9 100644
> > --- a/btrfs.c
> > +++ b/btrfs.c
> > @@ -272,6 +272,8 @@ int main(int argc, char **argv)
> >
> >         if (!strcmp(bname, "btrfsck")) {
> >                 argv[0] = "check";
> > +       } else if (!strcmp(bname, "fsck.btrfsck")) {
> > +               exit(0);
> >         } else {
> >                 argc--;
> >                 argv++;
> > --
> > 1.7.9.5
> >
> > --
> > 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
>
> Shouldn't it be fsck.btrfs, not fsck.btrfsck?
>
> Also, fsck.xfs does a bit more than just an exit(0), maybe there's
> some merit to what it does. It's a simple shellscript, so check it
> out.

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Eric Sandeen]

On 4/15/13 9:03 AM, Jan Alexander Steffens wrote:
> On Mon, Apr 15, 2013 at 3:47 PM, Dan McGrath <danmcgrath.ca@gmail.com> wrote:
>> As per FAQ: It is safe to and recommended to turn fsck.btrfs into a no-op
>>
>> Signed-off-by: Dan McGrath <danmcgrath.ca@gmail.com>
>> ---
>>  btrfs.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/btrfs.c b/btrfs.c
>> index 691adef..78161a9 100644
>> --- a/btrfs.c
>> +++ b/btrfs.c
>> @@ -272,6 +272,8 @@ int main(int argc, char **argv)
>>
>>         if (!strcmp(bname, "btrfsck")) {
>>                 argv[0] = "check";
>> +       } else if (!strcmp(bname, "fsck.btrfsck")) {
>> +               exit(0);
>>         } else {
>>                 argc--;
>>                 argv++;
>> --
>> 1.7.9.5
>>
>> --
>> 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
> 
> Shouldn't it be fsck.btrfs, not fsck.btrfsck?
> 
> Also, fsck.xfs does a bit more than just an exit(0), maybe there's
> some merit to what it does. It's a simple shellscript, so check it
> out.

Yup I think the idea for xfs was that if called by initscripts w/ common
initscript options, be nice and quiet, but if it looks like it was invoked
by someone expecting something to happen, be more informative.

Git blame of the shell script is at 
http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfsprogs.git;a=blame;f=fsck/xfs_fsck.sh;h=c5a96e688b994c36d9ab1b0206225f2f5e7b12e8;hb=HEAD
- it hasn't changed in forever but you might get some hints at why
it does what it does.

Expecting fsck.$FS in initscripts is just a leftover from pre-journaling-fs
days, I think.

-Eric

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Dan McGrath]

Jan,

I got a chance to sit down and dig a little bit deeper into
`fsck.xfs`. Here is what I discovered.

The "(a|A|y|p)" options in the XFS script appear to be nothing more
than the expected `fsck` options that imply automated checks (as is
clearly implied by the use of AUTO). While I have yet to specifically
test the capitalized "A", my guess is that it matches the "-A" options
from fsck(8) for when the system is going through the fstab. The
syntax itself appears to assume that the dev name is the last param
(as indicated by the argc/$#, which gets eval'd into the DEV
variable).

After doing some tests with a hacked up version of the `fsck.xfs`
script, it would appear that the generic `fsck` script calls each
script in order and passes it some parameters to test, since if I pass
`fsck` some random/btrfsck switches:

  # fsck --repair /dev/storage/lv_btrfs

I get an error back from `fsck.ext4`:

  fsck from util-linux 2.20.1
  fsck.ext4: invalid option -- 'e'

Since the man page for fsck(8) hints that there are no real standards
for calling conventions, but most support (a|n|r|y), and as how
unknown options cause an error with fsck (thanks to ext4), perhaps it
would be a good idea for btrfsck to align itself with the standard
options for fsck (a/n/r/y/A)? If anything, the "A" option (for fstab
based) would appear to be the real target of a noop style exit(0) that
I had originally tested.

Also, thanks for the link Eric, just came in as I am typing this reply :)

Anyways, thought I would reply back with some insight on the matter
and see what others had to say, since I am in no position to dictate
the direction that brtfsck/fsck.btrfs should take as far as wrapper
script or no is concerned. Look forward to your replies! o/


Dan

On Mon, Apr 15, 2013 at 10:03 AM, Jan Alexander Steffens
<jan.steffens@gmail.com> wrote:
> On Mon, Apr 15, 2013 at 3:47 PM, Dan McGrath <danmcgrath.ca@gmail.com> wrote:
>> As per FAQ: It is safe to and recommended to turn fsck.btrfs into a no-op
>>
>> Signed-off-by: Dan McGrath <danmcgrath.ca@gmail.com>
>> ---
>>  btrfs.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/btrfs.c b/btrfs.c
>> index 691adef..78161a9 100644
>> --- a/btrfs.c
>> +++ b/btrfs.c
>> @@ -272,6 +272,8 @@ int main(int argc, char **argv)
>>
>>         if (!strcmp(bname, "btrfsck")) {
>>                 argv[0] = "check";
>> +       } else if (!strcmp(bname, "fsck.btrfsck")) {
>> +               exit(0);
>>         } else {
>>                 argc--;
>>                 argv++;
>> --
>> 1.7.9.5
>>
>> --
>> 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
>
> Shouldn't it be fsck.btrfs, not fsck.btrfsck?
>
> Also, fsck.xfs does a bit more than just an exit(0), maybe there's
> some merit to what it does. It's a simple shellscript, so check it
> out.

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Eric Sandeen]

On 4/15/13 11:45 AM, Dan McGrath wrote:
> Jan,
> 
> I got a chance to sit down and dig a little bit deeper into
> `fsck.xfs`. Here is what I discovered.
> 
> The "(a|A|y|p)" options in the XFS script appear to be nothing more
> than the expected `fsck` options that imply automated checks (as is
> clearly implied by the use of AUTO). While I have yet to specifically
> test the capitalized "A", my guess is that it matches the "-A" options
> from fsck(8) for when the system is going through the fstab. The
> syntax itself appears to assume that the dev name is the last param
> (as indicated by the argc/$#, which gets eval'd into the DEV
> variable).
> 
> After doing some tests with a hacked up version of the `fsck.xfs`
> script, it would appear that the generic `fsck` script calls each
> script in order and passes it some parameters to test, since if I pass
> `fsck` some random/btrfsck switches:
> 
>   # fsck --repair /dev/storage/lv_btrfs
> 
> I get an error back from `fsck.ext4`:
> 
>   fsck from util-linux 2.20.1
>   fsck.ext4: invalid option -- 'e'

2 things; from the fsck manpage:

       fsck [-sAVRTMNP] [-C [fd]] [-t fstype] [filesys...]  [--] [fs-specific-options]

so I think you need:

fsck -- --repair /dev/storage/lv_btrfs

But the other issues seems to be that fsck & blkid are autodetecting
the device as ext4, not btrfs; a separate issue.

-Eric

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Dan McGrath]

Jan,

It would appear that:

  # fsck -- --repair /dev/storage/lv_btrfs

doesn't work, but if I put the fs-specific-options at the end:

  # fsck /dev/storage/lv_btrfs -- --repair

it works fine. As we were/are discussing on irc, I also have no idea
where the ext4 lines come from. *scratcheshead*

And to save you guys some testing, here is the script I used and some
sample output:

fsck.btrfs:
========
#!/bin/sh -f

# fsck exit codes for convenience
# 0    - No errors
# 1    - Filesystem errors corrected
# 2    - System should be rebooted
# 4    - Filesystem errors left uncorrected
# 8    - Operational error
# 16   - Usage or syntax error
# 32   - Fsck canceled by user request
# 128  - Shared-library error

AUTO=false
while getopts ":aApy" c
do
        case $c in
        a|A|p|y)        AUTO=true; echo "DEBUG: $c";;
        esac
done
eval DEV=\${$#}
if [ ! -e $DEV ]; then
        echo "$0: $DEV does not exist"
        exit 8
fi
if $AUTO; then
        echo "$0: BTRFS file system."
else
        echo "If you wish to check the consistency of an BTRFS filesystem or"
        echo "repair a damaged filesystem, see btrfsck(8)."
fi
exit 0
========

Output:
======
# fsck /dev/storage/lv_btrfs -- --repair
fsck from util-linux 2.20.1
DEBUG: p
DEBUG: a
/sbin/fsck.btrfs: BTRFS file system.

# fsck /dev/storage/lv_btrfs
fsck from util-linux 2.20.1
If you wish to check the consistency of an BTRFS filesystem or
repair a damaged filesystem, see btrfsck(8).

Note the "p" and "a" switches when called with custom fs switches, but
not when called standalone.


On Mon, Apr 15, 2013 at 1:01 PM, Eric Sandeen <sandeen@redhat.com> wrote:
> On 4/15/13 11:45 AM, Dan McGrath wrote:
>> Jan,
>>
>> I got a chance to sit down and dig a little bit deeper into
>> `fsck.xfs`. Here is what I discovered.
>>
>> The "(a|A|y|p)" options in the XFS script appear to be nothing more
>> than the expected `fsck` options that imply automated checks (as is
>> clearly implied by the use of AUTO). While I have yet to specifically
>> test the capitalized "A", my guess is that it matches the "-A" options
>> from fsck(8) for when the system is going through the fstab. The
>> syntax itself appears to assume that the dev name is the last param
>> (as indicated by the argc/$#, which gets eval'd into the DEV
>> variable).
>>
>> After doing some tests with a hacked up version of the `fsck.xfs`
>> script, it would appear that the generic `fsck` script calls each
>> script in order and passes it some parameters to test, since if I pass
>> `fsck` some random/btrfsck switches:
>>
>>   # fsck --repair /dev/storage/lv_btrfs
>>
>> I get an error back from `fsck.ext4`:
>>
>>   fsck from util-linux 2.20.1
>>   fsck.ext4: invalid option -- 'e'
>
> 2 things; from the fsck manpage:
>
>        fsck [-sAVRTMNP] [-C [fd]] [-t fstype] [filesys...]  [--] [fs-specific-options]
>
> so I think you need:
>
> fsck -- --repair /dev/storage/lv_btrfs
>
> But the other issues seems to be that fsck & blkid are autodetecting
> the device as ext4, not btrfs; a separate issue.
>
> -Eric
>
>

Re: [PATCH] btrfs-progs: No-op when called as fsck.btrfsck

[Zach Brown]

> Anyways, thought I would reply back with some insight on the matter
> and see what others had to say, since I am in no position to dictate
> the direction that brtfsck/fsck.btrfs should take as far as wrapper
> script or no is concerned. Look forward to your replies! o/

FWIW: debian has been carrying a patch to ignore fsck args to help it
run as fsck.btrfs.  I think modern versions of their btrfs-tools package
that are based on more recent btrfs-progs also include the hunk to
recognize being called as fsck.btrfs.

It might be worth finding someone who knows the history of the debian
package.  I'm certainly not that person :).  Here's the result of some
light googling:

  http://ftp.de.debian.org/debian/pool/main/b/btrfs-tools/btrfs-tools_0.19+20130131-3+really20121004-1.debian.tar.xz

debian/patches/08-fsck.patch:

Author: Sten Heinze <shze@gmx.de>
Description:
 Ignore all arguments starting with -a in btrfsck for the time being
 (Closes: #567681, #571300, #612809, #668832).

diff -Naurp btrfs-tools.orig/btrfsck.c btrfs-tools/btrfsck.c
--- btrfs-tools.orig/btrfsck.c	2012-12-10 10:44:57.283351955 +0100
+++ btrfs-tools/btrfsck.c	2012-12-10 10:56:20.971516720 +0100
@@ -3477,6 +3477,12 @@ static void print_usage(void)
 	exit(1);
 }
 
+static void print_info(void)
+{
+	fprintf(stderr, "%s\n", BTRFS_BUILD_VERSION);
+	exit(0);
+}
+
 static struct option long_options[] = {
 	{ "super", 1, NULL, 's' },
 	{ "repair", 0, NULL, 0 },
@@ -3501,7 +3507,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt_long(ac, av, "s:", long_options,
+		c = getopt_long(ac, av, "s:anry", long_options,
 				&option_index);
 		if (c < 0)
 			break;
@@ -3514,6 +3520,12 @@ int main(int ac, char **av)
 				break;
 			case '?':
 				print_usage();
+				break;
+			case 'a':
+			case 'n':
+			case 'r':
+			case 'y':
+				print_info();
 		}
 		if (option_index == 1) {
 			printf("enabling repair mode\n");