Re: [Qemu-devel] [PATCH v3 04/16] qemu-img: Expose PreallocMode for resizing

[Max Reitz]

On 2017-05-30 22:57, Eric Blake wrote:
> On 05/26/2017 11:55 AM, Max Reitz wrote:
>> Add a --preallocation command line option to qemu-img resize which can
>> be used to set the PreallocMode parameter of blk_truncate().
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  qemu-img.c| 33 ++---
>>  qemu-img.texi |  7 ++-
>>  2 files changed, 36 insertions(+), 4 deletions(-)
> 
>> @@ -3553,8 +3566,16 @@ static int img_resize(int argc, char **argv)
>>  goto out;
>>  }
>>  
>> +current_size = blk_getlength(blk);
>> +if (current_size < 0) {
>> +error_report("Failed to inquire current image length: %s",
>> + strerror(-current_size));
>> +ret = -1;
>> +goto out;
>> +}
>> +
>>  if (relative) {
>> -total_size = blk_getlength(blk) + n * relative;
>> +total_size = current_size + n * relative;
> 
> You snuck in a bug fix here (reporting failure, rather than using a
> bogus total_size, if querying the size fails).  Please mention that in
> the commit message.
> 
>> @@ -541,6 +541,11 @@ After using this command to grow a disk image, you must 
>> use file system and
>>  partitioning tools inside the VM to actually begin using the new space on 
>> the
>>  device.
>>  
>> +When growing an image, the @code{--preallocation} option may be used to 
>> specify
>> +how the additional image area should be allocated on the host.  See the 
>> format
>> +description in the @code{NOTES} section which values are allowed.  Using 
>> this
>> +option may result in more data being allocated than necessary.
> 
> Should we tone it down a bit by saying 'slightly more data'?  (We'd
> rather over-estimate than fall short, but our over-estimation will
> probably be < 1% off, and not something drastic like an order of
> magnitude off).

I wanted to be on the safe side for future updates for other formats
(where I'm not sure how well we can do this).

I somehow have the feeling that XFS preallocation may play its game,
too, although I can't seem to trigger it...

Anyway, as long as we can justify, say, ~10 % as "slightly", I'm OK with
adding it.

> With the improved commit message,
> Reviewed-by: Eric Blake 

Will do, and as always: Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 04/16] qemu-img: Expose PreallocMode for resizing

[Eric Blake]

On 05/26/2017 11:55 AM, Max Reitz wrote:
> Add a --preallocation command line option to qemu-img resize which can
> be used to set the PreallocMode parameter of blk_truncate().
> 
> Signed-off-by: Max Reitz 
> ---
>  qemu-img.c| 33 ++---
>  qemu-img.texi |  7 ++-
>  2 files changed, 36 insertions(+), 4 deletions(-)

> @@ -3553,8 +3566,16 @@ static int img_resize(int argc, char **argv)
>  goto out;
>  }
>  
> +current_size = blk_getlength(blk);
> +if (current_size < 0) {
> +error_report("Failed to inquire current image length: %s",
> + strerror(-current_size));
> +ret = -1;
> +goto out;
> +}
> +
>  if (relative) {
> -total_size = blk_getlength(blk) + n * relative;
> +total_size = current_size + n * relative;

You snuck in a bug fix here (reporting failure, rather than using a
bogus total_size, if querying the size fails).  Please mention that in
the commit message.

> @@ -541,6 +541,11 @@ After using this command to grow a disk image, you must 
> use file system and
>  partitioning tools inside the VM to actually begin using the new space on the
>  device.
>  
> +When growing an image, the @code{--preallocation} option may be used to 
> specify
> +how the additional image area should be allocated on the host.  See the 
> format
> +description in the @code{NOTES} section which values are allowed.  Using this
> +option may result in more data being allocated than necessary.

Should we tone it down a bit by saying 'slightly more data'?  (We'd
rather over-estimate than fall short, but our over-estimation will
probably be < 1% off, and not something drastic like an order of
magnitude off).

With the improved commit message,
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 04/16] qemu-img: Expose PreallocMode for resizing

