Re: [PATCH 2/3] btrfs-progs: image: enlarge the output file if no tree modification is needed for restore

[Su Yue <l@damenly.su>]

On Fri 26 Mar 2021 at 20:50, Qu Wenruo <wqu@suse.com> wrote:

> [BUG]
> If restoring dumpped image into a new file, under most cases 
> kernel will
> reject it:
>
>  # mkfs.btrfs -f /dev/test/test
>  # btrfs-image /dev/test/test /tmp/dump
>  # btrfs-image -r /tmp/dump ~/test.img
>  # mount ~/test.img /mnt/btrfs
>  mount: /mnt/btrfs: wrong fs type, bad option, bad superblock on 
>  /dev/loop0, missing codepage or helper program, or other error.
>  # dmesg -t | tail -n 7
>  loop0: detected capacity change from 10592 to 0
>  BTRFS info (device loop0): disk space caching is enabled
>  BTRFS info (device loop0): has skinny extents
>  BTRFS info (device loop0): flagging fs with big metadata 
>  feature
>  BTRFS error (device loop0): device total_bytes should be at 
>  most 5423104 but found 10737418240
>  BTRFS error (device loop0): failed to read chunk tree: -22
>  BTRFS error (device loop0): open_ctree failed
>
> [CAUSE]
> When btrfs-image restores an image into a file, and the source 
> image
> contains only single device, then we don't need to modify the
> chunk/device tree, as we can reuse the existing chunk/dev tree 
> without
> any problem.
>
> This also means, for such restore, we also won't do any target 
> file
> enlarge. This behavior itself is fine, as at that time, kernel 
> won't
> check if the device is smaller than the device size recorded in 
> device
> tree.
>
> But later kernel commit 3a160a933111 ("btrfs: drop never met 
> disk total
> bytes check in verify_one_dev_extent") introduces new check on 
> device
> size at mount time, rejecting any loop file which is smaller 
> than the
> original device size.
>
> [FIX]
> Do extra file enlarge for single device restore.
>
> Reported-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  image/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/image/main.c b/image/main.c
> index 24393188e5e3..9933f69d0fdb 100644
> --- a/image/main.c
> +++ b/image/main.c
> @@ -2706,6 +2706,49 @@ static int restore_metadump(const char 
> *input, FILE *out, int old_restore,
>  		close_ctree(info->chunk_root);
>  		if (ret)
>  			goto out;
> +	} else {
> +		struct btrfs_root *root;
> +		struct stat st;
> +		u64 dev_size;
> +
> +		if (!info) {
>
Here info is not NULL else above fixup_chunks_and_devices() 
already
crashed at deferencing fs_info->super_copy. So I guess the branch
can be deleted?

> +			root = open_ctree_fd(fileno(out), target, 0, 0);
> +			if (!root) {
> +				error("open ctree failed in %s", target);
> +				ret = -EIO;
> +				goto out;
> +			}
> +
> +			info = root->fs_info;
> +
> +			dev_size = btrfs_stack_device_total_bytes(
> +					&info->super_copy->dev_item);
> +			close_ctree(root);
> +			info = NULL;
> +		} else {
> +			dev_size = btrfs_stack_device_total_bytes(
> +					&info->super_copy->dev_item);
> +		}
> +
> +		/*
> +		 * We don't need extra tree modification, but if the 
> output is
> +		 * a file, we need to enlarge the output file so that
> +		 * newer kernel won't report error.
> +		 */
> +		ret = fstat(fileno(out), &st);
> +		if (ret < 0) {
> +			error("failed to stat result image: %m");
> +			ret = -errno;
> +			goto out;
> +		}
> +		if (S_ISREG(st.st_mode)) {
> +			ret = ftruncate64(fileno(out), dev_size);
> +			if (ret < 0) {
> +				error("failed to enlarge result image: %m");
> +				ret = -errno;
> +				goto out;
>
I see there is a same pattern inside fixup_device_size().

--
Su
> +			}
> +		}
>  	}
>  out:
>  	mdrestore_destroy(&mdrestore, num_threads);