On 2018/11/27 下午4:46, Nikolay Borisov wrote:
>
>
> On 27.11.18 г. 10:38 ч., Qu Wenruo wrote:
>> Current fixup_devices() will only remove DEV_ITEMs and reset DEV_ITEM
>> size.
>> Later we need to do more fixup works, so change the name to
>> fixup_chunks_and_devices() and refactor the original device size fixup
>> to fixup_device_size().
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>
> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
>
> However, one minor nit below.
>
>> ---
>> image/main.c | 52 ++++++++++++++++++++++++++++++++++++----------------
>> 1 file changed, 36 insertions(+), 16 deletions(-)
>>
>> diff --git a/image/main.c b/image/main.c
>> index c680ab19de6c..bbfcf8f19298 100644
>> --- a/image/main.c
>> +++ b/image/main.c
>> @@ -2084,28 +2084,19 @@ static void remap_overlapping_chunks(struct mdrestore_struct *mdres)
>> }
>> }
>>
>> -static int fixup_devices(struct btrfs_fs_info *fs_info,
>> - struct mdrestore_struct *mdres, off_t dev_size)
>> +static int fixup_device_size(struct btrfs_trans_handle *trans,
>> + struct mdrestore_struct *mdres,
>> + off_t dev_size)
>> {
>> - struct btrfs_trans_handle *trans;
>> + struct btrfs_fs_info *fs_info = trans->fs_info;
>> struct btrfs_dev_item *dev_item;
>> struct btrfs_path path;
>> - struct extent_buffer *leaf;
>> struct btrfs_root *root = fs_info->chunk_root;
>> struct btrfs_key key;
>> + struct extent_buffer *leaf;
>
> nit: Unnecessary change
Doesn't it look better when all btrfs_ prefix get batched together? :)
Thanks,
Qu
>
>> u64 devid, cur_devid;
>> int ret;
>>
>> - if (btrfs_super_log_root(fs_info->super_copy)) {
>> - warning(
>> - "log tree detected, its generation will not match superblock");
>> - }
>> - trans = btrfs_start_transaction(fs_info->tree_root, 1);
>> - if (IS_ERR(trans)) {
>> - error("cannot starting transaction %ld", PTR_ERR(trans));
>> - return PTR_ERR(trans);
>> - }
>> -
>> dev_item = &fs_info->super_copy->dev_item;
>>
>> devid = btrfs_stack_device_id(dev_item);
>> @@ -2123,7 +2114,7 @@ again:
>> ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
>> if (ret < 0) {
>> error("search failed: %d", ret);
>> - exit(1);
>> + return ret;
>> }
>>
>> while (1) {
>> @@ -2170,12 +2161,41 @@ again:
>> }
>>
>> btrfs_release_path(&path);
>> + return 0;
>> +}
>> +
>> +static int fixup_chunks_and_devices(struct btrfs_fs_info *fs_info,
>> + struct mdrestore_struct *mdres, off_t dev_size)
>> +{
>> + struct btrfs_trans_handle *trans;
>> + int ret;
>> +
>> + if (btrfs_super_log_root(fs_info->super_copy)) {
>> + warning(
>> + "log tree detected, its generation will not match superblock");
>> + }
>> + trans = btrfs_start_transaction(fs_info->tree_root, 1);
>> + if (IS_ERR(trans)) {
>> + error("cannot starting transaction %ld", PTR_ERR(trans));
>> + return PTR_ERR(trans);
>> + }
>> +
>> + ret = fixup_device_size(trans, mdres, dev_size);
>> + if (ret < 0)
>> + goto error;
>> +
>> ret = btrfs_commit_transaction(trans, fs_info->tree_root);
>> if (ret) {
>> error("unable to commit transaction: %d", ret);
>> return ret;
>> }
>> return 0;
>> +error:
>> + error(
>> +"failed to fix chunks and devices mapping, the fs may not be mountable: %s",
>> + strerror(-ret));
>> + btrfs_abort_transaction(trans, ret);
>> + return ret;
>> }
>>
>> static int restore_metadump(const char *input, FILE *out, int old_restore,
>> @@ -2282,7 +2302,7 @@ static int restore_metadump(const char *input, FILE *out, int old_restore,
>> return 1;
>> }
>>
>> - ret = fixup_devices(info, &mdrestore, st.st_size);
>> + ret = fixup_chunks_and_devices(info, &mdrestore, st.st_size);
>> close_ctree(info->chunk_root);
>> if (ret)
>> goto out;
>>
