From c80d8b06cfa59f5d8229379c85dcb2c3bb9c881b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 18 Sep 2019 11:51:40 +0200 Subject: block: Add @exact parameter to bdrv_co_truncate() We have two drivers (iscsi and file-posix) that (in some cases) return success from their .bdrv_co_truncate() implementation if the block device is larger than the requested offset, but cannot be shrunk. Some callers do not want that behavior, so this patch adds a new parameter that they can use to turn off that behavior. This patch just adds the parameter and lets the block/io.c and block/block-backend.c functions pass it around. All other callers always pass false and none of the implementations evaluate it, so that this patch does not change existing behavior. Future patches take care of that. Suggested-by: Maxim Levitsky Signed-off-by: Max Reitz Message-id: 20190918095144.955-5-mreitz@redhat.com Reviewed-by: Maxim Levitsky Signed-off-by: Max Reitz --- qemu-io-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'qemu-io-cmds.c') diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 349256a5fe..5e9017c979 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1710,7 +1710,7 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv) return offset; } - ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err); + ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err); if (ret < 0) { error_report_err(local_err); return ret; -- cgit 1.4.1 From e8d04f92378c2de7b464e04469a657fd37eb29ea Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 18 Sep 2019 11:51:43 +0200 Subject: 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(-) (limited to 'qemu-io-cmds.c') 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; -- cgit 1.4.1