On 27.11.18 г. 10:50 ч., Qu Wenruo wrote:
>
>
> 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? :)
Didn't even realize this was the intended effect. IMO doesn't make much
difference, what does, though, is probably reverse christmas tree, ie
longer variable names
come before shorter
ones
But I guess this is a matter of taste, no need to resend.
>
> 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;
>>>
>
