On 2018/10/11 下午8:07, Nikolay Borisov wrote:
>
>
> On 8.10.2018 15:30, Qu Wenruo wrote:
>> When restoring btrfs image, the total_bytes of device item is not
>> updated correctly. In fact total_bytes can be left 0 for restored image.
>>
>> It doesn't trigger any error because btrfs check never checks
>> total_bytes of dev item.
>> However this is going to change.
>>
>> Fix it by populating total_bytes of device item with the end position of
>> last dev extent to make later btrfs check happy.
>
> 'Setting it to the end position' really means "setting the total device
> size to the allocated space on the device". Is it more clear if stated
> as the second way ?
Not exactly.
Don't forget that we have 0~1M reserved, and it's possible to have dev
extent holes.
So it's not "allocated space" but really "the end position of the last
dev extent"
Thanks,
Qu
>
> In any case:
>
> Reviewed-by: Nikolay Borisov <[email protected]>
>
>>
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>> image/main.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/image/main.c b/image/main.c
>> index 351c5a256938..d5b89bc3149f 100644
>> --- a/image/main.c
>> +++ b/image/main.c
>> @@ -2082,15 +2082,17 @@ 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)
>> + struct mdrestore_struct *mdres, int out_fd)
>> {
>> struct btrfs_trans_handle *trans;
>> struct btrfs_dev_item *dev_item;
>> + struct btrfs_dev_extent *dev_ext;
>> struct btrfs_path path;
>> struct extent_buffer *leaf;
>> struct btrfs_root *root = fs_info->chunk_root;
>> struct btrfs_key key;
>> u64 devid, cur_devid;
>> + u64 dev_size; /* Get from last dev extents */
>> int ret;
>>
>> trans = btrfs_start_transaction(fs_info->tree_root, 1);
>> @@ -2101,16 +2103,56 @@ static int fixup_devices(struct btrfs_fs_info
>> *fs_info,
>>
>> dev_item = &fs_info->super_copy->dev_item;
>>
>> + btrfs_init_path(&path);
>> devid = btrfs_stack_device_id(dev_item);
>>
>> + key.objectid = devid;
>> + key.type = BTRFS_DEV_EXTENT_KEY;
>> + key.offset = (u64)-1;
>> +
>> + ret = btrfs_search_slot(NULL, fs_info->dev_root, &key, &path, 0, 0);
>> + if (ret < 0) {
>> + error("failed to locate last dev extent of devid %llu: %s",
>> + devid, strerror(-ret));
>> + btrfs_release_path(&path);
>> + return ret;
>> + }
>> + if (ret == 0) {
>
> nit: I'd prefer if this is an else if since it emphasizes the fact that
> the if/elseif construct works on a single value.
>
>> + error("found invalid dev extent devid %llu offset -1",
>> + devid);
>> + btrfs_release_path(&path);
>> + return -EUCLEAN;
>> + }
>> + ret = btrfs_previous_item(fs_info->dev_root, &path, devid,
>> + BTRFS_DEV_EXTENT_KEY);
>> + if (ret > 0)
>> + ret = -ENOENT;
>> + if (ret < 0) {
>
> ditto
>
>> + error("failed to locate last dev extent of devid %llu: %s",
>> + devid, strerror(-ret));
>> + btrfs_release_path(&path);
>> + return ret;
>> + }
>> +
>> + btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
>> + dev_ext = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> + struct btrfs_dev_extent);
>> + dev_size = key.offset + btrfs_dev_extent_length(path.nodes[0], dev_ext);
>> + btrfs_release_path(&path);
>> +
>> btrfs_set_stack_device_total_bytes(dev_item, dev_size);
>> btrfs_set_stack_device_bytes_used(dev_item, mdres->alloced_chunks);
>> + /* Don't forget to enlarge the real file */
>> + ret = ftruncate64(out_fd, dev_size);
>> + if (ret < 0) {
>> + error("failed to enlarge result image: %s", strerror(errno));
>> + return -errno;
>> + }
>>
>> key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
>> key.type = BTRFS_DEV_ITEM_KEY;
>> key.offset = 0;
>>
>> - btrfs_init_path(&path);
>>
>> again:
>> ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
>> @@ -2275,7 +2317,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_devices(info, &mdrestore, fileno(out));
>> close_ctree(info->chunk_root);
>> if (ret)
>> goto out;
>>
