From e8d04f92378c2de7b464e04469a657fd37eb29ea Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 18 Sep 2019 11:51:43 +0200 Subject: [PATCH] block: Pass truncate exact=true where reasonable This is a change in behavior, so all instances need a good justification. The comments added here should explain my reasoning. qed already had a comment that suggests it always expected bdrv_truncate()/blk_truncate() to behave as if exact=true were passed (c743849bee7 came eight months before 55b949c8476), so it was simply broken until now. Signed-off-by: Max Reitz Message-id: 20190918095144.955-8-mreitz@redhat.com Reviewed-by: Maxim Levitsky [mreitz: Changed comment in qed.c to explain why a new QED file must be empty, as requested and suggested by Maxim] Signed-off-by: Max Reitz --- block/parallels.c | 11 +++++++++-- block/qcow2.c | 6 +++++- block/qed.c | 7 +++++-- qemu-img.c | 7 ++++++- qemu-io-cmds.c | 7 ++++++- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index a1a92c97a4..603211f83c 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -487,7 +487,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs, res->leaks += count; if (fix & BDRV_FIX_LEAKS) { Error *local_err = NULL; - ret = bdrv_truncate(bs->file, res->image_end_offset, false, + + /* + * In order to really repair the image, we must shrink it. + * That means we have to pass exact=true. + */ + ret = bdrv_truncate(bs->file, res->image_end_offset, true, PREALLOC_MODE_OFF, &local_err); if (ret < 0) { error_report_err(local_err); @@ -880,7 +885,9 @@ static void parallels_close(BlockDriverState *bs) if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) { s->header->inuse = 0; parallels_update_header(bs); - bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false, + + /* errors are ignored, so we might as well pass exact=true */ + bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true, PREALLOC_MODE_OFF, NULL); } diff --git a/block/qcow2.c b/block/qcow2.c index 9f32dae41f..7c18721741 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -5323,7 +5323,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, return ret; } - ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp); + /* + * Amending image options should ensure that the image has + * exactly the given new values, so pass exact=true here. + */ + ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, errp); blk_unref(blk); if (ret < 0) { return ret; diff --git a/block/qed.c b/block/qed.c index 7c2a65af40..d8c4e5fb1e 100644 --- a/block/qed.c +++ b/block/qed.c @@ -673,8 +673,11 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts, l1_size = header.cluster_size * header.table_size; - /* File must start empty and grow, check truncate is supported */ - ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp); + /* + * The QED format associates file length with allocation status, + * so a new file (which is empty) must have a length of 0. + */ + ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, errp); if (ret < 0) { goto out; } diff --git a/qemu-img.c b/qemu-img.c index c738297b04..b288c967b5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3831,7 +3831,12 @@ static int img_resize(int argc, char **argv) } } - ret = blk_truncate(blk, total_size, false, prealloc, &err); + /* + * The user expects the image to have the desired size after + * resizing, so pass @exact=true. It is of no use to report + * success when the image has not actually been resized. + */ + ret = blk_truncate(blk, total_size, true, prealloc, &err); if (ret < 0) { error_report_err(err); goto out; diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 5e9017c979..1b7e700020 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1710,7 +1710,12 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv) return offset; } - ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err); + /* + * qemu-io is a debugging tool, so let us be strict here and pass + * exact=true. It is better to err on the "emit more errors" side + * than to be overly permissive. + */ + ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, &local_err); if (ret < 0) { error_report_err(local_err); return ret;