[PULL,67/69] block: Pass truncate exact=true where reasonable

Commit Message

Max Reitz Oct. 28, 2019, 12:14 p.m. UTC

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 <[e-mail]>
Message-id: [e-mail]
Reviewed-by: Maxim Levitsky <[e-mail]>
[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 <[e-mail]>
---
 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(-)

Patch

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;