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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to