| Message ID | 1366033629-25121-2-git-send-email-danmcgrath.ca@gmail.com (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
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. -- 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
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. -- 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
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 -- 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
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. -- 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
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 -- 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
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
>
>
--
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
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++;
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(+)