[Max Reitz]

Add a --preallocation command line option to qemu-img resize which can
be used to set the PreallocMode parameter of blk_truncate().

Signed-off-by: Max Reitz 
---
 qemu-img.c| 33 ++---
 qemu-img.texi |  7 ++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 236fd2e..7463424 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,7 @@
 #include "qemu/osdep.h"
 #include "qemu-version.h"
 #include "qapi/error.h"
+#include "qapi/util.h"
 #include "qapi-visit.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qerror.h"
@@ -62,6 +63,7 @@ enum {
 OPTION_NO_DRAIN = 262,
 OPTION_TARGET_IMAGE_OPTS = 263,
 OPTION_SIZE = 264,
+OPTION_PREALLOCATION = 265,
 };
 
 typedef enum OutputFormat {
@@ -3435,9 +3437,10 @@ 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;
+int64_t n, total_size, current_size;
 bool quiet = false;
 BlockBackend *blk = NULL;
+PreallocMode prealloc = PREALLOC_MODE_OFF;
 QemuOpts *param;
 
 static QemuOptsList resize_options = {
@@ -3471,6 +3474,7 @@ static int img_resize(int argc, char **argv)
 {"help", no_argument, 0, 'h'},
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+{"preallocation", required_argument, 0, OPTION_PREALLOCATION},
 {0, 0, 0, 0}
 };
 c = getopt_long(argc, argv, ":f:hq",
@@ -3505,6 +3509,15 @@ static int img_resize(int argc, char **argv)
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
+case OPTION_PREALLOCATION:
+prealloc = qapi_enum_parse(PreallocMode_lookup, optarg,
+   PREALLOC_MODE__MAX, PREALLOC_MODE__MAX,
+   NULL);
+if (prealloc == PREALLOC_MODE__MAX) {
+error_report("Invalid preallocation mode '%s'", optarg);
+return 1;
+}
+break;
 }
 }
 if (optind != argc - 1) {
@@ -3553,8 +3566,16 @@ static int img_resize(int argc, char **argv)
 goto out;
 }
 
+current_size = blk_getlength(blk);
+if (current_size < 0) {
+error_report("Failed to inquire current image length: %s",
+ strerror(-current_size));
+ret = -1;
+goto out;
+}
+
 if (relative) {
-total_size = blk_getlength(blk) + n * relative;
+total_size = current_size + n * relative;
 } else {
 total_size = n;
 }
@@ -3564,7 +3585,13 @@ static int img_resize(int argc, char **argv)
 goto out;
 }
 
-ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, );
+if (total_size <= current_size && prealloc != PREALLOC_MODE_OFF) {
+error_report("Preallocation can only be used for growing images");
+ret = -1;
+goto out;
+}
+
+ret = blk_truncate(blk, total_size, prealloc, );
 if (!ret) {
 qprintf(quiet, "Image resized.\n");
 } else {
diff --git a/qemu-img.texi b/qemu-img.texi
index 3db035f..64bcf87 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -529,7 +529,7 @@ qemu-img rebase -b base.img diff.qcow2
 At this point, @code{modified.img} can be discarded, since
 @code{base.img + diff.qcow2} contains the same information.
 
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [--preallocation=@var{prealloc}] @var{filename} [+ | -]@var{size}
 
 Change the disk image as if it had been created with @var{size}.
 
@@ -541,6 +541,11 @@ After using this command to grow a disk image, you must 
use file system and
 partitioning tools inside the VM to actually begin using the new space on the
 device.
 
+When growing an image, the @code{--preallocation} option may be used to specify
+how the additional image area should be allocated on the host.  See the format
+description in the @code{NOTES} section which values are allowed.  Using this
+option may result in more data being allocated than necessary.
+
 @item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options} 
@var{filename}
 
 Amends the image format specific @var{options} for the image file
-- 
2.9.4