On 5/28/14, 9:10 AM, Shilong Wang wrote: > 2014-05-28 21:56 GMT+08:00 Eric Sandeen <sand...@redhat.com>: >> 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.f...@cn.fujitsu.com> >>> --- >>> 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 majord...@vger.kernel.org >> 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 majord...@vger.kernel.org > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html