Re: [Qemu-devel] [PATCH 1/3] block: Image file option amendment

[Kevin Wolf]

Am 28.08.2013 um 10:08 hat Max Reitz geschrieben:
> This patch adds the "amend" option to qemu-img which allows changing
> image options on existing image files. It also adds the generic bdrv
> implementation which is basically just a wrapper for the image format
> specific function.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block.c                   |  8 +++++
>  include/block/block.h     |  2 ++
>  include/block/block_int.h |  3 ++
>  qemu-img-cmds.hx          |  6 ++++
>  qemu-img.c                | 90 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-img.texi             |  5 +++
>  6 files changed, 114 insertions(+)

> +static int img_amend(int argc, char **argv)
> +{
> +    int c, ret = 0;
> +    char *options = NULL;
> +    QEMUOptionParameter *create_options = NULL, *options_param = NULL;
> +    const char *fmt = NULL, *filename;
> +    bool quiet = false;
> +    BlockDriverState *bs = NULL;
> +    BlockDriver *bdrv;
> +
> +    for (;;) {
> +        c = getopt(argc, argv, "h?qf:o:");
> +        if (c == -1) {
> +            break;
> +        }
> +
> +        switch (c) {
> +            case 'h':
> +            case '?':
> +                help();
> +                break;
> +            case 'o':
> +                options = optarg;
> +                break;
> +            case 'f':
> +                fmt = optarg;
> +                break;
> +            case 'q':
> +                quiet = true;
> +                break;
> +        }
> +    }
> +
> +    if (optind != argc - 1) {
> +        help();
> +    }
> +
> +    if (!options) {
> +        help();
> +    }
> +
> +    filename = argv[argc - 1];
> +
> +    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, 
> quiet);
> +    if (!bs) {
> +        error_report("Could not open image.");
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    bdrv = bs->drv;
> +    if (!bdrv) {

This can't happen. A successful bdrv_open() always results in a
BlockDriverState that has a driver assigned.

> +        error_report("No driver found for this image file.");
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    fmt = bdrv->format_name;
> +
> +    if (is_help_option(options)) {
> +        ret = print_block_option_help(filename, fmt);
> +        goto out;
> +    }
> +
> +    create_options = append_option_parameters(create_options,
> +            bdrv->create_options);
> +    options_param = parse_option_parameters(options, create_options,
> +            options_param);
> +    if (options_param == NULL) {
> +        error_report("Invalid options for file format '%s'.", fmt);
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    ret = bdrv_amend_options(bs, options_param);
> +    if (ret < 0) {
> +        error_report("Error while amending options: %s", strerror(-ret));
> +        goto out;
> +    }
> +
> +out:
> +    if (bs) {
> +        bdrv_delete(bs);
> +    }
> +    if (ret) {
> +        return 1;
> +    }
> +    return 0;
> +}

Some free_option_parameters() calls are missing here.

Kevin