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 <mail@xxxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
