On Thu, 03/24 23:33, Max Reitz wrote:
> When passing -S 0 to qemu-img convert, the target image is supposed to
> be fully allocated. Right now, this is not the case if the source image
> contains areas which bdrv_get_block_status() reports as being zero.
>
> This patch changes a zeroed area's status from BLK_ZERO to BLK_DATA
> before invoking convert_write() if -S 0 has been specified. In addition,
> the check whether convert_read() actually needs to do anything
> (basically only if the current area is a BLK_DATA area) is pulled out of
> that function to the caller.
>
> If -S 0 has been specified, zeroed areas need to be written as data to
> the output, thus they then have to be accounted when calculating the
> progress made.
>
> This patch changes the reference output for iotest 122; contrary to what
> it assumed, -S 0 really should allocate everything in the output, not
> just areas that are filled with zeros (as opposed to being zeroed).
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> qemu-img.c | 26 +++++++++++++++-----------
> tests/qemu-iotests/122.out | 6 ++----
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 29eae2a..b2e07bb 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1507,10 +1507,6 @@ static int convert_read(ImgConvertState *s, int64_t
> sector_num, int nb_sectors,
> int n;
> int ret;
>
> - if (s->status == BLK_ZERO || s->status == BLK_BACKING_FILE) {
> - return 0;
> - }
> -
> assert(nb_sectors <= s->buf_sectors);
> while (nb_sectors > 0) {
> BlockBackend *blk;
> @@ -1648,7 +1644,8 @@ static int convert_do_copy(ImgConvertState *s)
> ret = n;
> goto fail;
> }
> - if (s->status == BLK_DATA) {
> + if (s->status == BLK_DATA || (!s->min_sparse && s->status ==
> BLK_ZERO))
> + {
> s->allocated_sectors += n;
> }
> sector_num += n;
> @@ -1668,17 +1665,24 @@ static int convert_do_copy(ImgConvertState *s)
> ret = n;
> goto fail;
> }
> - if (s->status == BLK_DATA) {
> + if (s->status == BLK_DATA || (!s->min_sparse && s->status ==
> BLK_ZERO))
> + {
> allocated_done += n;
> qemu_progress_print(100.0 * allocated_done /
> s->allocated_sectors,
> 0);
> }
>
> - ret = convert_read(s, sector_num, n, buf);
> - if (ret < 0) {
> - error_report("error while reading sector %" PRId64
> - ": %s", sector_num, strerror(-ret));
> - goto fail;
> + if (s->status == BLK_DATA) {
> + ret = convert_read(s, sector_num, n, buf);
> + if (ret < 0) {
> + error_report("error while reading sector %" PRId64
> + ": %s", sector_num, strerror(-ret));
> + goto fail;
> + }
> + } else if (!s->min_sparse && s->status == BLK_ZERO) {
> + n = MIN(n, s->buf_sectors);
> + memset(buf, 0, n * BDRV_SECTOR_SIZE);
> + s->status = BLK_DATA;
> }
>
> ret = convert_write(s, sector_num, n, buf);
> diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
> index 0068e96..98814de 100644
> --- a/tests/qemu-iotests/122.out
> +++ b/tests/qemu-iotests/122.out
> @@ -112,16 +112,14 @@ read 3145728/3145728 bytes at offset 0
> 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> read 63963136/63963136 bytes at offset 3145728
> 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true,
> "offset": 327680},
> -{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data":
> false}]
> +[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true,
> "offset": 327680}]
>
> convert -c -S 0:
> read 3145728/3145728 bytes at offset 0
> 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> read 63963136/63963136 bytes at offset 3145728
> 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true},
> -{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data":
> false}]
> +[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true}]
> Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> wrote 33554432/33554432 bytes at offset 0
> 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> --
> 2.7.4
>
Looks sane to me,
Reviewed-by: Fam Zheng <f...@redhat.com>
