All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
@ 2017-03-03 13:51 Stefan Hajnoczi
  2017-03-03 13:51 ` [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API Stefan Hajnoczi
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 13:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, Alberto Garcia, John Snow, Stefan Hajnoczi

RFCv1:
 * Publishing patch series with just raw support, no qcow2 yet.  Please review
   the command-line interface and let me know if you are happy with this
   approach.

Users and management tools sometimes need to know the size required for a new
disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
Image formats like qcow2 have non-trivial metadata that makes it hard to
estimate the exact size without knowledge of file format internals.

This patch series introduces a new qemu-img subcommand that calculates the
required size for both image creation and conversion scenarios.

The conversion scenario is:

  $ qemu-img max-size -f raw -O qcow2 input.img
  107374184448

Here an existing image file is taken and the output includes the space required
for data from the input image file.

The creation scenario is:

  $ qemu-img max-size -O qcow2 --size 5G
  196688

Stefan Hajnoczi (4):
  block: add bdrv_max_size() API
  raw-format: add bdrv_max_size() support
  qemu-img: add max-size subcommand
  iotests: add test 178 for qemu-img max-size

 include/block/block.h      |   2 +
 include/block/block_int.h  |   2 +
 block.c                    |  37 +++++++++
 block/raw-format.c         |  16 ++++
 qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
 qemu-img-cmds.hx           |   6 ++
 tests/qemu-iotests/178     |  75 +++++++++++++++++
 tests/qemu-iotests/178.out |  25 ++++++
 tests/qemu-iotests/group   |   1 +
 9 files changed, 360 insertions(+)
 create mode 100755 tests/qemu-iotests/178
 create mode 100644 tests/qemu-iotests/178.out

-- 
2.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
@ 2017-03-03 13:51 ` Stefan Hajnoczi
  2017-03-07 10:27   ` Alberto Garcia
  2017-03-03 13:51 ` [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support Stefan Hajnoczi
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 13:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, Alberto Garcia, John Snow, Stefan Hajnoczi

bdrv_max_size() provides a conservative maximum for the size of a new
image.  This information is handy if storage needs to be allocated (e.g.
a SAN or an LVM volume) ahead of time.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/block.h     |  2 ++
 include/block/block_int.h |  2 ++
 block.c                   | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index c7c4a3a..12e1532 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -298,6 +298,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
+uint64_t bdrv_max_size(BlockDriver *drv, QemuOpts *opts,
+                       BlockDriverState *in_bs, Error **errp);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
 void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
 int bdrv_commit(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a57c0bf..0354e22 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -201,6 +201,8 @@ struct BlockDriver {
     int64_t (*bdrv_getlength)(BlockDriverState *bs);
     bool has_variable_length;
     int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+    uint64_t (*bdrv_max_size)(QemuOpts *opts, BlockDriverState *in_bs,
+                              Error **errp);
 
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
diff --git a/block.c b/block.c
index f293ccb..4a228f0 100644
--- a/block.c
+++ b/block.c
@@ -3188,6 +3188,43 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
     return -ENOTSUP;
 }
 
+/*
+ * bdrv_max_size:
+ * @drv: Format driver
+ * @opts: Creation options
+ * @in_bs: Existing image containing data for new image (may be NULL)
+ * @errp: Error object
+ *
+ * Calculate maximum file size required by a new image.  The value is
+ * conservative so actual creation of the image never requires more space than
+ * what is returned by this function.
+ *
+ * Note that this value is the file size including sparse regions, not the
+ * allocated file size.  A new 5 GB raw file therefore has a maximum size of 5
+ * GB, not 0!
+ *
+ * If @in_bs is given then space for allocated clusters and zero clusters
+ * from that image are included in the calculation.  If @opts contains a
+ * backing file that is shared by @in_bs then backing clusters are omitted
+ * from the calculation.
+ *
+ * If @in_bs is NULL then the calculation includes no allocated clusters
+ * unless a preallocation option is given in @opts.
+ *
+ * Returns the maximum size for the new image or 0 on error.
+ */
+uint64_t bdrv_max_size(BlockDriver *drv, QemuOpts *opts,
+                       BlockDriverState *in_bs, Error **errp)
+{
+    if (!drv->bdrv_max_size) {
+        error_setg(errp, "Block driver '%s' does not support max size "
+                   "calculation", drv->format_name);
+        return 0;
+    }
+
+    return drv->bdrv_max_size(opts, in_bs, errp);
+}
+
 /**
  * Return number of sectors on success, -errno on error.
  */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
  2017-03-03 13:51 ` [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API Stefan Hajnoczi
@ 2017-03-03 13:51 ` Stefan Hajnoczi
  2017-03-07 10:32   ` Alberto Garcia
  2017-03-03 13:51 ` [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand Stefan Hajnoczi
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 13:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, Alberto Garcia, John Snow, Stefan Hajnoczi

Maximum size calculation is trivial for the raw format: it's just the
requested image size (because there is no metadata).

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-format.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/block/raw-format.c b/block/raw-format.c
index 86fbc65..f8cb6b3 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -312,6 +312,21 @@ static int64_t raw_getlength(BlockDriverState *bs)
     return s->size;
 }
 
+static uint64_t raw_max_size(QemuOpts *opts, BlockDriverState *in_bs,
+                             Error **errp)
+{
+    if (in_bs) {
+        int64_t size = bdrv_nb_sectors(in_bs);
+        if (size < 0) {
+            error_setg_errno(errp, -size, "Unable to get image size");
+            return 0;
+        }
+        return (uint64_t)size * BDRV_SECTOR_SIZE;
+    }
+
+    return qemu_opt_get_size(opts, "size", 0);
+}
+
 static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     return bdrv_get_info(bs->file->bs, bdi);
@@ -477,6 +492,7 @@ BlockDriver bdrv_raw = {
     .bdrv_truncate        = &raw_truncate,
     .bdrv_getlength       = &raw_getlength,
     .has_variable_length  = true,
+    .bdrv_max_size        = &raw_max_size,
     .bdrv_get_info        = &raw_get_info,
     .bdrv_refresh_limits  = &raw_refresh_limits,
     .bdrv_probe_blocksizes = &raw_probe_blocksizes,
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
  2017-03-03 13:51 ` [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API Stefan Hajnoczi
  2017-03-03 13:51 ` [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support Stefan Hajnoczi
@ 2017-03-03 13:51 ` Stefan Hajnoczi
  2017-03-03 21:56   ` Nir Soffer
  2017-03-03 13:51 ` [Qemu-devel] [RFC 4/4] iotests: add test 178 for qemu-img max-size Stefan Hajnoczi
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 13:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, Alberto Garcia, John Snow, Stefan Hajnoczi

The max-size subcommand calculates the maximum size required by a new
image file.  This can be used by users or management tools that need to
allocate space on an LVM volume, SAN LUN, etc before creating or
converting an image file.

Suggested-by: Maor Lipchuk <mlipchuk@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.c       | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-img-cmds.hx |   6 ++
 2 files changed, 202 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 98b836b..f0a5a85 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -59,6 +59,7 @@ enum {
     OPTION_PATTERN = 260,
     OPTION_FLUSH_INTERVAL = 261,
     OPTION_NO_DRAIN = 262,
+    OPTION_SIZE = 263,
 };
 
 typedef enum OutputFormat {
@@ -4287,6 +4288,201 @@ out:
     return 0;
 }
 
+static int img_max_size(int argc, char **argv)
+{
+    static const struct option long_options[] = {
+        {"help", no_argument, 0, 'h'},
+        {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+        {"object", required_argument, 0, OPTION_OBJECT},
+        {"output", required_argument, 0, OPTION_OUTPUT},
+        {"size", required_argument, 0, OPTION_SIZE},
+        {0, 0, 0, 0}
+    };
+    OutputFormat output_format = OFORMAT_HUMAN;
+    BlockBackend *in_blk = NULL;
+    BlockDriver *drv;
+    const char *filename = NULL;
+    const char *fmt = NULL;
+    const char *out_fmt = "raw";
+    char *options = NULL;
+    char *snapshot_name = NULL;
+    QemuOpts *opts = NULL;
+    QemuOpts *object_opts = NULL;
+    QemuOpts *sn_opts = NULL;
+    QemuOptsList *create_opts = NULL;
+    bool image_opts = false;
+    uint64_t img_size = ~0ULL;
+    uint64_t ret_size;
+    Error *local_err = NULL;
+    int ret = 1;
+    int c;
+
+    while ((c = getopt_long(argc, argv, "hf:O:o:l:",
+                            long_options, NULL)) != -1) {
+        switch (c) {
+        case '?':
+        case 'h':
+            help();
+            break;
+        case 'f':
+            fmt = optarg;
+            break;
+        case 'O':
+            out_fmt = optarg;
+            break;
+        case 'o':
+            if (!is_valid_option_list(optarg)) {
+                error_report("Invalid option list: %s", optarg);
+                goto fail;
+            }
+            if (!options) {
+                options = g_strdup(optarg);
+            } else {
+                char *old_options = options;
+                options = g_strdup_printf("%s,%s", options, optarg);
+                g_free(old_options);
+            }
+            break;
+        case 'l':
+            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
+                sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
+                                                  optarg, false);
+                if (!sn_opts) {
+                    error_report("Failed in parsing snapshot param '%s'",
+                                 optarg);
+                    goto fail;
+                }
+            } else {
+                snapshot_name = optarg;
+            }
+            break;
+        case OPTION_OBJECT:
+            object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
+                                                  optarg, true);
+            if (!object_opts) {
+                goto fail;
+            }
+            break;
+        case OPTION_IMAGE_OPTS:
+            image_opts = true;
+            break;
+        case OPTION_OUTPUT:
+            if (!strcmp(optarg, "json")) {
+                output_format = OFORMAT_JSON;
+            } else if (!strcmp(optarg, "human")) {
+                output_format = OFORMAT_HUMAN;
+            } else {
+                error_report("--output must be used with human or json "
+                             "as argument.");
+                goto fail;
+            }
+            break;
+        case OPTION_SIZE:
+        {
+            int64_t sval;
+
+            sval = cvtnum(optarg);
+            if (sval < 0) {
+                if (sval == -ERANGE) {
+                    error_report("Image size must be less than 8 EiB!");
+                } else {
+                    error_report("Invalid image size specified! You may use "
+                                 "k, M, G, T, P or E suffixes for ");
+                    error_report("kilobytes, megabytes, gigabytes, terabytes, "
+                                 "petabytes and exabytes.");
+                }
+                goto fail;
+            }
+            img_size = (uint64_t)sval;
+        }
+        break;
+        }
+    }
+
+    if (qemu_opts_foreach(&qemu_object_opts,
+                          user_creatable_add_opts_foreach,
+                          NULL, NULL)) {
+        goto fail;
+    }
+
+    if (argc - optind > 1) {
+        error_report("At most one filename argument is allowed.");
+        goto fail;
+    } else if (argc - optind == 1) {
+        filename = argv[optind];
+    }
+
+    if (!filename &&
+        (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
+        error_report("--object, --image-opts, -f, and -l "
+                     "require a filename argument.");
+        goto fail;
+    }
+    if (filename && img_size != ~0ULL) {
+        error_report("--size N cannot be used together with a filename.");
+        goto fail;
+    }
+    if (!filename && img_size == ~0ULL) {
+        error_report("Either --size N or one filename must be specified.");
+        goto fail;
+    }
+
+    if (filename) {
+        in_blk = img_open(image_opts, filename, fmt, 0, false, false);
+        if (!in_blk) {
+            goto fail;
+        }
+    }
+
+    drv = bdrv_find_format(out_fmt);
+    if (!drv) {
+        error_report("Unknown file format '%s'", out_fmt);
+        goto fail;
+    }
+    if (!drv->create_opts) {
+        error_report("Format driver '%s' does not support image creation",
+                     drv->format_name);
+        goto fail;
+    }
+
+    create_opts = qemu_opts_append(create_opts, drv->create_opts);
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    if (options) {
+        qemu_opts_do_parse(opts, options, NULL, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_report("Invalid options for file format '%s'", out_fmt);
+            goto fail;
+        }
+    }
+    if (img_size != ~0ULL) {
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
+    }
+
+    ret_size = bdrv_max_size(drv, opts, in_blk ? blk_bs(in_blk) : NULL,
+                             &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        goto fail;
+    }
+
+    if (output_format == OFORMAT_HUMAN) {
+        printf("%" PRIu64 "\n", ret_size);
+    } else {
+        printf("{ \"size\": %" PRIu64 " }\n", ret_size);
+    }
+
+    ret = 0;
+
+fail:
+    qemu_opts_del(object_opts);
+    qemu_opts_del(opts);
+    qemu_opts_del(sn_opts);
+    qemu_opts_free(create_opts);
+    g_free(options);
+    blk_unref(in_blk);
+    return ret;
+}
 
 static const img_cmd_t img_cmds[] = {
 #define DEF(option, callback, arg_string)        \
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 9c9702c..80997f9 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -87,3 +87,9 @@ STEXI
 @item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
 @end table
 ETEXI
+
+DEF("max-size", img_max_size,
+"max-size [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
+STEXI
+TODO max-size documentation
+ETEXI
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [RFC 4/4] iotests: add test 178 for qemu-img max-size
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2017-03-03 13:51 ` [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand Stefan Hajnoczi
@ 2017-03-03 13:51 ` Stefan Hajnoczi
  2017-03-03 20:48 ` [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand John Snow
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 13:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, Alberto Garcia, John Snow, Stefan Hajnoczi

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/178     | 75 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/178.out | 25 ++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100755 tests/qemu-iotests/178
 create mode 100644 tests/qemu-iotests/178.out

diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
new file mode 100755
index 0000000..313981b
--- /dev/null
+++ b/tests/qemu-iotests/178
@@ -0,0 +1,75 @@
+#!/bin/bash
+#
+# qemu-img max-size tests
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=stefanha@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+echo "Input validation"
+echo
+
+_make_test_img 1G
+
+$QEMU_IMG max-size # missing arguments
+$QEMU_IMG max-size --size 2G "$TEST_IMG" # only one allowed
+$QEMU_IMG max-size "$TEST_IMG" a # only one filename allowed
+$QEMU_IMG max-size --object secret,id=sec0,data=MTIzNDU2,format=base64 # missing filename
+$QEMU_IMG max-size --image-opts # missing filename
+$QEMU_IMG max-size -f qcow2 # missing filename
+$QEMU_IMG max-size -l snap1 # missing filename
+$QEMU_IMG max-size -o , # invalid option list
+$QEMU_IMG max-size -l snapshot.foo # invalid snapshot option
+$QEMU_IMG max-size --output foo # invalid output format
+$QEMU_IMG max-size --size -1 # invalid image size
+$QEMU_IMG max-size -O foo "$TEST_IMG" # unknown image file format
+
+echo
+echo "Size calculation"
+echo
+
+for ofmt in human json; do
+    $QEMU_IMG max-size --output=$ofmt -O "$IMGFMT" --size 2G
+    $QEMU_IMG max-size --output=$ofmt -f "$IMGFMT" -O "$IMGFMT" "$TEST_IMG"
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/178.out b/tests/qemu-iotests/178.out
new file mode 100644
index 0000000..6750ad6
--- /dev/null
+++ b/tests/qemu-iotests/178.out
@@ -0,0 +1,25 @@
+QA output created by 178
+Input validation
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Either --size N or one filename must be specified.
+qemu-img: --size N cannot be used together with a filename.
+qemu-img: At most one filename argument is allowed.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: Invalid option list: ,
+qemu-img: Invalid parameter 'snapshot.foo'
+qemu-img: Failed in parsing snapshot param 'snapshot.foo'
+qemu-img: --output must be used with human or json as argument.
+qemu-img: Image size must be less than 8 EiB!
+qemu-img: Unknown file format 'foo'
+
+Size calculation
+
+2147483648
+1073741824
+{ "size": 2147483648 }
+{ "size": 1073741824 }
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1f4bf03..846f962 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -168,3 +168,4 @@
 173 rw auto
 174 auto
 175 auto quick
+178 auto quick
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
                   ` (3 preceding siblings ...)
  2017-03-03 13:51 ` [Qemu-devel] [RFC 4/4] iotests: add test 178 for qemu-img max-size Stefan Hajnoczi
@ 2017-03-03 20:48 ` John Snow
  2017-03-03 21:38 ` Nir Soffer
  2017-03-07 10:36 ` Daniel P. Berrange
  6 siblings, 0 replies; 19+ messages in thread
From: John Snow @ 2017-03-03 20:48 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Nir Soffer, Maor Lipchuk, Alberto Garcia



On 03/03/2017 08:51 AM, Stefan Hajnoczi wrote:
> RFCv1:
>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>    the command-line interface and let me know if you are happy with this
>    approach.
> 
> Users and management tools sometimes need to know the size required for a new
> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
> Image formats like qcow2 have non-trivial metadata that makes it hard to
> estimate the exact size without knowledge of file format internals.
> 
> This patch series introduces a new qemu-img subcommand that calculates the
> required size for both image creation and conversion scenarios.
> 
> The conversion scenario is:
> 
>   $ qemu-img max-size -f raw -O qcow2 input.img
>   107374184448
> 
> Here an existing image file is taken and the output includes the space required
> for data from the input image file.
> 
> The creation scenario is:
> 
>   $ qemu-img max-size -O qcow2 --size 5G
>   196688
> 

It looks sane to me, implementation looks clean enough.
Thanks!

> Stefan Hajnoczi (4):
>   block: add bdrv_max_size() API
>   raw-format: add bdrv_max_size() support
>   qemu-img: add max-size subcommand
>   iotests: add test 178 for qemu-img max-size
> 
>  include/block/block.h      |   2 +
>  include/block/block_int.h  |   2 +
>  block.c                    |  37 +++++++++
>  block/raw-format.c         |  16 ++++
>  qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
>  qemu-img-cmds.hx           |   6 ++
>  tests/qemu-iotests/178     |  75 +++++++++++++++++
>  tests/qemu-iotests/178.out |  25 ++++++
>  tests/qemu-iotests/group   |   1 +
>  9 files changed, 360 insertions(+)
>  create mode 100755 tests/qemu-iotests/178
>  create mode 100644 tests/qemu-iotests/178.out
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
                   ` (4 preceding siblings ...)
  2017-03-03 20:48 ` [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand John Snow
@ 2017-03-03 21:38 ` Nir Soffer
  2017-03-03 22:02   ` John Snow
  2017-03-07 10:36 ` Daniel P. Berrange
  6 siblings, 1 reply; 19+ messages in thread
From: Nir Soffer @ 2017-03-03 21:38 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Eric Blake, Kevin Wolf, Maor Lipchuk,
	Daniel P. Berrange, Alberto Garcia, John Snow

On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> RFCv1:
>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>    the command-line interface and let me know if you are happy with this
>    approach.
>
> Users and management tools sometimes need to know the size required for a new
> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
> Image formats like qcow2 have non-trivial metadata that makes it hard to
> estimate the exact size without knowledge of file format internals.
>
> This patch series introduces a new qemu-img subcommand that calculates the
> required size for both image creation and conversion scenarios.
>
> The conversion scenario is:
>
>   $ qemu-img max-size -f raw -O qcow2 input.img
>   107374184448

Isn't this the minimal size required to convert input.img?

>
> Here an existing image file is taken and the output includes the space required
> for data from the input image file.
>
> The creation scenario is:
>
>   $ qemu-img max-size -O qcow2 --size 5G
>   196688

Again, this is the minimal size.

So maybe use min-size?

Or:

    qemu-img measure -f raw -O qcow2 input.img

Works nicely with other verbs like create, convert, check.

Now about the return value, do we want to return both the minimum size
and the maximum size?

For ovirt use case, we currently calculate the maximum size by multiplying
by 1.1. We use this when doing automatic extending of ovirt thin provisioned
disk. We start with 1G lv, and extend it each time it becomes full, stopping
when we reach virtual size * 1.1. Using more accurate calculation instead
can be nicer.

So we can retrun:

{
    "min-size": 196688,
    "max-size": 5905580032
}

Anyway thanks for working on this!

>
> Stefan Hajnoczi (4):
>   block: add bdrv_max_size() API
>   raw-format: add bdrv_max_size() support
>   qemu-img: add max-size subcommand
>   iotests: add test 178 for qemu-img max-size
>
>  include/block/block.h      |   2 +
>  include/block/block_int.h  |   2 +
>  block.c                    |  37 +++++++++
>  block/raw-format.c         |  16 ++++
>  qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
>  qemu-img-cmds.hx           |   6 ++
>  tests/qemu-iotests/178     |  75 +++++++++++++++++
>  tests/qemu-iotests/178.out |  25 ++++++
>  tests/qemu-iotests/group   |   1 +
>  9 files changed, 360 insertions(+)
>  create mode 100755 tests/qemu-iotests/178
>  create mode 100644 tests/qemu-iotests/178.out
>
> --
> 2.9.3
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand
  2017-03-03 13:51 ` [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand Stefan Hajnoczi
@ 2017-03-03 21:56   ` Nir Soffer
  2017-03-10  4:42     ` Stefan Hajnoczi
  0 siblings, 1 reply; 19+ messages in thread
From: Nir Soffer @ 2017-03-03 21:56 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Eric Blake, Kevin Wolf, Maor Lipchuk,
	Daniel P. Berrange, Alberto Garcia, John Snow

On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The max-size subcommand calculates the maximum size required by a new
> image file.  This can be used by users or management tools that need to
> allocate space on an LVM volume, SAN LUN, etc before creating or
> converting an image file.
>
> Suggested-by: Maor Lipchuk <mlipchuk@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.c       | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-img-cmds.hx |   6 ++
>  2 files changed, 202 insertions(+)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 98b836b..f0a5a85 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -59,6 +59,7 @@ enum {
>      OPTION_PATTERN = 260,
>      OPTION_FLUSH_INTERVAL = 261,
>      OPTION_NO_DRAIN = 262,
> +    OPTION_SIZE = 263,
>  };
>
>  typedef enum OutputFormat {
> @@ -4287,6 +4288,201 @@ out:
>      return 0;
>  }
>
> +static int img_max_size(int argc, char **argv)
> +{
> +    static const struct option long_options[] = {
> +        {"help", no_argument, 0, 'h'},
> +        {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> +        {"object", required_argument, 0, OPTION_OBJECT},
> +        {"output", required_argument, 0, OPTION_OUTPUT},
> +        {"size", required_argument, 0, OPTION_SIZE},
> +        {0, 0, 0, 0}
> +    };
> +    OutputFormat output_format = OFORMAT_HUMAN;
> +    BlockBackend *in_blk = NULL;
> +    BlockDriver *drv;
> +    const char *filename = NULL;
> +    const char *fmt = NULL;
> +    const char *out_fmt = "raw";
> +    char *options = NULL;
> +    char *snapshot_name = NULL;
> +    QemuOpts *opts = NULL;
> +    QemuOpts *object_opts = NULL;
> +    QemuOpts *sn_opts = NULL;
> +    QemuOptsList *create_opts = NULL;
> +    bool image_opts = false;
> +    uint64_t img_size = ~0ULL;
> +    uint64_t ret_size;
> +    Error *local_err = NULL;
> +    int ret = 1;
> +    int c;
> +
> +    while ((c = getopt_long(argc, argv, "hf:O:o:l:",
> +                            long_options, NULL)) != -1) {
> +        switch (c) {
> +        case '?':
> +        case 'h':
> +            help();
> +            break;
> +        case 'f':
> +            fmt = optarg;
> +            break;
> +        case 'O':
> +            out_fmt = optarg;
> +            break;
> +        case 'o':
> +            if (!is_valid_option_list(optarg)) {
> +                error_report("Invalid option list: %s", optarg);
> +                goto fail;
> +            }
> +            if (!options) {
> +                options = g_strdup(optarg);
> +            } else {
> +                char *old_options = options;
> +                options = g_strdup_printf("%s,%s", options, optarg);
> +                g_free(old_options);
> +            }
> +            break;
> +        case 'l':
> +            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
> +                sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
> +                                                  optarg, false);
> +                if (!sn_opts) {
> +                    error_report("Failed in parsing snapshot param '%s'",
> +                                 optarg);
> +                    goto fail;
> +                }
> +            } else {
> +                snapshot_name = optarg;
> +            }
> +            break;
> +        case OPTION_OBJECT:
> +            object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> +                                                  optarg, true);
> +            if (!object_opts) {
> +                goto fail;
> +            }
> +            break;
> +        case OPTION_IMAGE_OPTS:
> +            image_opts = true;
> +            break;
> +        case OPTION_OUTPUT:
> +            if (!strcmp(optarg, "json")) {
> +                output_format = OFORMAT_JSON;
> +            } else if (!strcmp(optarg, "human")) {
> +                output_format = OFORMAT_HUMAN;
> +            } else {
> +                error_report("--output must be used with human or json "
> +                             "as argument.");
> +                goto fail;
> +            }
> +            break;
> +        case OPTION_SIZE:
> +        {
> +            int64_t sval;
> +
> +            sval = cvtnum(optarg);
> +            if (sval < 0) {
> +                if (sval == -ERANGE) {
> +                    error_report("Image size must be less than 8 EiB!");
> +                } else {
> +                    error_report("Invalid image size specified! You may use "
> +                                 "k, M, G, T, P or E suffixes for ");
> +                    error_report("kilobytes, megabytes, gigabytes, terabytes, "
> +                                 "petabytes and exabytes.");
> +                }
> +                goto fail;
> +            }
> +            img_size = (uint64_t)sval;
> +        }
> +        break;
> +        }
> +    }
> +
> +    if (qemu_opts_foreach(&qemu_object_opts,
> +                          user_creatable_add_opts_foreach,
> +                          NULL, NULL)) {
> +        goto fail;
> +    }
> +
> +    if (argc - optind > 1) {
> +        error_report("At most one filename argument is allowed.");
> +        goto fail;
> +    } else if (argc - optind == 1) {
> +        filename = argv[optind];
> +    }
> +
> +    if (!filename &&
> +        (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
> +        error_report("--object, --image-opts, -f, and -l "
> +                     "require a filename argument.");
> +        goto fail;
> +    }
> +    if (filename && img_size != ~0ULL) {
> +        error_report("--size N cannot be used together with a filename.");
> +        goto fail;
> +    }
> +    if (!filename && img_size == ~0ULL) {
> +        error_report("Either --size N or one filename must be specified.");
> +        goto fail;
> +    }
> +
> +    if (filename) {
> +        in_blk = img_open(image_opts, filename, fmt, 0, false, false);
> +        if (!in_blk) {
> +            goto fail;
> +        }
> +    }
> +
> +    drv = bdrv_find_format(out_fmt);
> +    if (!drv) {
> +        error_report("Unknown file format '%s'", out_fmt);
> +        goto fail;
> +    }
> +    if (!drv->create_opts) {
> +        error_report("Format driver '%s' does not support image creation",
> +                     drv->format_name);
> +        goto fail;
> +    }
> +
> +    create_opts = qemu_opts_append(create_opts, drv->create_opts);
> +    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> +    if (options) {
> +        qemu_opts_do_parse(opts, options, NULL, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +            error_report("Invalid options for file format '%s'", out_fmt);
> +            goto fail;
> +        }
> +    }
> +    if (img_size != ~0ULL) {
> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
> +    }
> +
> +    ret_size = bdrv_max_size(drv, opts, in_blk ? blk_bs(in_blk) : NULL,
> +                             &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +        goto fail;
> +    }
> +
> +    if (output_format == OFORMAT_HUMAN) {
> +        printf("%" PRIu64 "\n", ret_size);
> +    } else {
> +        printf("{ \"size\": %" PRIu64 " }\n", ret_size);
> +    }
> +
> +    ret = 0;
> +
> +fail:

This looks more like out: to me, since we use it both for normal
and abnormal flows.

> +    qemu_opts_del(object_opts);
> +    qemu_opts_del(opts);
> +    qemu_opts_del(sn_opts);
> +    qemu_opts_free(create_opts);
> +    g_free(options);
> +    blk_unref(in_blk);
> +    return ret;
> +}
>
>  static const img_cmd_t img_cmds[] = {
>  #define DEF(option, callback, arg_string)        \
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 9c9702c..80997f9 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -87,3 +87,9 @@ STEXI
>  @item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
>  @end table
>  ETEXI
> +
> +DEF("max-size", img_max_size,
> +"max-size [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
> +STEXI
> +TODO max-size documentation
> +ETEXI
> --
> 2.9.3
>

Nir

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 21:38 ` Nir Soffer
@ 2017-03-03 22:02   ` John Snow
  2017-03-03 22:15     ` Nir Soffer
  0 siblings, 1 reply; 19+ messages in thread
From: John Snow @ 2017-03-03 22:02 UTC (permalink / raw)
  To: Nir Soffer, Stefan Hajnoczi
  Cc: qemu-devel, Eric Blake, Kevin Wolf, Maor Lipchuk,
	Daniel P. Berrange, Alberto Garcia



On 03/03/2017 04:38 PM, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>
>> RFCv1:
>>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>>    the command-line interface and let me know if you are happy with this
>>    approach.
>>
>> Users and management tools sometimes need to know the size required for a new
>> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
>> Image formats like qcow2 have non-trivial metadata that makes it hard to
>> estimate the exact size without knowledge of file format internals.
>>
>> This patch series introduces a new qemu-img subcommand that calculates the
>> required size for both image creation and conversion scenarios.
>>
>> The conversion scenario is:
>>
>>   $ qemu-img max-size -f raw -O qcow2 input.img
>>   107374184448
> 
> Isn't this the minimal size required to convert input.img?
> 

It's an upper bound for the property being measured, which is current
allocation size, not maximum potential size after growth.

>>
>> Here an existing image file is taken and the output includes the space required
>> for data from the input image file.
>>
>> The creation scenario is:
>>
>>   $ qemu-img max-size -O qcow2 --size 5G
>>   196688
> 
> Again, this is the minimal size.
> 
> So maybe use min-size?
> 
> Or:
> 
>     qemu-img measure -f raw -O qcow2 input.img
> 
> Works nicely with other verbs like create, convert, check.
> 

Measure what? This is strictly less descriptive even if "max-size" isn't
a verb.

> Now about the return value, do we want to return both the minimum size
> and the maximum size?
> 
> For ovirt use case, we currently calculate the maximum size by multiplying
> by 1.1. We use this when doing automatic extending of ovirt thin provisioned
> disk. We start with 1G lv, and extend it each time it becomes full, stopping
> when we reach virtual size * 1.1. Using more accurate calculation instead
> can be nicer.
> 
> So we can retrun:
> 
> {
>     "min-size": 196688,
>     "max-size": 5905580032
> }
> 
> Anyway thanks for working on this!
> 

It sounds like you want something different from what was intuited by
Maor Lipchuck. There are two things to estimate:

(A) An estimate of the possible size of an image after conversion to a
different format, and
(B) An estimate of the possible size after full allocation.

I got the sense that Maor was asking for (A), but perhaps I am wrong
about that. However, both are "maximums" in different senses.

--js

>>
>> Stefan Hajnoczi (4):
>>   block: add bdrv_max_size() API
>>   raw-format: add bdrv_max_size() support
>>   qemu-img: add max-size subcommand
>>   iotests: add test 178 for qemu-img max-size
>>
>>  include/block/block.h      |   2 +
>>  include/block/block_int.h  |   2 +
>>  block.c                    |  37 +++++++++
>>  block/raw-format.c         |  16 ++++
>>  qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
>>  qemu-img-cmds.hx           |   6 ++
>>  tests/qemu-iotests/178     |  75 +++++++++++++++++
>>  tests/qemu-iotests/178.out |  25 ++++++
>>  tests/qemu-iotests/group   |   1 +
>>  9 files changed, 360 insertions(+)
>>  create mode 100755 tests/qemu-iotests/178
>>  create mode 100644 tests/qemu-iotests/178.out
>>
>> --
>> 2.9.3
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 22:02   ` John Snow
@ 2017-03-03 22:15     ` Nir Soffer
  2017-03-04  0:14       ` Nir Soffer
  2017-03-10  7:58       ` Stefan Hajnoczi
  0 siblings, 2 replies; 19+ messages in thread
From: Nir Soffer @ 2017-03-03 22:15 UTC (permalink / raw)
  To: John Snow
  Cc: Nir Soffer, Stefan Hajnoczi, Kevin Wolf, qemu-devel,
	Maor Lipchuk, Alberto Garcia

On Sat, Mar 4, 2017 at 12:02 AM, John Snow <jsnow@redhat.com> wrote:
>
>
> On 03/03/2017 04:38 PM, Nir Soffer wrote:
>> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>
>>> RFCv1:
>>>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>>>    the command-line interface and let me know if you are happy with this
>>>    approach.
>>>
>>> Users and management tools sometimes need to know the size required for a new
>>> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
>>> Image formats like qcow2 have non-trivial metadata that makes it hard to
>>> estimate the exact size without knowledge of file format internals.
>>>
>>> This patch series introduces a new qemu-img subcommand that calculates the
>>> required size for both image creation and conversion scenarios.
>>>
>>> The conversion scenario is:
>>>
>>>   $ qemu-img max-size -f raw -O qcow2 input.img
>>>   107374184448
>>
>> Isn't this the minimal size required to convert input.img?
>>
>
> It's an upper bound for the property being measured, which is current
> allocation size, not maximum potential size after growth.

>From my point of view, this is the minimal size you must allocate if you
want to convert the image to logical volume.

>
>>>
>>> Here an existing image file is taken and the output includes the space required
>>> for data from the input image file.
>>>
>>> The creation scenario is:
>>>
>>>   $ qemu-img max-size -O qcow2 --size 5G
>>>   196688
>>
>> Again, this is the minimal size.
>>
>> So maybe use min-size?
>>
>> Or:
>>
>>     qemu-img measure -f raw -O qcow2 input.img
>>
>> Works nicely with other verbs like create, convert, check.
>>
>
> Measure what? This is strictly less descriptive even if "max-size" isn't
> a verb.

measure-size?

>> Now about the return value, do we want to return both the minimum size
>> and the maximum size?
>>
>> For ovirt use case, we currently calculate the maximum size by multiplying
>> by 1.1. We use this when doing automatic extending of ovirt thin provisioned
>> disk. We start with 1G lv, and extend it each time it becomes full, stopping
>> when we reach virtual size * 1.1. Using more accurate calculation instead
>> can be nicer.
>>
>> So we can retrun:
>>
>> {
>>     "min-size": 196688,
>>     "max-size": 5905580032
>> }
>>
>> Anyway thanks for working on this!
>>
>
> It sounds like you want something different from what was intuited by
> Maor Lipchuck. There are two things to estimate:
>
> (A) An estimate of the possible size of an image after conversion to a
> different format, and
> (B) An estimate of the possible size after full allocation.
>
> I got the sense that Maor was asking for (A), but perhaps I am wrong
> about that. However, both are "maximums" in different senses.

Both are minimum when you have to allocate the space.

Maor ask about A because he is working on fixing allocation when
converting existing files, but we also have other use cases like B.

Nir

>
> --js
>
>>>
>>> Stefan Hajnoczi (4):
>>>   block: add bdrv_max_size() API
>>>   raw-format: add bdrv_max_size() support
>>>   qemu-img: add max-size subcommand
>>>   iotests: add test 178 for qemu-img max-size
>>>
>>>  include/block/block.h      |   2 +
>>>  include/block/block_int.h  |   2 +
>>>  block.c                    |  37 +++++++++
>>>  block/raw-format.c         |  16 ++++
>>>  qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
>>>  qemu-img-cmds.hx           |   6 ++
>>>  tests/qemu-iotests/178     |  75 +++++++++++++++++
>>>  tests/qemu-iotests/178.out |  25 ++++++
>>>  tests/qemu-iotests/group   |   1 +
>>>  9 files changed, 360 insertions(+)
>>>  create mode 100755 tests/qemu-iotests/178
>>>  create mode 100644 tests/qemu-iotests/178.out
>>>
>>> --
>>> 2.9.3
>>>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 22:15     ` Nir Soffer
@ 2017-03-04  0:14       ` Nir Soffer
  2017-03-10  7:58       ` Stefan Hajnoczi
  1 sibling, 0 replies; 19+ messages in thread
From: Nir Soffer @ 2017-03-04  0:14 UTC (permalink / raw)
  To: John Snow
  Cc: Nir Soffer, Stefan Hajnoczi, Kevin Wolf, qemu-devel,
	Maor Lipchuk, Alberto Garcia

On Sat, Mar 4, 2017 at 12:15 AM, Nir Soffer <nirsof@gmail.com> wrote:
> On Sat, Mar 4, 2017 at 12:02 AM, John Snow <jsnow@redhat.com> wrote:
>>
>>
>> On 03/03/2017 04:38 PM, Nir Soffer wrote:
>>> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>>
>>>> RFCv1:
>>>>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>>>>    the command-line interface and let me know if you are happy with this
>>>>    approach.
>>>>
>>>> Users and management tools sometimes need to know the size required for a new
>>>> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
>>>> Image formats like qcow2 have non-trivial metadata that makes it hard to
>>>> estimate the exact size without knowledge of file format internals.
>>>>
>>>> This patch series introduces a new qemu-img subcommand that calculates the
>>>> required size for both image creation and conversion scenarios.
>>>>
>>>> The conversion scenario is:
>>>>
>>>>   $ qemu-img max-size -f raw -O qcow2 input.img
>>>>   107374184448
>>>
>>> Isn't this the minimal size required to convert input.img?
>>>
>>
>> It's an upper bound for the property being measured, which is current
>> allocation size, not maximum potential size after growth.
>
> From my point of view, this is the minimal size you must allocate if you
> want to convert the image to logical volume.

Thinking more about this, max-size is correct for this use case, the
maximum size of the image, used as the minimal allocation.

>
>>
>>>>
>>>> Here an existing image file is taken and the output includes the space required
>>>> for data from the input image file.
>>>>
>>>> The creation scenario is:
>>>>
>>>>   $ qemu-img max-size -O qcow2 --size 5G
>>>>   196688
>>>
>>> Again, this is the minimal size.
>>>
>>> So maybe use min-size?
>>>
>>> Or:
>>>
>>>     qemu-img measure -f raw -O qcow2 input.img
>>>
>>> Works nicely with other verbs like create, convert, check.
>>>
>>
>> Measure what? This is strictly less descriptive even if "max-size" isn't
>> a verb.
>
> measure-size?
>
>>> Now about the return value, do we want to return both the minimum size
>>> and the maximum size?
>>>
>>> For ovirt use case, we currently calculate the maximum size by multiplying
>>> by 1.1. We use this when doing automatic extending of ovirt thin provisioned
>>> disk. We start with 1G lv, and extend it each time it becomes full, stopping
>>> when we reach virtual size * 1.1. Using more accurate calculation instead
>>> can be nicer.
>>>
>>> So we can retrun:
>>>
>>> {
>>>     "min-size": 196688,
>>>     "max-size": 5905580032
>>> }
>>>
>>> Anyway thanks for working on this!
>>>
>>
>> It sounds like you want something different from what was intuited by
>> Maor Lipchuck. There are two things to estimate:
>>
>> (A) An estimate of the possible size of an image after conversion to a
>> different format, and
>> (B) An estimate of the possible size after full allocation.
>>
>> I got the sense that Maor was asking for (A), but perhaps I am wrong
>> about that. However, both are "maximums" in different senses.
>
> Both are minimum when you have to allocate the space.
>
> Maor ask about A because he is working on fixing allocation when
> converting existing files, but we also have other use cases like B.
>
> Nir
>
>>
>> --js
>>
>>>>
>>>> Stefan Hajnoczi (4):
>>>>   block: add bdrv_max_size() API
>>>>   raw-format: add bdrv_max_size() support
>>>>   qemu-img: add max-size subcommand
>>>>   iotests: add test 178 for qemu-img max-size
>>>>
>>>>  include/block/block.h      |   2 +
>>>>  include/block/block_int.h  |   2 +
>>>>  block.c                    |  37 +++++++++
>>>>  block/raw-format.c         |  16 ++++
>>>>  qemu-img.c                 | 196 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  qemu-img-cmds.hx           |   6 ++
>>>>  tests/qemu-iotests/178     |  75 +++++++++++++++++
>>>>  tests/qemu-iotests/178.out |  25 ++++++
>>>>  tests/qemu-iotests/group   |   1 +
>>>>  9 files changed, 360 insertions(+)
>>>>  create mode 100755 tests/qemu-iotests/178
>>>>  create mode 100644 tests/qemu-iotests/178.out
>>>>
>>>> --
>>>> 2.9.3
>>>>
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API
  2017-03-03 13:51 ` [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API Stefan Hajnoczi
@ 2017-03-07 10:27   ` Alberto Garcia
  0 siblings, 0 replies; 19+ messages in thread
From: Alberto Garcia @ 2017-03-07 10:27 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, John Snow

On Fri 03 Mar 2017 02:51:47 PM CET, Stefan Hajnoczi wrote:
> bdrv_max_size() provides a conservative maximum for the size of a new
> image.  This information is handy if storage needs to be allocated (e.g.
> a SAN or an LVM volume) ahead of time.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support
  2017-03-03 13:51 ` [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support Stefan Hajnoczi
@ 2017-03-07 10:32   ` Alberto Garcia
  2017-03-10  4:33     ` Stefan Hajnoczi
  0 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2017-03-07 10:32 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Eric Blake, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
	Nir Soffer, John Snow

On Fri 03 Mar 2017 02:51:48 PM CET, Stefan Hajnoczi wrote:

> +static uint64_t raw_max_size(QemuOpts *opts, BlockDriverState *in_bs,
> +                             Error **errp)
> +{
> +    if (in_bs) {
> +        int64_t size = bdrv_nb_sectors(in_bs);
> +        if (size < 0) {
> +            error_setg_errno(errp, -size, "Unable to get image size");
> +            return 0;
> +        }
> +        return (uint64_t)size * BDRV_SECTOR_SIZE;
> +    }

Why not use bdrv_getlength() directly? It gives you the size in bytes.

Berto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
                   ` (5 preceding siblings ...)
  2017-03-03 21:38 ` Nir Soffer
@ 2017-03-07 10:36 ` Daniel P. Berrange
  2017-03-07 12:11   ` Alberto Garcia
  6 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrange @ 2017-03-07 10:36 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Eric Blake, Kevin Wolf, Maor Lipchuk, Nir Soffer,
	Alberto Garcia, John Snow

On Fri, Mar 03, 2017 at 09:51:46PM +0800, Stefan Hajnoczi wrote:
> RFCv1:
>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
>    the command-line interface and let me know if you are happy with this
>    approach.
> 
> Users and management tools sometimes need to know the size required for a new
> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
> Image formats like qcow2 have non-trivial metadata that makes it hard to
> estimate the exact size without knowledge of file format internals.
> 
> This patch series introduces a new qemu-img subcommand that calculates the
> required size for both image creation and conversion scenarios.
> 
> The conversion scenario is:
> 
>   $ qemu-img max-size -f raw -O qcow2 input.img
>   107374184448
> 
> Here an existing image file is taken and the output includes the space required
> for data from the input image file.
> 
> The creation scenario is:
> 
>   $ qemu-img max-size -O qcow2 --size 5G
>   196688

Hmm, so that appears to be indicating the amount of physical space that
a qcow2 image would take up before any data has been written to it.

That's not actually what I was thinking. I would like to know the maximum
possible physical space that a 5G qcow2 image would take up once data is
written to every sector. Obviously this is impossible to say if you allow
for internal snapshots, but I think it is fine to say that we ignore
internal snapshots for purposes of this command.

IOW, I would expect  something like:

$ qemu-img max-size -O qcow2 --size 5G
5905580032

(I just blindly assumed qcow2 metadata is a 10% overhead for sake of
illustrating the number here)


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-07 10:36 ` Daniel P. Berrange
@ 2017-03-07 12:11   ` Alberto Garcia
  2017-03-07 12:25     ` Daniel P. Berrange
  0 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2017-03-07 12:11 UTC (permalink / raw)
  To: Daniel P. Berrange, Stefan Hajnoczi
  Cc: qemu-devel, Eric Blake, Kevin Wolf, Maor Lipchuk, Nir Soffer, John Snow

On Tue 07 Mar 2017 11:36:54 AM CET, Daniel P. Berrange wrote:
>> The creation scenario is:
>> 
>>   $ qemu-img max-size -O qcow2 --size 5G
>>   196688
>
> Hmm, so that appears to be indicating the amount of physical space
> that a qcow2 image would take up before any data has been written to
> it.
>
> That's not actually what I was thinking. I would like to know the
> maximum possible physical space that a 5G qcow2 image would take up
> once data is written to every sector. Obviously this is impossible to
> say if you allow for internal snapshots, but I think it is fine to say
> that we ignore internal snapshots for purposes of this command.

We have clearly two different use cases here, although I wonder how
useful the one that you are describing is. After all the maximum size of
a fully allocated imaged is always going to be the virtual size plus a
small overhead for the metadata. I haven't made the numbers for all
cases, but I'll take the risk and say it's always going to be really
small (the 10% you use to illustrate your point is way too much).

A fully allocated 1TB qcow2 image needs less than 200MB of metadata
(that's 0.02% of the total size). If we reduce the cluster size to its
minimum (512 bytes) then it's around 20GB (still ~2% of the total size).

Computing the size that you need to convert the data that is currently
allocated is a different matter, because it depends a lot on the
scenario: whether you are using preallocation, or you have backing
images, etc.

Berto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-07 12:11   ` Alberto Garcia
@ 2017-03-07 12:25     ` Daniel P. Berrange
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2017-03-07 12:25 UTC (permalink / raw)
  To: Alberto Garcia
  Cc: Stefan Hajnoczi, qemu-devel, Eric Blake, Kevin Wolf,
	Maor Lipchuk, Nir Soffer, John Snow

On Tue, Mar 07, 2017 at 01:11:44PM +0100, Alberto Garcia wrote:
> On Tue 07 Mar 2017 11:36:54 AM CET, Daniel P. Berrange wrote:
> >> The creation scenario is:
> >> 
> >>   $ qemu-img max-size -O qcow2 --size 5G
> >>   196688
> >
> > Hmm, so that appears to be indicating the amount of physical space
> > that a qcow2 image would take up before any data has been written to
> > it.
> >
> > That's not actually what I was thinking. I would like to know the
> > maximum possible physical space that a 5G qcow2 image would take up
> > once data is written to every sector. Obviously this is impossible to
> > say if you allow for internal snapshots, but I think it is fine to say
> > that we ignore internal snapshots for purposes of this command.
> 
> We have clearly two different use cases here, although I wonder how
> useful the one that you are describing is. After all the maximum size of
> a fully allocated imaged is always going to be the virtual size plus a
> small overhead for the metadata. I haven't made the numbers for all
> cases, but I'll take the risk and say it's always going to be really
> small (the 10% you use to illustrate your point is way too much).
> 
> A fully allocated 1TB qcow2 image needs less than 200MB of metadata
> (that's 0.02% of the total size). If we reduce the cluster size to its
> minimum (512 bytes) then it's around 20GB (still ~2% of the total size).

I know of several places where this info is useful. OpenStack stores
images in a filesystem. It knows what space is available in the filesystem
and wants to avoid creating qcow2 file that would overcommit that available
space. Knowing the qcow2 overhead would allow it to compute that without
having to resort to over-cautious guesswork which needlessly leaves
space unused. Similarly when creating a file that has a backing file,
qcow2 does not allow use of preallocation, and openstack thus uses
fallocate after qemu has created the file in order to reserve the
space needed to allow for maximum possible growth of the qcow2 file.
oVirt stores qcow2 images inside block devices and it would be useful
to know maximum possible size in order to correctly size the block
device without having to resort to guesswork.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support
  2017-03-07 10:32   ` Alberto Garcia
@ 2017-03-10  4:33     ` Stefan Hajnoczi
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-10  4:33 UTC (permalink / raw)
  To: Alberto Garcia
  Cc: Stefan Hajnoczi, qemu-devel, Kevin Wolf, Nir Soffer,
	Maor Lipchuk, John Snow

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

On Tue, Mar 07, 2017 at 11:32:29AM +0100, Alberto Garcia wrote:
> On Fri 03 Mar 2017 02:51:48 PM CET, Stefan Hajnoczi wrote:
> 
> > +static uint64_t raw_max_size(QemuOpts *opts, BlockDriverState *in_bs,
> > +                             Error **errp)
> > +{
> > +    if (in_bs) {
> > +        int64_t size = bdrv_nb_sectors(in_bs);
> > +        if (size < 0) {
> > +            error_setg_errno(errp, -size, "Unable to get image size");
> > +            return 0;
> > +        }
> > +        return (uint64_t)size * BDRV_SECTOR_SIZE;
> > +    }
> 
> Why not use bdrv_getlength() directly? It gives you the size in bytes.

Good idea.  Will fix.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand
  2017-03-03 21:56   ` Nir Soffer
@ 2017-03-10  4:42     ` Stefan Hajnoczi
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-10  4:42 UTC (permalink / raw)
  To: Nir Soffer
  Cc: Stefan Hajnoczi, Kevin Wolf, John Snow, qemu-devel, Maor Lipchuk,
	Alberto Garcia

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

On Fri, Mar 03, 2017 at 11:56:54PM +0200, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > +    ret_size = bdrv_max_size(drv, opts, in_blk ? blk_bs(in_blk) : NULL,
> > +                             &local_err);
> > +    if (local_err) {
> > +        error_report_err(local_err);
> > +        goto fail;
> > +    }
> > +
> > +    if (output_format == OFORMAT_HUMAN) {
> > +        printf("%" PRIu64 "\n", ret_size);
> > +    } else {
> > +        printf("{ \"size\": %" PRIu64 " }\n", ret_size);
> > +    }
> > +
> > +    ret = 0;
> > +
> > +fail:
> 
> This looks more like out: to me, since we use it both for normal
> and abnormal flows.

Looking at the source file there is no consistency: "err", "fail", "out"
are all used in various places.  I'm happy to switch this to "out", but
for the record, I find "fail" clearer since it communicates the
intention that this is an error return.

Will fix.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand
  2017-03-03 22:15     ` Nir Soffer
  2017-03-04  0:14       ` Nir Soffer
@ 2017-03-10  7:58       ` Stefan Hajnoczi
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2017-03-10  7:58 UTC (permalink / raw)
  To: Nir Soffer
  Cc: John Snow, Kevin Wolf, Alberto Garcia, qemu-devel, Nir Soffer,
	Maor Lipchuk, Stefan Hajnoczi

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

On Sat, Mar 04, 2017 at 12:15:00AM +0200, Nir Soffer wrote:
> On Sat, Mar 4, 2017 at 12:02 AM, John Snow <jsnow@redhat.com> wrote:
> >
> >
> > On 03/03/2017 04:38 PM, Nir Soffer wrote:
> >> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >>>
> >>> RFCv1:
> >>>  * Publishing patch series with just raw support, no qcow2 yet.  Please review
> >>>    the command-line interface and let me know if you are happy with this
> >>>    approach.
> >>>
> >>> Users and management tools sometimes need to know the size required for a new
> >>> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of time.
> >>> Image formats like qcow2 have non-trivial metadata that makes it hard to
> >>> estimate the exact size without knowledge of file format internals.
> >>>
> >>> This patch series introduces a new qemu-img subcommand that calculates the
> >>> required size for both image creation and conversion scenarios.
> >>>
> >>> The conversion scenario is:
> >>>
> >>>   $ qemu-img max-size -f raw -O qcow2 input.img
> >>>   107374184448
> >>
> >> Isn't this the minimal size required to convert input.img?
> >>
> >
> > It's an upper bound for the property being measured, which is current
> > allocation size, not maximum potential size after growth.
> 
> From my point of view, this is the minimal size you must allocate if you
> want to convert the image to logical volume.
> 
> >
> >>>
> >>> Here an existing image file is taken and the output includes the space required
> >>> for data from the input image file.
> >>>
> >>> The creation scenario is:
> >>>
> >>>   $ qemu-img max-size -O qcow2 --size 5G
> >>>   196688
> >>
> >> Again, this is the minimal size.
> >>
> >> So maybe use min-size?
> >>
> >> Or:
> >>
> >>     qemu-img measure -f raw -O qcow2 input.img
> >>
> >> Works nicely with other verbs like create, convert, check.
> >>
> >
> > Measure what? This is strictly less descriptive even if "max-size" isn't
> > a verb.
> 
> measure-size?

You're right, the sub-command should be a verb.

> >> Now about the return value, do we want to return both the minimum size
> >> and the maximum size?
> >>
> >> For ovirt use case, we currently calculate the maximum size by multiplying
> >> by 1.1. We use this when doing automatic extending of ovirt thin provisioned
> >> disk. We start with 1G lv, and extend it each time it becomes full, stopping
> >> when we reach virtual size * 1.1. Using more accurate calculation instead
> >> can be nicer.
> >>
> >> So we can retrun:
> >>
> >> {
> >>     "min-size": 196688,
> >>     "max-size": 5905580032
> >> }
> >>
> >> Anyway thanks for working on this!
> >>
> >
> > It sounds like you want something different from what was intuited by
> > Maor Lipchuck. There are two things to estimate:
> >
> > (A) An estimate of the possible size of an image after conversion to a
> > different format, and
> > (B) An estimate of the possible size after full allocation.
> >
> > I got the sense that Maor was asking for (A), but perhaps I am wrong
> > about that. However, both are "maximums" in different senses.
> 
> Both are minimum when you have to allocate the space.
> 
> Maor ask about A because he is working on fixing allocation when
> converting existing files, but we also have other use cases like B.

Daniel Berrange is also interested in the size of a fully populated
image file.  I'll expand the patch series to report both sizes.

Stefan

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-03-13  6:55 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-03 13:51 [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand Stefan Hajnoczi
2017-03-03 13:51 ` [Qemu-devel] [RFC 1/4] block: add bdrv_max_size() API Stefan Hajnoczi
2017-03-07 10:27   ` Alberto Garcia
2017-03-03 13:51 ` [Qemu-devel] [RFC 2/4] raw-format: add bdrv_max_size() support Stefan Hajnoczi
2017-03-07 10:32   ` Alberto Garcia
2017-03-10  4:33     ` Stefan Hajnoczi
2017-03-03 13:51 ` [Qemu-devel] [RFC 3/4] qemu-img: add max-size subcommand Stefan Hajnoczi
2017-03-03 21:56   ` Nir Soffer
2017-03-10  4:42     ` Stefan Hajnoczi
2017-03-03 13:51 ` [Qemu-devel] [RFC 4/4] iotests: add test 178 for qemu-img max-size Stefan Hajnoczi
2017-03-03 20:48 ` [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand John Snow
2017-03-03 21:38 ` Nir Soffer
2017-03-03 22:02   ` John Snow
2017-03-03 22:15     ` Nir Soffer
2017-03-04  0:14       ` Nir Soffer
2017-03-10  7:58       ` Stefan Hajnoczi
2017-03-07 10:36 ` Daniel P. Berrange
2017-03-07 12:11   ` Alberto Garcia
2017-03-07 12:25     ` Daniel P. Berrange

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.