On Mon, Oct 08, 2012 at 09:07:49PM +0200, Dieter Ries wrote: > Status reports of the checking process should be printed to stdout > instead of stderr, as that is normal program output and not related to > problems in btrfsck.
I agree that the important messages from fsck process should be printed to stdout, and the rest like 'cannot find a valid fs on /dev' belong to stderr so the user can simply call btrfsck > logfile and does not miss any messages in the log, while will be informed that the process cannot proceed for some urgent reason. So far we all do 'btrfsck >& logfile' to be sure we don't miss anythingk. > Signed-off-by: Dieter Ries <[email protected]> > --- > btrfsck.c | 20 +++++++++++++++----- > 1 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/btrfsck.c b/btrfsck.c > index 516bcdf..83275cd 100644 > --- a/btrfsck.c > +++ b/btrfsck.c > @@ -3559,7 +3559,7 @@ int main(int ac, char **av) > > root = info->fs_root; > > - fprintf(stderr, "checking extents\n"); > + printf("checking extents... "); > if (rw) > trans = btrfs_start_transaction(root, 1); > > @@ -3567,22 +3567,32 @@ int main(int ac, char **av) > fprintf(stderr, "Reinit crc root\n"); > ret = btrfs_fsck_reinit_root(trans, info->csum_root); > if (ret) { > + printf("\n"); This looks strange, it's missing the context where it actually adds the newline. > fprintf(stderr, "crc root initialization failed\n"); ^^^ this is IMHO another printf candidate, though btrfs_fsck_reinit_root will not fail. > return -EIO; > } > goto out; > } > ret = check_extents(trans, root, repair); > - if (ret) > + if (ret) { > fprintf(stderr, "Errors found in extent allocation tree\n"); same here > + printf("\n"); > + } > + else > + printf("Done!\n"); > > - fprintf(stderr, "checking fs roots\n"); > + printf("checking fs roots... "); If you remove the newline, the output may be buffered and not visible until the newline arrives > ret = check_fs_roots(root, &root_cache); > - if (ret) > + if (ret) { > + printf("\n"); > goto out; > + } > + else } else { > + printf("Done!\n"); } > > - fprintf(stderr, "checking root refs\n"); > + printf("checking root refs... "); > ret = check_root_refs(root, &root_cache); Done, but what if ret is not zero? This goes unreported. > + printf("Done!\n"); > out: > free_root_recs(&root_cache); > if (rw) { I think doing the stdout/stderr split properly would need more than fixing btrfsck.c, it uses code from other .c files, looks like an overhaul of the logging in the whole codebase. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
