Skip to content

Commit

Permalink
block: Pass truncate exact=true where reasonable
Browse files Browse the repository at this point in the history
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
(c743849 came eight months before 55b949c), so it was simply
broken until now.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190918095144.955-8-mreitz@redhat.com
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
[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 <mreitz@redhat.com>
  • Loading branch information
XanClic committed Oct 28, 2019
1 parent e61a28a commit e8d04f9
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 7 deletions.
11 changes: 9 additions & 2 deletions block/parallels.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
6 changes: 5 additions & 1 deletion block/qcow2.c
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions block/qed.c
Expand Up @@ -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;
}
Expand Down
7 changes: 6 additions & 1 deletion qemu-img.c
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion qemu-io-cmds.c
Expand Up @@ -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;
Expand Down

0 comments on commit e8d04f9

Please sign in to comment.