On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote: > When truncating a format node, the @exact parameter is generally handled > simply by virtue of the format storing the new size in the image > metadata. Such formats do not need to pass on the parameter to their > file nodes. > > There are exceptions, though: > - raw and crypto cannot store the image size, and thus must pass on > @exact. > > - When using qcow2 with an external data file, it just makes sense to > keep its size in sync with the qcow2 virtual disk (because the > external data file is the virtual disk). Therefore, we should pass > @exact when truncating it. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block/crypto.c | 2 +- > block/qcow2.c | 15 ++++++++++++++- > block/raw-format.c | 2 +- > 3 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/block/crypto.c b/block/crypto.c > index e5a1a2cdf3..24823835c1 100644 > --- a/block/crypto.c > +++ b/block/crypto.c > @@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t > offset, bool exact, > > offset += payload_offset; > > - return bdrv_co_truncate(bs->file, offset, false, prealloc, errp); > + return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); > } > > static void block_crypto_close(BlockDriverState *bs) > diff --git a/block/qcow2.c b/block/qcow2.c > index 157b9b75d9..4ef19dd29a 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3822,6 +3822,13 @@ static int coroutine_fn > qcow2_co_truncate(BlockDriverState *bs, int64_t offset, > if ((last_cluster + 1) * s->cluster_size < old_file_size) { > Error *local_err = NULL; > > + /* > + * Do not pass @exact here: It will not help the user if > + * we get an error here just because they wanted to shrink > + * their qcow2 image (on a block device) with qemu-img. > + * (And on the qcow2 layer, the @exact requirement is > + * always fulfilled, so there is no need to pass it on.) > + */ > bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size, > false, PREALLOC_MODE_OFF, &local_err); > if (local_err) { > @@ -3840,7 +3847,12 @@ static int coroutine_fn > qcow2_co_truncate(BlockDriverState *bs, int64_t offset, > switch (prealloc) { > case PREALLOC_MODE_OFF: > if (has_data_file(bs)) { > - ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, > errp); > + /* > + * If the caller wants an exact resize, the external data > + * file should be resized to the exact target size, too, > + * so we pass @exact here. > + */ > + ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, > errp); > if (ret < 0) { > goto fail; > } > @@ -3925,6 +3937,7 @@ static int coroutine_fn > qcow2_co_truncate(BlockDriverState *bs, int64_t offset, > /* Allocate the data area */ > new_file_size = allocation_start + > nb_new_data_clusters * s->cluster_size; > + /* Image file grows, so @exact does not matter */ > ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, > errp); > > if (ret < 0) { > error_prepend(errp, "Failed to resize underlying file: "); > diff --git a/block/raw-format.c b/block/raw-format.c > index 57d84d0bae..3a76ec7dd2 100644 > --- a/block/raw-format.c > +++ b/block/raw-format.c > @@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState > *bs, int64_t offset, > > s->size = offset; > offset += s->offset; > - return bdrv_co_truncate(bs->file, offset, false, prealloc, errp); > + return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); > } > > static void raw_eject(BlockDriverState *bs, bool eject_flag)
Looks all right. Reviewed-by: Maxim Levitsky <maximlevi...@redhat.com> Best regards, Maxim Levitsky