-------- Original Message --------
Subject: Re: [PATCH v3 00/10] Enhance btrfs-find-root and open_ctree()
to provide better chance on damaged btrfs.
From: David Sterba <dsterba@xxxxxxx>
To: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
Date: 2015年02月12日 21:16
On Thu, Feb 12, 2015 at 09:36:01AM +0800, Qu Wenruo wrote:
Subject: Re: [PATCH v3 00/10] Enhance btrfs-find-root and open_ctree()
to provide better chance on damaged btrfs.
From: David Sterba <dsterba@xxxxxxx>
To: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
Date: 2015年02月12日 01:52
On Wed, Feb 11, 2015 at 08:33:03AM +0800, Qu Wenruo wrote:
Also, since only 2 patches is modified(although other part is slightly
modified to match the change), to avoid mail bombing, I created the pull
request on github and only send the first 2 patches with cover-letter.
https://github.com/kdave/btrfs-progs/pull/5
Sending the changed patches only is ok (if you point me at the rest of
the patches), but it's not necessary to open the github pull request.
The version to version changelogs are also stored in the commit
changelogs, that's a bit unexpected for a branch to be pulled.
Oh, very sorry for this.
I was meant to save your time, but I forgot that pull branch won't emit
the changelog like patches.
Pulled except the last patch, and I've cleaned up some bits so please
have a look. It's basically what I'd tell you during a normal review but
now it was easier to do myself.
Thanks for merging and modifying them.
It seems that my naming sense is not so good and the new naming looks
good for me, except some of them,
like OPEN_CTREE_SUPPRESS_CHECK_TREE_ERROR seems too long for me, but
that's all right and doesn't
do any harm.
Yeah it's long and I'm not completely happy about that but I gave
preference to descriptiveness as we may want to add other fine tuning
flags to open_ctree. As the TODO you've added to the enum definition
says, we may split the flags and then cleanup as needed.
And it seems that the patch I send it still out of date and some naming
changes in my v3 patch doesn't show in
it...
I've used the v3 branch from github. Please send incremental fixes if I
missed something.
The modification seems better than mine, so that's completely OK for me.
No need for other patches.
My concern about the patch "btrfs-progs: Allow open_ctree use backup
tree root or search it automatically if primary..." is the
'automatically' part. Falling to the backup roots should be IMO on
request. The tools should have (and some of them already do have)
commandline options to request a given backup root. That way the user
can try the default action and then decide if the backup roots are fine
for use.
What about ask user to do such fallback method?
IMHO, such way should take a balance for average user and advanced user
who can, for example, extract
the bad block and fix it manually without corruption.
Agreed this is about finding a good balance. However, here a regular
user could do more harm than good if the tool just "does something" and
does not stop if there's not a safe way forward.
Current btrfsck has a problem that doesn't give enough info on possible
solutions.
Right, this should be improved in the long term. This requires
documentation of the internals and some level of knowledge even if the
tool provides options how to proceed.
The case is much like --init-(csum/extent)-tree, we provide such
options, but when a tree block in extent
tree happens, the backref mismatch error messages won't really help to
guide user to use --init-extent-tree
option.
How do you think about the ask-user method?
The options are:
* exit if user input/decision is required
* pass command line options to checker to preset the behaviour
* add a specialized subcommand to fix a particular class of errors
* run checker in interactive mode that asks user if she/he wants to fix
the problem, possibly how if there are more options
Thanks for all the options.
These options are better than my original just ask_user() method.
The superblock and root backup options are very handy in the analysis
phase, so one can see the differences in the output. I think the same
holds for the find-root command. We don't have a "cookbook" how to
analyze a broken filesystem. Your and my experience is probably
different in how to do the analysis but I believe we will find a common
ground and implement that into the tools and documentation.
This makes sense, I was only focused on the btrfsck --repair work,
forgot that we can use btrfsck and btrfs-find-root
to do analysis before we do repair.
Considering analysis, I'd like a add a prompt if we failed to read tree
root, guiding user to use the new
option like --search-root(which will first use backup and then
find-root) or btrfs-find-root to find the best tree root.
Thanks,
Qu
--
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
