Re: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root overwritting existing tree blocks
On Fri, Jul 06, 2018 at 01:53:24AM +, Gu, Jinxiang wrote: > > >> Subject: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild > > >> root overwritting existing tree blocks > > >> > > >> We have function btrfs_fsck_reinit_root() to reinit csum or extent tree. > > > > > > Nit： > > > or fs/file tree > > > > Nope, btrfs check is only capable of reinit csum or extent tree. > Sorry, not fs/file tree. But DATA RELOC TREE. > Call stack is reset_balance -> btrfs_fsck_reinit_root, and the root is get by > btrfs_read_fs_root(fs_info, ), which key is > (BTRFS_DATA_RELOC_TREE_OBJECTID, BTRFS_ROOT_ITEM_KEY , (u64)-1) So, it's correct that the data reloc tree is reinitialized, but this is part of extent tree reset and there's no separate commandline option for it. I understand the changelog as a reference to --init-csum-tree and --init-extent-tree . -- 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
RE: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root overwritting existing tree blocks
> -Original Message-
> From: Qu Wenruo [mailto:quwenruo.bt...@gmx.com]
> Sent: Thursday, July 05, 2018 5:41 PM
> To: Gu, Jinxiang/顾 金香 ; Qu Wenruo ;
> linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild
> root overwritting existing tree blocks
>
>
>
> On 2018年07月05日 16:50, Gu, Jinxiang wrote:
> >
> >
> >> -Original Message-
> >> From: linux-btrfs-ow...@vger.kernel.org
> >> [mailto:linux-btrfs-ow...@vger.kernel.org] On Behalf Of Qu Wenruo
> >> Sent: Thursday, July 05, 2018 3:37 PM
> >> To: linux-btrfs@vger.kernel.org
> >> Subject: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild
> >> root overwritting existing tree blocks
> >>
> >> We have function btrfs_fsck_reinit_root() to reinit csum or extent tree.
> >
> > Nit：
> > or fs/file tree
>
> Nope, btrfs check is only capable of reinit csum or extent tree.
Sorry, not fs/file tree. But DATA RELOC TREE.
Call stack is reset_balance -> btrfs_fsck_reinit_root, and the root is get by
btrfs_read_fs_root(fs_info, ), which key is
(BTRFS_DATA_RELOC_TREE_OBJECTID, BTRFS_ROOT_ITEM_KEY , (u64)-1)
>
> Thanks,
> Qu
>
> >
> >> However this function allows us to let it overwrite existing tree blocks
> >> using @overwrite parameter.
> >>
> >> Such behavior is pretty dangerous while no caller is using this feature
> >> explicitly.
> >>
> >> So just remove @overwrite parameter and allow btrfs_fsck_reinit_root()
> >> to error out when it fails to allocate tree block.
> >>
> >> Signed-off-by: Qu Wenruo
> >> ---
> >> check/main.c | 26 --
> >> 1 file changed, 8 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/check/main.c b/check/main.c
> >> index 8db300abb825..c8c347236543 100644
> >> --- a/check/main.c
> >> +++ b/check/main.c
> >> @@ -8379,7 +8379,7 @@ static int do_check_chunks_and_extents(struct
> >> btrfs_fs_info *fs_info)
> >> }
> >>
> >> static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
> >> - struct btrfs_root *root, int overwrite)
> >> +struct btrfs_root *root)
> >> {
> >>struct extent_buffer *c;
> >>struct extent_buffer *old = root->node;
> >> @@ -8389,21 +8389,13 @@ static int btrfs_fsck_reinit_root(struct
> >> btrfs_trans_handle *trans,
> >>
> >>level = 0;
> >>
> >> - if (overwrite) {
> >> - c = old;
> >> - extent_buffer_get(c);
> >> - goto init;
> >> - }
> >>c = btrfs_alloc_free_block(trans, root,
> >> root->fs_info->nodesize,
> >> root->root_key.objectid,
> >> _key, level, 0, 0);
> >> - if (IS_ERR(c)) {
> >> - c = old;
> >> - extent_buffer_get(c);
> >> - overwrite = 1;
> >> - }
> >> -init:
> >> + if (IS_ERR(c))
> >> + return PTR_ERR(c);
> >> +
> >>memset_extent_buffer(c, 0, 0, sizeof(struct btrfs_header));
> >>btrfs_set_header_level(c, level);
> >>btrfs_set_header_bytenr(c, c->start);
> >> @@ -8422,9 +8414,7 @@ init:
> >>/*
> >> * this case can happen in the following case:
> >> *
> >> - * 1.overwrite previous root.
> >> - *
> >> - * 2.reinit reloc data root, this is because we skip pin
> >> + * reinit reloc data root, this is because we skip pin
> >> * down reloc data tree before which means we can allocate
> >> * same block bytenr here.
> >> */
> >> @@ -8609,7 +8599,7 @@ reinit_data_reloc:
> >>goto out;
> >>}
> >>record_root_in_trans(trans, root);
> >> - ret = btrfs_fsck_reinit_root(trans, root, 0);
> >> + ret = btrfs_fsck_reinit_root(trans, root);
> >>if (ret)
> >>goto out;
> >>ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
> >> @@ -8675,7 +8665,7 @@ again:
> >>}
> >>
> >>/* Ok we can allocate now, reinit the extent root */
> >> - ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root, 0);
> >> + ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root);
> >>if (ret) {
> >
Re: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root overwritting existing tree blocks
On 2018年07月05日 16:50, Gu, Jinxiang wrote:
>
>
>> -Original Message-
>> From: linux-btrfs-ow...@vger.kernel.org
>> [mailto:linux-btrfs-ow...@vger.kernel.org] On Behalf Of Qu Wenruo
>> Sent: Thursday, July 05, 2018 3:37 PM
>> To: linux-btrfs@vger.kernel.org
>> Subject: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root
>> overwritting existing tree blocks
>>
>> We have function btrfs_fsck_reinit_root() to reinit csum or extent tree.
>
> Nit：
> or fs/file tree
Nope, btrfs check is only capable of reinit csum or extent tree.
Thanks,
Qu
>
>> However this function allows us to let it overwrite existing tree blocks
>> using @overwrite parameter.
>>
>> Such behavior is pretty dangerous while no caller is using this feature
>> explicitly.
>>
>> So just remove @overwrite parameter and allow btrfs_fsck_reinit_root()
>> to error out when it fails to allocate tree block.
>>
>> Signed-off-by: Qu Wenruo
>> ---
>> check/main.c | 26 --
>> 1 file changed, 8 insertions(+), 18 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 8db300abb825..c8c347236543 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -8379,7 +8379,7 @@ static int do_check_chunks_and_extents(struct
>> btrfs_fs_info *fs_info)
>> }
>>
>> static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
>> - struct btrfs_root *root, int overwrite)
>> + struct btrfs_root *root)
>> {
>> struct extent_buffer *c;
>> struct extent_buffer *old = root->node;
>> @@ -8389,21 +8389,13 @@ static int btrfs_fsck_reinit_root(struct
>> btrfs_trans_handle *trans,
>>
>> level = 0;
>>
>> -if (overwrite) {
>> -c = old;
>> -extent_buffer_get(c);
>> -goto init;
>> -}
>> c = btrfs_alloc_free_block(trans, root,
>> root->fs_info->nodesize,
>> root->root_key.objectid,
>> _key, level, 0, 0);
>> -if (IS_ERR(c)) {
>> -c = old;
>> -extent_buffer_get(c);
>> -overwrite = 1;
>> -}
>> -init:
>> +if (IS_ERR(c))
>> +return PTR_ERR(c);
>> +
>> memset_extent_buffer(c, 0, 0, sizeof(struct btrfs_header));
>> btrfs_set_header_level(c, level);
>> btrfs_set_header_bytenr(c, c->start);
>> @@ -8422,9 +8414,7 @@ init:
>> /*
>> * this case can happen in the following case:
>> *
>> - * 1.overwrite previous root.
>> - *
>> - * 2.reinit reloc data root, this is because we skip pin
>> + * reinit reloc data root, this is because we skip pin
>> * down reloc data tree before which means we can allocate
>> * same block bytenr here.
>> */
>> @@ -8609,7 +8599,7 @@ reinit_data_reloc:
>> goto out;
>> }
>> record_root_in_trans(trans, root);
>> -ret = btrfs_fsck_reinit_root(trans, root, 0);
>> +ret = btrfs_fsck_reinit_root(trans, root);
>> if (ret)
>> goto out;
>> ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
>> @@ -8675,7 +8665,7 @@ again:
>> }
>>
>> /* Ok we can allocate now, reinit the extent root */
>> -ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root, 0);
>> +ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root);
>> if (ret) {
>> fprintf(stderr, "extent root initialization failed\n");
>> /*
>> @@ -9764,7 +9754,7 @@ int cmd_check(int argc, char **argv)
>>
>> if (init_csum_tree) {
>> printf("Reinitialize checksum tree\n");
>> -ret = btrfs_fsck_reinit_root(trans, info->csum_root, 0);
>> +ret = btrfs_fsck_reinit_root(trans, info->csum_root);
>> if (ret) {
>> error("checksum tree initialization failed: %d",
>> ret);
>> --
>
> Reviewed-by: Gu Jinxiang
>
>
>> 2.18.0
>>
>> --
>> 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
>>
>
>
>
> N嫥叉靣笡y氊b瞂千v豝�)藓{.n�+壏{眓谶�)韰骅w* jg� 秹殠娸/侁鋤罐枈�2娹櫒璀�&�)摺玜囤 瓽珴閔�鎗:+v墾妛鑶佶
>
signature.asc
Description: OpenPGP digital signature
RE: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root overwritting existing tree blocks
> -Original Message-
> From: linux-btrfs-ow...@vger.kernel.org
> [mailto:linux-btrfs-ow...@vger.kernel.org] On Behalf Of Qu Wenruo
> Sent: Thursday, July 05, 2018 3:37 PM
> To: linux-btrfs@vger.kernel.org
> Subject: [PATCH 1/4] btrfs-progs: check: Remove the ability to rebuild root
> overwritting existing tree blocks
>
> We have function btrfs_fsck_reinit_root() to reinit csum or extent tree.
Nit：
or fs/file tree
> However this function allows us to let it overwrite existing tree blocks
> using @overwrite parameter.
>
> Such behavior is pretty dangerous while no caller is using this feature
> explicitly.
>
> So just remove @overwrite parameter and allow btrfs_fsck_reinit_root()
> to error out when it fails to allocate tree block.
>
> Signed-off-by: Qu Wenruo
> ---
> check/main.c | 26 --
> 1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/check/main.c b/check/main.c
> index 8db300abb825..c8c347236543 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -8379,7 +8379,7 @@ static int do_check_chunks_and_extents(struct
> btrfs_fs_info *fs_info)
> }
>
> static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
> -struct btrfs_root *root, int overwrite)
> + struct btrfs_root *root)
> {
> struct extent_buffer *c;
> struct extent_buffer *old = root->node;
> @@ -8389,21 +8389,13 @@ static int btrfs_fsck_reinit_root(struct
> btrfs_trans_handle *trans,
>
> level = 0;
>
> - if (overwrite) {
> - c = old;
> - extent_buffer_get(c);
> - goto init;
> - }
> c = btrfs_alloc_free_block(trans, root,
> root->fs_info->nodesize,
> root->root_key.objectid,
> _key, level, 0, 0);
> - if (IS_ERR(c)) {
> - c = old;
> - extent_buffer_get(c);
> - overwrite = 1;
> - }
> -init:
> + if (IS_ERR(c))
> + return PTR_ERR(c);
> +
> memset_extent_buffer(c, 0, 0, sizeof(struct btrfs_header));
> btrfs_set_header_level(c, level);
> btrfs_set_header_bytenr(c, c->start);
> @@ -8422,9 +8414,7 @@ init:
> /*
>* this case can happen in the following case:
>*
> - * 1.overwrite previous root.
> - *
> - * 2.reinit reloc data root, this is because we skip pin
> + * reinit reloc data root, this is because we skip pin
>* down reloc data tree before which means we can allocate
>* same block bytenr here.
>*/
> @@ -8609,7 +8599,7 @@ reinit_data_reloc:
> goto out;
> }
> record_root_in_trans(trans, root);
> - ret = btrfs_fsck_reinit_root(trans, root, 0);
> + ret = btrfs_fsck_reinit_root(trans, root);
> if (ret)
> goto out;
> ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
> @@ -8675,7 +8665,7 @@ again:
> }
>
> /* Ok we can allocate now, reinit the extent root */
> - ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root, 0);
> + ret = btrfs_fsck_reinit_root(trans, fs_info->extent_root);
> if (ret) {
> fprintf(stderr, "extent root initialization failed\n");
> /*
> @@ -9764,7 +9754,7 @@ int cmd_check(int argc, char **argv)
>
> if (init_csum_tree) {
> printf("Reinitialize checksum tree\n");
> - ret = btrfs_fsck_reinit_root(trans, info->csum_root, 0);
> + ret = btrfs_fsck_reinit_root(trans, info->csum_root);
> if (ret) {
> error("checksum tree initialization failed: %d",
> ret);
> --
Reviewed-by: Gu Jinxiang
> 2.18.0
>
> --
> 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
>
