qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Leandro Dorileo <l@dorileo.org>
To: Chunyan Liu <cyliu@suse.com>
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
Date: Thu, 1 May 2014 19:14:30 -0300	[thread overview]
Message-ID: <20140501221430.GA12592@dorilex-lnv.MultilaserAP> (raw)
In-Reply-To: <1398762656-26079-13-git-send-email-cyliu@suse.com>

On Tue, Apr 29, 2014 at 05:10:36PM +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.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>


Did you send the patch series twice? I commented in the first v26 emails...


-- 
Leandro Dorileo

> ---
> 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)) {
>          error_report("Invalid options for file format '%s'", fmt);
>          ret = -1;
>          goto out;
>      }
>  
> -    ret = bdrv_amend_options(bs, options_param);
> +    ret = bdrv_amend_options(bs, NULL, opts);
>      if (ret < 0) {
>          error_report("Error while amending options: %s", strerror(-ret));
>          goto out;
> @@ -2766,8 +2762,8 @@ out:
>      if (bs) {
>          bdrv_unref(bs);
>      }
> -    free_option_parameters(create_options);
> -    free_option_parameters(options_param);
> +    qemu_opts_del(opts);
> +    qemu_opts_free(create_opts);
>      g_free(options);
>  
>      if (ret) {
> -- 
> 1.8.4.5
> 
> 

  parent reply	other threads:[~2014-05-01 22:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1398762656-26079-1-git-send-email-cyliu@suse.com>
     [not found] ` <1398762656-26079-14-git-send-email-cyliu@suse.com>
2014-05-01 19:18   ` [Qemu-devel] [PATCH V26 13/32] vvfat.c: handle cross_driver's create_options and create_opts Eric Blake
2014-05-01 19:22     ` Eric Blake
     [not found] ` <1398762656-26079-20-git-send-email-cyliu@suse.com>
2014-05-01 21:50   ` [Qemu-devel] [PATCH V26 19/32] qcow2.c: replace QEMUOptionParameter with QemuOpts Eric Blake
     [not found] ` <1398762656-26079-18-git-send-email-cyliu@suse.com>
2014-05-01 21:57   ` [Qemu-devel] [PATCH V26 17/32] nfs.c: " Eric Blake
2014-05-06 13:20   ` Stefan Hajnoczi
     [not found] ` <1398762656-26079-13-git-send-email-cyliu@suse.com>
2014-05-01 22:14   ` Leandro Dorileo [this message]
     [not found]   ` <5360587E.5050409@redhat.com>
2014-05-04  3:51     ` [Qemu-devel] [PATCH V26 12/32] change block layer to support both QemuOpts and QEMUOptionParamter Chun Yan Liu
     [not found] ` <1398762656-26079-3-git-send-email-cyliu@suse.com>
2014-05-06 13:14   ` [Qemu-devel] [PATCH V26 02/32] QemuOpts: add def_value_str to QemuOptDesc Stefan Hajnoczi
     [not found] ` <1398762656-26079-4-git-send-email-cyliu@suse.com>
     [not found]   ` <535FD3B6.6020701@redhat.com>
2014-05-04  3:20     ` [Qemu-devel] [PATCH V26 03/32] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters Chun Yan Liu
2014-05-06 13:14   ` Stefan Hajnoczi
     [not found] ` <1398762656-26079-5-git-send-email-cyliu@suse.com>
2014-05-06 13:15   ` [Qemu-devel] [PATCH V26 04/32] qapi: output def_value_str when query command line options Stefan Hajnoczi
     [not found] ` <1398762656-26079-16-git-send-email-cyliu@suse.com>
2014-05-01 19:25   ` [Qemu-devel] [PATCH V26 15/32] gluster.c: replace QEMUOptionParameter with QemuOpts Eric Blake
2014-05-06 13:18   ` Stefan Hajnoczi
     [not found] ` <1398762656-26079-21-git-send-email-cyliu@suse.com>
2014-05-05 23:08   ` [Qemu-devel] [PATCH V26 20/32] qed.c: " Eric Blake
2014-05-06 13:22   ` Stefan Hajnoczi
     [not found] ` <1398762656-26079-28-git-send-email-cyliu@suse.com>
2014-05-06 13:24   ` [Qemu-devel] [PATCH V26 27/32] vdi.c: " Stefan Hajnoczi
2014-05-06 13:26 ` [Qemu-devel] [PATCH V26 00/32] " Stefan Hajnoczi
2014-05-19  3:02   ` Chun Yan Liu
2014-05-19 21:11     ` Leandro Dorileo
2014-06-03  5:34       ` Chun Yan Liu
     [not found] <1398762521-25733-1-git-send-email-cyliu@suse.com>
     [not found] ` <1398762521-25733-13-git-send-email-cyliu@suse.com>
2014-05-01 22:09   ` [Qemu-devel] [PATCH V26 12/32] change block layer to support both QemuOpts and QEMUOptionParamter Leandro Dorileo
2014-05-05  9:03     ` Chun Yan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140501221430.GA12592@dorilex-lnv.MultilaserAP \
    --to=l@dorileo.org \
    --cc=cyliu@suse.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).