Re: [PATCH 1/3] btrfs-progs: cleanup btrfs-rescue output msgs

[Gui Hecheng <guihc.fnst@cn.fujitsu.com>]

On Wed, 2014-05-28 at 18:24 +0200, David Sterba wrote:
> On Thu, May 15, 2014 at 09:29:07AM +0800, Gui Hecheng wrote:
> > Use enum defined error codes to represent different kinds of errs
> > for super-recover and chunk-recover.
> 
> I think this change hides the low-level errors (like ENOMEM) that can
> possibly result into "recovery not possible", though it can be restarted
> and could work fine.
> 
> The human readable error messages are good, but should also reflect if
> the error was fatal or not and say "why".
> 
> Examples:
> 
> > @@ -2092,12 +2113,14 @@ int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
> >  	ret = recover_prepare(&rc, path);
> >  	if (ret) {
> >  		fprintf(stderr, "recover prepare error\n");
> > +		ret = ERR_CR_FAILED_TO_RECOVER;
> 
> eg. recover_prepare can fail if it does not find the path or due to ENOMEM
> 
> >  		return ret;
> >  	}
> >  
> >  	ret = scan_devices(&rc);
> >  	if (ret) {
> >  		fprintf(stderr, "scan chunk headers error\n");
> > +		ret = ERR_CR_FAILED_TO_RECOVER;
> 
> device open fails, or ENOMEM
> 
> >  		goto fail_rc;
> >  	}
> 
> So, somehow wrap both values into one and convert into the enhanced
> messages.

Hi David,
Thanks for your advice. I'll rework it soon and resend.

Something to confirm below:

o Actually I've kept almost all the "fprintf"s in the original place as
*low level* messages(except an "PTR_ERR", I'll add it back).
But it seems that the original "fprintf"s do hide low level errors, and
I'll try to enhance them in the original "fprintf"s.

o What I've added are just some *user level* messages which will show
along with the low level messages, bug not replace them.
I would like to just keep this part.

What do you think?

-Gui