Re: [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand

[Stefan Hajnoczi]

On Thu, Mar 16, 2017 at 02:46:12AM +0100, Max Reitz wrote:
> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> > The measure subcommand calculates the 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 <address@hidden>
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > ---
> >  qemu-img.c       | 213 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qemu-img-cmds.hx |   9 +++
> 
> Looking forward to the documentation in the non-RFC version. O:-)

Thanks for reminding me that the qemu-img man page needs documentation
for this new sub-command :).

> >  2 files changed, 222 insertions(+)
> > 
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 98b836b..e8c6dcc 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,218 @@ out:
> >      return 0;
> >  }
> >  
> > +static void dump_json_block_measure_info(BlockMeasureInfo *info)
> > +{
> > +    QString *str;
> > +    QObject *obj;
> > +    Visitor *v = qobject_output_visitor_new(&obj);
> > +
> > +    visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort);
> > +    visit_complete(v, &obj);
> > +    str = qobject_to_json_pretty(obj);
> > +    assert(str != NULL);
> > +    printf("%s\n", qstring_get_str(str));
> > +    qobject_decref(obj);
> > +    visit_free(v);
> > +    QDECREF(str);
> > +}
> > +
> > +static int img_measure(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;
> > +    BlockMeasureInfo info;
> > +    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 out;
> > +            }
> > +            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 out;
> > +                }
> > +            } else {
> > +                snapshot_name = optarg;
> > +            }
> > +            break;
> > +        case OPTION_OBJECT:
> > +            object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> > +                                                  optarg, true);
> > +            if (!object_opts) {
> > +                goto out;
> > +            }
> > +            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 out;
> > +            }
> > +            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.");
> 
> BTW, I love how we say "EiB" in these error messages but the non-i
> suffixes the user can specify are still power-of-two units.
> 
> (Also, this error message explicitly says "kilobytes" and so on instead
> of "kibibytes". I'm fine with it not least because it's pre-existing,
> though. (Furthermore, I'd rather have it be "EB" than the latter message
> contain "kibibytes, mebibytes" and so on.))
> 
> > +                }
> > +                goto out;
> > +            }
> > +            img_size = (uint64_t)sval;
> > +        }
> > +        break;
> > +        }
> > +    }
> > +
> > +    if (qemu_opts_foreach(&qemu_object_opts,
> > +                          user_creatable_add_opts_foreach,
> > +                          NULL, NULL)) {
> > +        goto out;
> > +    }
> > +
> > +    if (argc - optind > 1) {
> > +        error_report("At most one filename argument is allowed.");
> 
> Not exactly like convert, then. :-(
> 
> But, yeah, it wouldn't really work for the current interface and we can
> always extend it later.
> 
> (But maybe that means the interface could be better. Maybe this central
> function here could collect a statistics of allocated/zero/free/...
> clusters of the input image (with a cluster size somehow specified by
> the output image driver, however that's supposed to work...) and pass it
> to the output driver. That would also save us from replicating the input
> block query code for all of the block drivers that implement
> bdrv_measure().)

I will look into this for the next revision, thanks.

Stefan

signature.asc
Description: PGP signature