[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V26 12/32] change block layer to support both Qe
|
From: |
Chun Yan Liu |
|
Subject: |
Re: [Qemu-devel] [PATCH V26 12/32] change block layer to support both QemuOpts and QEMUOptionParamter |
|
Date: |
Mon, 05 May 2014 03:03:03 -0600 |
>>> On 5/2/2014 at 06:09 AM, in message
<address@hidden>, Leandro Dorileo
<address@hidden> wrote:
> Chunyan,
>
> On Tue, Apr 29, 2014 at 05:08:21PM +0800, Chunyan Liu wrote:
> > Change block layer to support both QemuOpts and QEMUOptionParameter.
> > After this patch, it will change backend drivers one by one. At the end,
> > QEMUOptionParameter will be removed and only QemuOpts is kept.
>
>
> This patch breaks the tests in the intermediate tree state, when you clean
> things
> up in the end of your series most problems this patch introduces are removed
> as well,
> I saw problems with 061 which I could easily find the problem (see below)
> and then
> I saw an error on 063 with a mem corruption (this last one I had no time to
> investigate
> more).
Fix qemu_opts_append:
g_free(tmp_list) -> qemu_opts_free(tmp_list)
> Your patch series is looking very good, just take some time to make sure
> you're not
> breaking the tests and the build in the intermediate states (on all you
> patches).
>
>
> >
> > Signed-off-by: Dong Xu Wang <address@hidden>
> > Signed-off-by: Chunyan Liu <address@hidden>
> > ---
> > Changes to V25:
> > * fix Eric's comments:
> > * update bdrv_create_co_entry and bdrv_amend_options code, to let it
> > more readable.
> > * add assertion in bdrv_register.
> > * improve comments to create_opts in header file.
> >
> > block.c | 158
> > ++++++++++++++++++++++++++++++++--------------
> > block/cow.c | 2 +-
> > block/qcow.c | 2 +-
> > block/qcow2.c | 2 +-
> > block/qed.c | 2 +-
> > block/raw_bsd.c | 2 +-
> > block/vhdx.c | 2 +-
> > block/vmdk.c | 4 +-
> > block/vvfat.c | 2 +-
> > include/block/block.h | 7 +-
> > include/block/block_int.h | 13 +++-
> > qemu-img.c | 94 +++++++++++++--------------
> > 12 files changed, 180 insertions(+), 110 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index 4745712..124f045 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -328,6 +328,13 @@ void bdrv_register(BlockDriver *bdrv)
> > }
> > }
> >
> > + if (bdrv->bdrv_create) {
> > + assert(!bdrv->bdrv_create2 && !bdrv->create_opts);
> > + assert(!bdrv->bdrv_amend_options2);
> > + } else if (bdrv->bdrv_create2) {
> > + assert(!bdrv->bdrv_create && !bdrv->create_options);
> > + assert(!bdrv->bdrv_amend_options);
> > + }
> > QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
> > }
> >
> > @@ -419,6 +426,7 @@ typedef struct CreateCo {
> > BlockDriver *drv;
> > char *filename;
> > QEMUOptionParameter *options;
> > + QemuOpts *opts;
> > int ret;
> > Error *err;
> > } CreateCo;
> > @@ -430,8 +438,28 @@ static void coroutine_fn bdrv_create_co_entry(void
> *opaque)
> >
> > CreateCo *cco = opaque;
> > assert(cco->drv);
> > + assert(!(cco->options && cco->opts));
> >
> > - ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
> > + if (cco->drv->bdrv_create2) {
> > + QemuOptsList *opts_list = NULL;
> > + if (cco->options) {
> > + opts_list = params_to_opts(cco->options);
> > + cco->opts = qemu_opts_create(opts_list, NULL, 0,
> > &error_abort);
> > + }
> > + ret = cco->drv->bdrv_create2(cco->filename, cco->opts,
> > &local_err);
> > + if (cco->options) {
> > + qemu_opts_del(cco->opts);
> > + qemu_opts_free(opts_list);
> > + }
> > + } else {
> > + if (cco->opts) {
> > + cco->options = opts_to_params(cco->opts);
> > + }
> > + ret = cco->drv->bdrv_create(cco->filename, cco->options,
> > &local_err);
> > + if (cco->opts) {
> > + free_option_parameters(cco->options);
> > + }
> > + }
> > if (local_err) {
> > error_propagate(&cco->err, local_err);
> > }
> > @@ -439,7 +467,8 @@ static void coroutine_fn bdrv_create_co_entry(void
> *opaque)
> > }
> >
> > int bdrv_create(BlockDriver *drv, const char* filename,
> > - QEMUOptionParameter *options, Error **errp)
> > + QEMUOptionParameter *options,
> > + QemuOpts *opts, Error **errp)
> > {
> > int ret;
> >
> > @@ -448,11 +477,12 @@ int bdrv_create(BlockDriver *drv, const char*
> filename,
> > .drv = drv,
> > .filename = g_strdup(filename),
> > .options = options,
> > + .opts = opts,
> > .ret = NOT_DONE,
> > .err = NULL,
> > };
> >
> > - if (!drv->bdrv_create) {
> > + if (!drv->bdrv_create && !drv->bdrv_create2) {
> > error_setg(errp, "Driver '%s' does not support image creation",
> drv->format_name);
> > ret = -ENOTSUP;
> > goto out;
> > @@ -484,7 +514,7 @@ out:
> > }
> >
> > int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
> > - Error **errp)
> > + QemuOpts *opts, Error **errp)
> > {
> > BlockDriver *drv;
> > Error *local_err = NULL;
> > @@ -496,7 +526,7 @@ int bdrv_create_file(const char* filename,
> QEMUOptionParameter *options,
> > return -ENOENT;
> > }
> >
> > - ret = bdrv_create(drv, filename, options, &local_err);
> > + ret = bdrv_create(drv, filename, options, opts, &local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > }
> > @@ -1184,7 +1214,8 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
> Error **errp)
> > char *tmp_filename = g_malloc0(PATH_MAX + 1);
> > int64_t total_size;
> > BlockDriver *bdrv_qcow2;
> > - QEMUOptionParameter *create_options;
> > + QemuOptsList *create_opts = NULL;
> > + QemuOpts *opts = NULL;
> > QDict *snapshot_options;
> > BlockDriverState *bs_snapshot;
> > Error *local_err;
> > @@ -1209,13 +1240,20 @@ void bdrv_append_temp_snapshot(BlockDriverState
> > *bs,
> Error **errp)
> > }
> >
> > bdrv_qcow2 = bdrv_find_format("qcow2");
> > - create_options = parse_option_parameters("",
> > bdrv_qcow2->create_options,
> > - NULL);
> > -
> > - set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
> >
> > - ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options,
> > &local_err);
> > - free_option_parameters(create_options);
> > + assert(!(bdrv_qcow2->create_options && bdrv_qcow2->create_opts));
> > + if (bdrv_qcow2->create_options) {
> > + create_opts = params_to_opts(bdrv_qcow2->create_options);
> > + } else {
> > + create_opts = bdrv_qcow2->create_opts;
> > + }
> > + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
> > + ret = bdrv_create(bdrv_qcow2, tmp_filename, NULL, opts, &local_err);
> > + qemu_opts_del(opts);
> > + if (bdrv_qcow2->create_options) {
> > + qemu_opts_free(create_opts);
> > + }
> > if (ret < 0) {
> > error_setg_errno(errp, -ret, "Could not create temporary overlay "
> > "'%s': %s", tmp_filename,
> > @@ -5310,8 +5348,10 @@ void bdrv_img_create(const char *filename, const
> > char
> *fmt,
> > char *options, uint64_t img_size, int flags,
> > Error **errp, bool quiet)
> > {
> > - QEMUOptionParameter *param = NULL, *create_options = NULL;
> > - QEMUOptionParameter *backing_fmt, *backing_file, *size;
> > + QemuOptsList *create_opts = NULL;
> > + QemuOpts *opts = NULL;
> > + const char *backing_fmt, *backing_file;
> > + int64_t size;
> > BlockDriver *drv, *proto_drv;
> > BlockDriver *backing_drv = NULL;
> > Error *local_err = NULL;
> > @@ -5330,28 +5370,25 @@ void bdrv_img_create(const char *filename, const
> char *fmt,
> > return;
> > }
> >
> > - create_options = append_option_parameters(create_options,
> > - drv->create_options);
> > - create_options = append_option_parameters(create_options,
> > - proto_drv->create_options);
> > + create_opts = qemu_opts_append(create_opts, drv->create_opts,
> > + drv->create_options);
> > + create_opts = qemu_opts_append(create_opts, proto_drv->create_opts,
> > + proto_drv->create_options);
> >
> > /* Create parameter list with default values */
> > - param = parse_option_parameters("", create_options, param);
> > -
> > - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
> > + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size);
> >
> > /* Parse -o options */
> > if (options) {
> > - param = parse_option_parameters(options, create_options, param);
> > - if (param == NULL) {
> > + if (qemu_opts_do_parse(opts, options, NULL) != 0) {
> > error_setg(errp, "Invalid options for file format '%s'.",
> fmt);
> > goto out;
> > }
> > }
> >
> > if (base_filename) {
> > - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
> > - base_filename)) {
> > + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) {
> > error_setg(errp, "Backing file not supported for file format
> '%s'",
> > fmt);
> > goto out;
> > @@ -5359,37 +5396,37 @@ void bdrv_img_create(const char *filename, const
> char *fmt,
> > }
> >
> > if (base_fmt) {
> > - if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt))
> > {
> > + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) {
> > error_setg(errp, "Backing file format not supported for file "
> > "format '%s'", fmt);
> > goto out;
> > }
> > }
> >
> > - backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
> > - if (backing_file && backing_file->value.s) {
> > - if (!strcmp(filename, backing_file->value.s)) {
> > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
> > + if (backing_file) {
> > + if (!strcmp(filename, backing_file)) {
> > error_setg(errp, "Error: Trying to create an image with the "
> > "same filename as the backing file");
> > goto out;
> > }
> > }
> >
> > - backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
> > - if (backing_fmt && backing_fmt->value.s) {
> > - backing_drv = bdrv_find_format(backing_fmt->value.s);
> > + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
> > + if (backing_fmt) {
> > + backing_drv = bdrv_find_format(backing_fmt);
> > if (!backing_drv) {
> > error_setg(errp, "Unknown backing file format '%s'",
> > - backing_fmt->value.s);
> > + backing_fmt);
> > goto out;
> > }
> > }
> >
> > // The size for the image must always be specified, with one
> exception:
> > // If we are using a backing file, we can obtain the size from there
> > - size = get_option_parameter(param, BLOCK_OPT_SIZE);
> > - if (size && size->value.n == -1) {
> > - if (backing_file && backing_file->value.s) {
> > + size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
> > + if (size == -1) {
> > + if (backing_file) {
> > BlockDriverState *bs;
> > uint64_t size;
> > char buf[32];
> > @@ -5400,11 +5437,11 @@ void bdrv_img_create(const char *filename, const
> char *fmt,
> > flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
> BDRV_O_NO_BACKING);
> >
> > bs = NULL;
> > - ret = bdrv_open(&bs, backing_file->value.s, NULL, NULL,
> back_flags,
> > + ret = bdrv_open(&bs, backing_file, NULL, NULL, back_flags,
> > backing_drv, &local_err);
> > if (ret < 0) {
> > error_setg_errno(errp, -ret, "Could not open '%s': %s",
> > - backing_file->value.s,
> > + backing_file,
> > error_get_pretty(local_err));
> > error_free(local_err);
> > local_err = NULL;
> > @@ -5414,7 +5451,7 @@ void bdrv_img_create(const char *filename, const char
> >
> *fmt,
> > size *= 512;
> >
> > snprintf(buf, sizeof(buf), "%" PRId64, size);
> > - set_option_parameter(param, BLOCK_OPT_SIZE, buf);
> > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size);
> >
> > bdrv_unref(bs);
> > } else {
> > @@ -5425,16 +5462,18 @@ void bdrv_img_create(const char *filename, const
> char *fmt,
> >
> > if (!quiet) {
> > printf("Formatting '%s', fmt=%s ", filename, fmt);
> > - print_option_parameters(param);
> > + qemu_opts_print(opts);
> > puts("");
> > }
> > - ret = bdrv_create(drv, filename, param, &local_err);
> > +
> > + ret = bdrv_create(drv, filename, NULL, opts, &local_err);
> > +
> > if (ret == -EFBIG) {
> > /* This is generally a better message than whatever the driver
> would
> > * deliver (especially because of the cluster_size_hint), since
> that
> > * is most probably not much different from "image too large". */
> > const char *cluster_size_hint = "";
> > - if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE))
> > {
> > + if (qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, 0)) {
> > cluster_size_hint = " (try using a larger cluster size)";
> > }
> > error_setg(errp, "The image size is too large for file format
> '%s'"
> > @@ -5444,9 +5483,8 @@ void bdrv_img_create(const char *filename, const char
> >
> *fmt,
> > }
> >
> > out:
> > - free_option_parameters(create_options);
> > - free_option_parameters(param);
> > -
> > + qemu_opts_del(opts);
> > + qemu_opts_free(create_opts);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > }
> > @@ -5464,12 +5502,36 @@ void
> > bdrv_add_before_write_notifier(BlockDriverState
> *bs,
> > notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
> > }
> >
> > -int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options)
> > +int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options,
> > + QemuOpts *opts)
> > {
> > - if (bs->drv->bdrv_amend_options == NULL) {
> > + int ret;
> > + assert(!(options && opts));
> > +
> > + if (!bs->drv->bdrv_amend_options && !bs->drv->bdrv_amend_options2) {
> > return -ENOTSUP;
> > }
> > - return bs->drv->bdrv_amend_options(bs, options);
> > + if (bs->drv->bdrv_amend_options2) {
> > + QemuOptsList *opts_list = NULL;
> > + if (options) {
> > + opts_list = params_to_opts(options);
> > + opts = qemu_opts_create(opts_list, NULL, 0, &error_abort);
> > + }
> > + ret = bs->drv->bdrv_amend_options2(bs, opts);
> > + if (options) {
> > + qemu_opts_del(opts);
> > + qemu_opts_free(opts_list);
> > + }
> > + } else {
> > + if (opts) {
> > + options = opts_to_params(opts);
> > + }
> > + ret = bs->drv->bdrv_amend_options(bs, options);
> > + if (opts) {
> > + free_option_parameters(options);
> > + }
> > + }
> > + return ret;
> > }
> >
> > /* This function will be called by the bdrv_recurse_is_first_non_filter
> method
> > diff --git a/block/cow.c b/block/cow.c
> > index 30deb88..26cf4a5 100644
> > --- a/block/cow.c
> > +++ b/block/cow.c
> > @@ -345,7 +345,7 @@ static int cow_create(const char *filename,
> QEMUOptionParameter *options,
> > options++;
> > }
> >
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > return ret;
> > diff --git a/block/qcow.c b/block/qcow.c
> > index d5a7d5f..9411aef 100644
> > --- a/block/qcow.c
> > +++ b/block/qcow.c
> > @@ -687,7 +687,7 @@ static int qcow_create(const char *filename,
> QEMUOptionParameter *options,
> > options++;
> > }
> >
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > return ret;
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index e903d97..49985e3 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1625,7 +1625,7 @@ static int qcow2_create2(const char *filename,
> > int64_t
> total_size,
> > Error *local_err = NULL;
> > int ret;
> >
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > return ret;
> > diff --git a/block/qed.c b/block/qed.c
> > index c130e42..2982640 100644
> > --- a/block/qed.c
> > +++ b/block/qed.c
> > @@ -566,7 +566,7 @@ static int qed_create(const char *filename, uint32_t
> cluster_size,
> > int ret = 0;
> > BlockDriverState *bs;
> >
> > - ret = bdrv_create_file(filename, NULL, &local_err);
> > + ret = bdrv_create_file(filename, NULL, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > return ret;
> > diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> > index 01ea692..9ae5fc2 100644
> > --- a/block/raw_bsd.c
> > +++ b/block/raw_bsd.c
> > @@ -145,7 +145,7 @@ static int raw_create(const char *filename,
> QEMUOptionParameter *options,
> > Error *local_err = NULL;
> > int ret;
> >
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > }
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index 509baaf..a9fcf6b 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -1796,7 +1796,7 @@ static int vhdx_create(const char *filename,
> QEMUOptionParameter *options,
> > block_size = block_size > VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX :
> > block_size;
> >
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > goto exit;
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 06a1f9f..3802863 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -1526,7 +1526,7 @@ static int vmdk_create_extent(const char *filename,
> int64_t filesize,
> > uint32_t *gd_buf = NULL;
> > int gd_buf_size;
> >
> > - ret = bdrv_create_file(filename, NULL, &local_err);
> > + ret = bdrv_create_file(filename, NULL, NULL, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
> > goto exit;
> > @@ -1866,7 +1866,7 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options,
> > if (!split && !flat) {
> > desc_offset = 0x200;
> > } else {
> > - ret = bdrv_create_file(filename, options, &local_err);
> > + ret = bdrv_create_file(filename, options, NULL, &local_err);
> > if (ret < 0) {
> > error_setg_errno(errp, -ret, "Could not create image file");
> > goto exit;
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index c3af7ff..155fc9b 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -2926,7 +2926,7 @@ static int enable_write_target(BDRVVVFATState *s)
> > set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count *
> 512);
> > set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
> >
> > - ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
> > + ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL,
> &local_err);
> > if (ret < 0) {
> > qerror_report_err(local_err);
> > error_free(local_err);
> > diff --git a/include/block/block.h b/include/block/block.h
> > index c12808a..bfa2192 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -177,9 +177,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
> > BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
> > bool readonly);
> > int bdrv_create(BlockDriver *drv, const char* filename,
> > - QEMUOptionParameter *options, Error **errp);
> > + QEMUOptionParameter *options, QemuOpts *opts, Error **errp);
> > int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
> > - Error **errp);
> > + QemuOpts *opts, Error **errp);
> > BlockDriverState *bdrv_new(const char *device_name, Error **errp);
> > void bdrv_make_anon(BlockDriverState *bs);
> > void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
> > @@ -284,7 +284,8 @@ typedef enum {
> >
> > int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode
> fix);
> >
> > -int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter
> *options);
> > +int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter
> *options,
> > + QemuOpts *opts);
> >
> > /* external snapshots */
> > bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index cd5bc73..59875db 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -118,6 +118,8 @@ struct BlockDriver {
> > void (*bdrv_rebind)(BlockDriverState *bs);
> > int (*bdrv_create)(const char *filename, QEMUOptionParameter *options,
> > Error **errp);
> > + /* FIXME: will remove the duplicate and rename back to bdrv_create
> later */
> > + int (*bdrv_create2)(const char *filename, QemuOpts *opts, Error
> **errp);
> > int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
> > int (*bdrv_make_empty)(BlockDriverState *bs);
> > /* aio */
> > @@ -217,7 +219,12 @@ struct BlockDriver {
> >
> > /* List of options for creating images, terminated by name == NULL */
> > QEMUOptionParameter *create_options;
> > -
> > + /* FIXME: will replace create_options.
> > + * These two fields are mutually exclusive. At most one is non-NULL.
> > + * create_options should only be set with bdrv_create, and create_opts
> > + * should only be set with bdrv_create2.
> > + */
> > + QemuOptsList *create_opts;
> >
> > /*
> > * Returns 0 for completed check, -errno for internal errors.
> > @@ -228,6 +235,10 @@ struct BlockDriver {
> >
> > int (*bdrv_amend_options)(BlockDriverState *bs,
> > QEMUOptionParameter *options);
> > + /* FIXME: will remove the duplicate and rename back to
> > + * bdrv_amend_options later
> > + */
> > + int (*bdrv_amend_options2)(BlockDriverState *bs, QemuOpts *opts);
> >
> > void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
> >
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 968b4c8..14d3acd 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -249,7 +249,7 @@ static int read_password(char *buf, int buf_size)
> > static int print_block_option_help(const char *filename, const char *fmt)
> > {
> > BlockDriver *drv, *proto_drv;
> > - QEMUOptionParameter *create_options = NULL;
> > + QemuOptsList *create_opts = NULL;
> >
> > /* Find driver and parse its options */
> > drv = bdrv_find_format(fmt);
> > @@ -258,21 +258,20 @@ static int print_block_option_help(const char
> *filename, const char *fmt)
> > return 1;
> > }
> >
> > - create_options = append_option_parameters(create_options,
> > - drv->create_options);
> > -
> > + create_opts = qemu_opts_append(create_opts, drv->create_opts,
> > + drv->create_options);
> > if (filename) {
> > proto_drv = bdrv_find_protocol(filename, true);
> > if (!proto_drv) {
> > error_report("Unknown protocol '%s'", filename);
> > return 1;
> > }
> > - create_options = append_option_parameters(create_options,
> > -
> > proto_drv->create_options);
> > + create_opts = qemu_opts_append(create_opts,
> > proto_drv->create_opts,
> > + proto_drv->create_options);
> > }
> >
> > - print_option_help(create_options);
> > - free_option_parameters(create_options);
> > + qemu_opts_print_help(create_opts);
> > + qemu_opts_free(create_opts);
> > return 0;
> > }
> >
> > @@ -326,19 +325,19 @@ fail:
> > return NULL;
> > }
> >
> > -static int add_old_style_options(const char *fmt, QEMUOptionParameter
> *list,
> > +static int add_old_style_options(const char *fmt, QemuOpts *opts,
> > const char *base_filename,
> > const char *base_fmt)
> > {
> > if (base_filename) {
> > - if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE,
> base_filename)) {
> > + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) {
> > error_report("Backing file not supported for file format
> '%s'",
> > fmt);
> > return -1;
> > }
> > }
> > if (base_fmt) {
> > - if (set_option_parameter(list, BLOCK_OPT_BACKING_FMT, base_fmt)) {
> > + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) {
> > error_report("Backing file format not supported for file "
> > "format '%s'", fmt);
> > return -1;
> > @@ -1169,8 +1168,9 @@ static int img_convert(int argc, char **argv)
> > size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
> > const uint8_t *buf1;
> > BlockDriverInfo bdi;
> > - QEMUOptionParameter *param = NULL, *create_options = NULL;
> > - QEMUOptionParameter *out_baseimg_param;
> > + QemuOpts *opts = NULL;
> > + QemuOptsList *create_opts = NULL;
> > + const char *out_baseimg_param;
> > char *options = NULL;
> > const char *snapshot_name = NULL;
> > int min_sparse = 8; /* Need at least 4k of zeros for sparse detection
> */
> > @@ -1359,40 +1359,36 @@ static int img_convert(int argc, char **argv)
> > goto out;
> > }
> >
> > - create_options = append_option_parameters(create_options,
> > - drv->create_options);
> > - create_options = append_option_parameters(create_options,
> > - proto_drv->create_options);
> > + create_opts = qemu_opts_append(create_opts, drv->create_opts,
> > + drv->create_options);
> > + create_opts = qemu_opts_append(create_opts, proto_drv->create_opts,
> > + proto_drv->create_options);
> >
> > - if (options) {
> > - param = parse_option_parameters(options, create_options, param);
> > - if (param == NULL) {
> > - error_report("Invalid options for file format '%s'.",
> > out_fmt);
> > - ret = -1;
> > - goto out;
> > - }
> > - } else {
> > - param = parse_option_parameters("", create_options, param);
> > + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> > + if (options && qemu_opts_do_parse(opts, options, NULL)) {
> > + error_report("Invalid options for file format '%s'.", out_fmt);
> > + ret = -1;
> > + goto out;
> > }
> >
> > - set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
> > - ret = add_old_style_options(out_fmt, param, out_baseimg, NULL);
> > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512);
> > + ret = add_old_style_options(out_fmt, opts, out_baseimg, NULL);
> > if (ret < 0) {
> > goto out;
> > }
> >
> > /* Get backing file name if -o backing_file was used */
> > - out_baseimg_param = get_option_parameter(param,
> BLOCK_OPT_BACKING_FILE);
> > + out_baseimg_param = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
> > if (out_baseimg_param) {
> > - out_baseimg = out_baseimg_param->value.s;
> > + out_baseimg = out_baseimg_param;
> > }
> >
> > /* Check if compression is supported */
> > if (compress) {
> > - QEMUOptionParameter *encryption =
> > - get_option_parameter(param, BLOCK_OPT_ENCRYPT);
> > - QEMUOptionParameter *preallocation =
> > - get_option_parameter(param, BLOCK_OPT_PREALLOC);
> > + bool encryption =
> > + qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, false);
> > + const char *preallocation =
> > + qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
> >
> > if (!drv->bdrv_write_compressed) {
> > error_report("Compression not supported for this file
> format");
> > @@ -1400,15 +1396,15 @@ static int img_convert(int argc, char **argv)
> > goto out;
> > }
> >
> > - if (encryption && encryption->value.n) {
> > + if (encryption) {
> > error_report("Compression and encryption not supported at "
> > "the same time");
> > ret = -1;
> > goto out;
> > }
> >
> > - if (preallocation && preallocation->value.s
> > - && strcmp(preallocation->value.s, "off"))
> > + if (preallocation
> > + && strcmp(preallocation, "off"))
> > {
> > error_report("Compression and preallocation not supported at "
> > "the same time");
> > @@ -1419,7 +1415,7 @@ static int img_convert(int argc, char **argv)
> >
> > if (!skip_create) {
> > /* Create the new image */
> > - ret = bdrv_create(drv, out_filename, param, &local_err);
> > + ret = bdrv_create(drv, out_filename, NULL, opts, &local_err);
> > if (ret < 0) {
> > error_report("%s: error while converting %s: %s",
> > out_filename, out_fmt,
> error_get_pretty(local_err));
> > @@ -1683,8 +1679,8 @@ out:
> > qemu_progress_print(100, 0);
> > }
> > qemu_progress_end();
> > - free_option_parameters(create_options);
> > - free_option_parameters(param);
> > + qemu_opts_del(opts);
> > + qemu_opts_free(create_opts);
> > qemu_vfree(buf);
> > if (sn_opts) {
> > qemu_opts_del(sn_opts);
> > @@ -2675,7 +2671,8 @@ static int img_amend(int argc, char **argv)
> > {
> > int c, ret = 0;
> > char *options = NULL;
> > - QEMUOptionParameter *create_options = NULL, *options_param = NULL;
> > + QemuOptsList *create_opts = NULL;
> > + QemuOpts *opts = NULL;
> > const char *fmt = NULL, *filename;
> > bool quiet = false;
> > BlockDriverState *bs = NULL;
> > @@ -2746,17 +2743,16 @@ static int img_amend(int argc, char **argv)
> > goto out;
> > }
> >
> > - create_options = append_option_parameters(create_options,
> > - bs->drv->create_options);
> > - options_param = parse_option_parameters(options, create_options,
> > - options_param);
> > - if (options_param == NULL) {
> > + create_opts = qemu_opts_append(create_opts, bs->drv->create_opts,
> > + bs->drv->create_options);
> > + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> > + if (options && qemu_opts_do_parse(opts, options, NULL)) {
>
>
> Since now we're calling qemu_opts_do_parse() instead of
> parse_option_parameters()
> you should update the 061 test output, otherwise the test will fail due to
> wrong
> output, the following should be enough:
>
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -281,7 +281,7 @@ Lazy refcounts only supported with compatibility level
> 1.1 and above (use compat
> qemu-img: Error while amending options: Invalid argument
> Unknown compatibility level 0.42.
> qemu-img: Error while amending options: Invalid argument
> -Unknown option 'foo'
> +qemu-img: Invalid parameter 'foo'
> qemu-img: Invalid options for file format 'qcow2'
> Changing the cluster size is not supported.
> qemu-img: Error while amending options: Operation not supported
>
> Regards...