From patchwork Thu May 1 22:09:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leandro Dorileo X-Patchwork-Id: 344821 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 87F6C1400A9 for ; Fri, 2 May 2014 08:10:48 +1000 (EST) Received: from localhost ([::1]:41578 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfzBl-0002Aa-NA for incoming@patchwork.ozlabs.org; Thu, 01 May 2014 18:10:45 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfzB4-0001Om-9z for qemu-devel@nongnu.org; Thu, 01 May 2014 18:10:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WfzAv-0003dw-7J for qemu-devel@nongnu.org; Thu, 01 May 2014 18:10:02 -0400 Received: from mail-yh0-f54.google.com ([209.85.213.54]:49101) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfzAv-0003dr-0z for qemu-devel@nongnu.org; Thu, 01 May 2014 18:09:53 -0400 Received: by mail-yh0-f54.google.com with SMTP id b6so3464147yha.41 for ; Thu, 01 May 2014 15:09:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=jtxvoTSCFB6bEXsSOWMaFaV7Ca5NRhQuRAORfH5JrIE=; b=eYBjD1k3bnV65rhtjco/NuZOGyXd7AJleb4Yiby6Jud5OA6tNvzEXuCOrb2wAlWc1r 6ItorGlJbnC2m+H9BRR7J4Eb7+TwPhiC5JeM/wMpPKRm/2Mcg78b8YlRAH4GQGAS+yWL 6vUuszwOCWWQ2kGnDM/Gb+D49mB0TA90PLcPciYl4BUmthfz208VznHxdMEFreJqK8sm bN9pkBdmVL8cecviLZkOtOI24i0eflnqH+MNewrINjpZlWAC1AJDqNUcimFuyfP4DfmL phFEufwWd5Jx7Bcf4SxZtEd5VDkjmhx5j/01Jv4/U9wOqmL/TBP/f5vqZ1avcv67wm5r +iSw== X-Gm-Message-State: ALoCoQlCkOR053IvAyAlon5oOb4502Y4yVlYEZpRv4YDvFm6htn04bYN0iaenhQXpJJq6teP0B7g X-Received: by 10.236.200.67 with SMTP id y43mr18247677yhn.77.1398982191462; Thu, 01 May 2014 15:09:51 -0700 (PDT) Received: from dorilex-lnv.MultilaserAP ([177.194.92.107]) by mx.google.com with ESMTPSA id f2sm51095749yhg.7.2014.05.01.15.09.48 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 01 May 2014 15:09:50 -0700 (PDT) Date: Thu, 1 May 2014 19:09:48 -0300 From: Leandro Dorileo To: Chunyan Liu Message-ID: <20140501220948.GK29944@dorilex-lnv.MultilaserAP> References: <1398762521-25733-1-git-send-email-cyliu@suse.com> <1398762521-25733-13-git-send-email-cyliu@suse.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1398762521-25733-13-git-send-email-cyliu@suse.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.213.54 Cc: qemu-devel@nongnu.org, stefanha@redhat.com Subject: Re: [Qemu-devel] [PATCH V26 12/32] change block layer to support both QemuOpts and QEMUOptionParamter X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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). 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 > Signed-off-by: Chunyan Liu > --- > 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: Regards... --- 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