On 5/28/14, 9:10 AM, Shilong Wang wrote:
> 2014-05-28 21:56 GMT+08:00 Eric Sandeen <sandeen@xxxxxxxxxx>:
>> The subject and the comment say what this change does, but
>> that's obvious from reading the code. Nothing says *why*
>> the change has been made. What does this fix, and how does
>> it fix it?
>
> Yup, the reason that we allow partial opening is that sometimes,
> we may have a corrupted extent tree(for example), but for fsck repair case,
> the broken tree maybe rebuilt.
>
> So if users only want to do check but not repaired, this patch will make fsck
> return failure as soon as possible and tell users that some critial roots have
> been corrupted...
>
> Let's come to your comments, Eric, you are absolutely right, i was a little
> lazy sometimes...
Sometimes I am too. ;)
> I would add necessay changelog to describe why we need this patch,
> thanks for your comments.
thanks. Perhaps it's obvious to people more familiar with the code, but
I think it's always helpful to be a bit more descriptive; things that make
perfect sense right now can sometimes be confusing a year or so later.
Thanks,
-Eric
> Regards,
> Wang
>>
>> Can you add/update the commit log so that some reader in the future
>> (or for that matter, a reviewer in the present) will have an idea
>> about the reason for this change? What was the failure case, what
>> was the failure mode, why does this change fix it, etc.
>>
>> Thanks,
>> -Eric
>>
>> On 5/28/14, 6:20 AM, Wang Shilong wrote:
>>> Signed-off-by: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
>>> ---
>>> cmds-check.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cmds-check.c b/cmds-check.c
>>> index db7df80..0e4e042 100644
>>> --- a/cmds-check.c
>>> +++ b/cmds-check.c
>>> @@ -6810,8 +6810,7 @@ int cmd_check(int argc, char **argv)
>>> int option_index = 0;
>>> int init_csum_tree = 0;
>>> int qgroup_report = 0;
>>> - enum btrfs_open_ctree_flags ctree_flags =
>>> - OPEN_CTREE_PARTIAL | OPEN_CTREE_EXCLUSIVE;
>>> + enum btrfs_open_ctree_flags ctree_flags = OPEN_CTREE_EXCLUSIVE;
>>>
>>> while(1) {
>>> int c;
>>> @@ -6877,6 +6876,10 @@ int cmd_check(int argc, char **argv)
>>> goto err_out;
>>> }
>>>
>>> + /* only allow partial opening under repair mode */
>>> + if (repair)
>>> + ctree_flags |= OPEN_CTREE_PARTIAL;
>>> +
>>> info = open_ctree_fs_info(argv[optind], bytenr, 0, ctree_flags);
>>> if (!info) {
>>> fprintf(stderr, "Couldn't open file system\n");
>>>
>>
>> --
>> 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
> --
> 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
>
--
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
