[Qemu-devel] [PATCH v2] qemu-img: Check post-truncation size

[Qemu-devel] [PATCH v2] qemu-img: Check post-truncation size

[Max Reitz]

Some block drivers (iscsi and file-posix when dealing with device files)
do not actually support truncation, even though they provide a
.bdrv_truncate() method and will happily return success when providing a
new size that does not exceed the current size.  This is because these
drivers expect the user to resize the image outside of qemu and then
provide qemu with that information through the block_resize command
(compare cb1b83e740384b4e0d950f3d7c81c02b8ce86c2e).

Of course, anyone using qemu-img resize will find that behavior useless.
So we should check the actual size of the image after the supposedly
successful truncation took place, emit an error if nothing changed and
emit a warning if the target size was not met.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1523065
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
v2: Drop dots in {error,warn}_report() messages [Eric]

v1: http://lists.nongnu.org/archive/html/qemu-block/2018-04/msg00441.html
---
 qemu-img.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 855fa52514..83208873a5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3381,7 +3381,7 @@ static int img_resize(int argc, char **argv)
     Error *err = NULL;
     int c, ret, relative;
     const char *filename, *fmt, *size;
-    int64_t n, total_size, current_size;
+    int64_t n, total_size, current_size, new_size;
     bool quiet = false;
     BlockBackend *blk = NULL;
     PreallocMode prealloc = PREALLOC_MODE_OFF;
@@ -3557,11 +3557,42 @@ static int img_resize(int argc, char **argv)
     }
 
     ret = blk_truncate(blk, total_size, prealloc, &err);
-    if (!ret) {
-        qprintf(quiet, "Image resized.\n");
-    } else {
+    if (ret < 0) {
         error_report_err(err);
+        goto out;
+    }
+
+    new_size = blk_getlength(blk);
+    if (new_size < 0) {
+        error_report("Failed to verify truncated image length: %s",
+                     strerror(-new_size));
+        ret = -1;
+        goto out;
     }
+
+    /* Some block drivers implement a truncation method, but only so
+     * the user can cause qemu to refresh the image's size from disk.
+     * The idea is that the user resizes the image outside of qemu and
+     * then invokes block_resize to inform qemu about it.
+     * (This includes iscsi and file-posix for device files.)
+     * Of course, that is not the behavior someone invoking
+     * qemu-img resize would find useful, so we catch that behavior
+     * here and tell the user. */
+    if (new_size != total_size && new_size == current_size) {
+        error_report("Image was not resized; resizing may not be supported "
+                     "for this image");
+        ret = -1;
+        goto out;
+    }
+
+    if (new_size != total_size) {
+        warn_report("Image should have been resized to %" PRIi64
+                    " bytes, but was resized to %" PRIi64 " bytes",
+                    total_size, new_size);
+    }
+
+    qprintf(quiet, "Image resized.\n");
+
 out:
     blk_unref(blk);
     if (ret) {
-- 
2.14.3

Re: [Qemu-devel] [Qemu-block] [PATCH v2] qemu-img: Check post-truncation size

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

On Sat, Apr 21, 2018 at 06:39:57PM +0200, Max Reitz wrote:
> Some block drivers (iscsi and file-posix when dealing with device files)
> do not actually support truncation, even though they provide a
> .bdrv_truncate() method and will happily return success when providing a
> new size that does not exceed the current size.  This is because these
> drivers expect the user to resize the image outside of qemu and then
> provide qemu with that information through the block_resize command
> (compare cb1b83e740384b4e0d950f3d7c81c02b8ce86c2e).
> 
> Of course, anyone using qemu-img resize will find that behavior useless.
> So we should check the actual size of the image after the supposedly
> successful truncation took place, emit an error if nothing changed and
> emit a warning if the target size was not met.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1523065
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: Drop dots in {error,warn}_report() messages [Eric]
> 
> v1: http://lists.nongnu.org/archive/html/qemu-block/2018-04/msg00441.html
> ---
>  qemu-img.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v2] qemu-img: Check post-truncation size

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

On 2018-04-21 18:39, Max Reitz wrote:
> Some block drivers (iscsi and file-posix when dealing with device files)
> do not actually support truncation, even though they provide a
> .bdrv_truncate() method and will happily return success when providing a
> new size that does not exceed the current size.  This is because these
> drivers expect the user to resize the image outside of qemu and then
> provide qemu with that information through the block_resize command
> (compare cb1b83e740384b4e0d950f3d7c81c02b8ce86c2e).
> 
> Of course, anyone using qemu-img resize will find that behavior useless.
> So we should check the actual size of the image after the supposedly
> successful truncation took place, emit an error if nothing changed and
> emit a warning if the target size was not met.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1523065
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: Drop dots in {error,warn}_report() messages [Eric]
> 
> v1: http://lists.nongnu.org/archive/html/qemu-block/2018-04/msg00441.html
> ---
>  qemu-img.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)

Thanks for the reviews (v1 and v2), applied to my block branch.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]