diff mbox

[v20,06/26] change block layer to support both QemuOpts and QEMUOptionParameter

Message ID 1392186806-10418-7-git-send-email-cyliu@suse.com
State New
Headers show

Commit Message

Chunyan Liu Feb. 12, 2014, 6:33 a.m. UTC
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>
---
 block.c                   |  110 ++++++++++++++++++++++++++++----------------
 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     |    4 +-
 include/block/block_int.h |    4 +-
 include/qemu/option.h     |    2 +
 qemu-img.c                |   87 +++++++++++++++++++++--------------
 util/qemu-option.c        |  111 +++++++++++++++++++++++++++++++++++++++++++++
 14 files changed, 250 insertions(+), 86 deletions(-)

Comments

Eric Blake Feb. 13, 2014, 12:12 a.m. UTC | #1
On 02/11/2014 11:33 PM, 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>
> ---
>  block.c                   |  110 ++++++++++++++++++++++++++++----------------
>  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     |    4 +-
>  include/block/block_int.h |    4 +-
>  include/qemu/option.h     |    2 +
>  qemu-img.c                |   87 +++++++++++++++++++++--------------
>  util/qemu-option.c        |  111 +++++++++++++++++++++++++++++++++++++++++++++
>  14 files changed, 250 insertions(+), 86 deletions(-)
> 
> +++ 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);
> +    int (*bdrv_create2)(const char *filename, QemuOpts *opts,
> +                       Error **errp);

Maybe a FIXME comment that shows we plan on removing the duplicate and
renaming back to a single sane name in a few more commits.

> +++ b/include/qemu/option.h
> @@ -168,4 +168,6 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
>  QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
>  void qemu_opts_free(QemuOptsList *list);
>  void qemu_opts_print_help(QemuOptsList *list);
> +QEMUOptionParameter *opts_to_params(QemuOpts *opts);
> +QemuOptsList *params_to_opts(QEMUOptionParameter *list);

I'd split this commit into two - one that adds these two conversion
functions, then the other that adds the create2 callback.

> +++ b/util/qemu-option.c
> @@ -1396,3 +1396,114 @@ void qemu_opts_print_help(QemuOptsList *list)
>                 list->desc[i].help : "");
>      }
>  }
> +
> +/* convert QEMUOptionParameter to QemuOpts */
> +QemuOptsList *params_to_opts(QEMUOptionParameter *list)
> +{
> +    QemuOptsList *opts = NULL;
> +    size_t num_opts, i = 0;
> +
> +    if (!list) {
> +        return NULL;
> +    }
> +
> +    num_opts = count_option_parameters(list);
> +    opts = g_malloc0(sizeof(QemuOptsList) +
> +                              (num_opts + 1) * sizeof(QemuOptDesc));

Indentation looks off.

> +    QTAILQ_INIT(&opts->head);
> +    opts->desc[i].name = NULL;
> +
> +    while (list && list->name) {
> +        opts->desc[i].name = strdup(list->name);
> +        opts->desc[i].help = strdup(list->help);
> +        switch (list->type) {
> +        case OPT_FLAG:
> +            opts->desc[i].type = QEMU_OPT_BOOL;
> +            opts->desc[i].def_value_str = list->value.n ? "on" : "off";
> +            break;
> +
> +        case OPT_NUMBER:
> +            opts->desc[i].type = QEMU_OPT_NUMBER;
> +            if (list->value.n) {
> +                char tmp[100];
> +                sprintf(tmp, "%" PRIu64, list->value.n);
> +                opts->desc[i].def_value_str = strdup(tmp);

Eww.  Just use g_strdup_printf and avoid tmp[] altogether.

> +            }
> +            break;
> +
> +        case OPT_SIZE:
> +            opts->desc[i].type = QEMU_OPT_SIZE;
> +            if (list->value.n) {
> +                char tmp[100];
> +                sprintf(tmp, "%" PRIu64, list->value.n);
> +                opts->desc[i].def_value_str = strdup(tmp);

and again

> +            }
> +            break;
> +
> +        case OPT_STRING:
> +            opts->desc[i].type = QEMU_OPT_STRING;
> +            if (list->value.s) {
> +                opts->desc[i].def_value_str = strdup(list->value.s);

This is a lot of use of strdup() without checking for malloc failure;
wouldn't it be better to use g_strdup (which can't fail, and which
gracefully handles NULL so you can also drop the 'if') and fixing the
counterpart free to use g_free?
Chunyan Liu Feb. 18, 2014, 6:44 a.m. UTC | #2
2014-02-13 8:12 GMT+08:00 Eric Blake <eblake@redhat.com>:

> On 02/11/2014 11:33 PM, 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>
> > ---
> >  block.c                   |  110
> ++++++++++++++++++++++++++++----------------
> >  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     |    4 +-
> >  include/block/block_int.h |    4 +-
> >  include/qemu/option.h     |    2 +
> >  qemu-img.c                |   87 +++++++++++++++++++++--------------
> >  util/qemu-option.c        |  111
> +++++++++++++++++++++++++++++++++++++++++++++
> >  14 files changed, 250 insertions(+), 86 deletions(-)
> >
> > +++ 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);
> > +    int (*bdrv_create2)(const char *filename, QemuOpts *opts,
> > +                       Error **errp);
>
> Maybe a FIXME comment that shows we plan on removing the duplicate and
> renaming back to a single sane name in a few more commits.
>
> > +++ b/include/qemu/option.h
> > @@ -168,4 +168,6 @@ int qemu_opts_foreach(QemuOptsList *list,
> qemu_opts_loopfunc func, void *opaque,
> >  QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
> >  void qemu_opts_free(QemuOptsList *list);
> >  void qemu_opts_print_help(QemuOptsList *list);
> > +QEMUOptionParameter *opts_to_params(QemuOpts *opts);
> > +QemuOptsList *params_to_opts(QEMUOptionParameter *list);
>
> I'd split this commit into two - one that adds these two conversion
> functions, then the other that adds the create2 callback.
>
> > +++ b/util/qemu-option.c
> > @@ -1396,3 +1396,114 @@ void qemu_opts_print_help(QemuOptsList *list)
> >                 list->desc[i].help : "");
> >      }
> >  }
> > +
> > +/* convert QEMUOptionParameter to QemuOpts */
> > +QemuOptsList *params_to_opts(QEMUOptionParameter *list)
> > +{
> > +    QemuOptsList *opts = NULL;
> > +    size_t num_opts, i = 0;
> > +
> > +    if (!list) {
> > +        return NULL;
> > +    }
> > +
> > +    num_opts = count_option_parameters(list);
> > +    opts = g_malloc0(sizeof(QemuOptsList) +
> > +                              (num_opts + 1) * sizeof(QemuOptDesc));
>
> Indentation looks off.
>
> > +    QTAILQ_INIT(&opts->head);
> > +    opts->desc[i].name = NULL;
> > +
> > +    while (list && list->name) {
> > +        opts->desc[i].name = strdup(list->name);
> > +        opts->desc[i].help = strdup(list->help);
> > +        switch (list->type) {
> > +        case OPT_FLAG:
> > +            opts->desc[i].type = QEMU_OPT_BOOL;
> > +            opts->desc[i].def_value_str = list->value.n ? "on" : "off";
> > +            break;
> > +
> > +        case OPT_NUMBER:
> > +            opts->desc[i].type = QEMU_OPT_NUMBER;
> > +            if (list->value.n) {
> > +                char tmp[100];
> > +                sprintf(tmp, "%" PRIu64, list->value.n);
> > +                opts->desc[i].def_value_str = strdup(tmp);
>
> Eww.  Just use g_strdup_printf and avoid tmp[] altogether.
>
> > +            }
> > +            break;
> > +
> > +        case OPT_SIZE:
> > +            opts->desc[i].type = QEMU_OPT_SIZE;
> > +            if (list->value.n) {
> > +                char tmp[100];
> > +                sprintf(tmp, "%" PRIu64, list->value.n);
> > +                opts->desc[i].def_value_str = strdup(tmp);
>
> and again
>
> > +            }
> > +            break;
> > +
> > +        case OPT_STRING:
> > +            opts->desc[i].type = QEMU_OPT_STRING;
> > +            if (list->value.s) {
> > +                opts->desc[i].def_value_str = strdup(list->value.s);
>
> This is a lot of use of strdup() without checking for malloc failure;
> wouldn't it be better to use g_strdup (which can't fail, and which
> gracefully handles NULL so you can also drop the 'if') and fixing the
> counterpart free to use g_free?
>
> Will correct. Thanks.


> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
diff mbox

Patch

diff --git a/block.c b/block.c
index cb21a5f..69cec2f 100644
--- a/block.c
+++ b/block.c
@@ -408,6 +408,7 @@  typedef struct CreateCo {
     BlockDriver *drv;
     char *filename;
     QEMUOptionParameter *options;
+    QemuOpts *opts;
     int ret;
     Error *err;
 } CreateCo;
@@ -420,7 +421,11 @@  static void coroutine_fn bdrv_create_co_entry(void *opaque)
     CreateCo *cco = opaque;
     assert(cco->drv);
 
-    ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+    if (cco->drv->bdrv_create2) {
+        ret = cco->drv->bdrv_create2(cco->filename, cco->opts, &local_err);
+    } else {
+        ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+    }
     if (error_is_set(&local_err)) {
         error_propagate(&cco->err, local_err);
     }
@@ -428,7 +433,7 @@  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;
 
@@ -437,11 +442,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;
@@ -473,7 +479,7 @@  out:
 }
 
 int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
-                     Error **errp)
+                     QemuOpts *opts, Error **errp)
 {
     BlockDriver *drv;
     Error *local_err = NULL;
@@ -485,7 +491,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 (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
     }
@@ -1246,7 +1252,8 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         BlockDriverState *bs1;
         int64_t total_size;
         BlockDriver *bdrv_qcow2;
-        QEMUOptionParameter *create_options;
+        QEMUOptionParameter *create_options = NULL;
+        QemuOpts *opts = NULL;
         QDict *snapshot_options;
 
         /* if snapshot, we create a temporary backing file and open it
@@ -1273,13 +1280,21 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         }
 
         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);
+        if (bdrv_qcow2->bdrv_create2) {
+            opts = qemu_opts_create(bdrv_qcow2->create_opts, NULL, 0,
+                                    &error_abort);
+            qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
+        } else {
+            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);
+        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, opts,
+                          &local_err);
         free_option_parameters(create_options);
+        qemu_opts_del(opts);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Could not create temporary overlay "
                              "'%s': %s", tmp_filename,
@@ -5197,7 +5212,10 @@  void bdrv_img_create(const char *filename, const char *fmt,
                      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;
@@ -5216,28 +5234,31 @@  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);
+    if (drv->bdrv_create2) {
+        create_opts = qemu_opts_append(create_opts, drv->create_opts);
+        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
+    } else {
+        create_options = append_option_parameters(create_options,
+                                                  drv->create_options);
+        create_options = append_option_parameters(create_options,
+                                                  proto_drv->create_options);
+        create_opts = params_to_opts(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;
@@ -5245,37 +5266,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];
@@ -5287,11 +5308,11 @@  void bdrv_img_create(const char *filename, const char *fmt,
 
             bs = bdrv_new("");
 
-            ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
+            ret = bdrv_open(bs, backing_file, 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;
@@ -5302,7 +5323,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 {
@@ -5313,16 +5334,23 @@  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);
+
+    if (drv->bdrv_create2) {
+        ret = bdrv_create(drv, filename, NULL, opts, &local_err);
+    } else {
+        param = opts_to_params(opts);
+        ret = bdrv_create(drv, filename, param, NULL, &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'"
@@ -5335,6 +5363,8 @@  out:
     free_option_parameters(create_options);
     free_option_parameters(param);
 
+    qemu_opts_del(opts);
+    qemu_opts_free(create_opts);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
     }
diff --git a/block/cow.c b/block/cow.c
index 7fc0b12..85c2971 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -344,7 +344,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) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/qcow.c b/block/qcow.c
index 948b0c5..992eed4 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -684,7 +684,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) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 99a1ad1..10702f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1485,7 +1485,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 b9ca7ac..243c539 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -564,7 +564,7 @@  static int qed_create(const char *filename, uint32_t cluster_size,
     int ret = 0;
     BlockDriverState *bs = NULL;
 
-    ret = bdrv_create_file(filename, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, NULL, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 978ae7a..297e03f 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -139,7 +139,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 (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
     }
diff --git a/block/vhdx.c b/block/vhdx.c
index 55689cf..23efc71 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1791,7 +1791,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 e809e2e..97e6608 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1487,7 +1487,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;
@@ -1825,7 +1825,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 664941c..c59cbdb 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2929,7 +2929,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 963a61f..0f8cc50 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -174,9 +174,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);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0bcf1c9..2acce0b 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);
+    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,7 @@  struct BlockDriver {
 
     /* List of options for creating images, terminated by name == NULL */
     QEMUOptionParameter *create_options;
-
+    QemuOptsList *create_opts;
 
     /*
      * Returns 0 for completed check, -errno for internal errors.
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 3957604..de438c8 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -168,4 +168,6 @@  int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
 QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
 void qemu_opts_print_help(QemuOptsList *list);
+QEMUOptionParameter *opts_to_params(QemuOpts *opts);
+QemuOptsList *params_to_opts(QEMUOptionParameter *list);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index c989850..18b4ab5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -242,6 +242,7 @@  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);
@@ -256,12 +257,19 @@  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_options = append_option_parameters(create_options,
-                                              proto_drv->create_options);
-    print_option_help(create_options);
+    if (drv->bdrv_create2) {
+        create_opts = qemu_opts_append(create_opts, drv->create_opts);
+        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
+        qemu_opts_print_help(create_opts);
+    } else {
+        create_options = append_option_parameters(create_options,
+                                                  drv->create_options);
+        create_options = append_option_parameters(create_options,
+                                                  proto_drv->create_options);
+        print_option_help(create_options);
+    }
     free_option_parameters(create_options);
+    qemu_opts_free(create_opts);
     return 0;
 }
 
@@ -316,19 +324,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;
@@ -1142,7 +1150,9 @@  static int img_convert(int argc, char **argv)
     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 */
@@ -1312,40 +1322,42 @@  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);
-
-    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;
-        }
+    if (drv->bdrv_create2) {
+        create_opts = qemu_opts_append(create_opts, drv->create_opts);
+        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
     } else {
-        param = parse_option_parameters("", create_options, param);
+        create_options = append_option_parameters(create_options,
+                                                  drv->create_options);
+        create_options = append_option_parameters(create_options,
+                                                  proto_drv->create_options);
+        create_opts = params_to_opts(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'.", 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");
@@ -1353,15 +1365,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");
@@ -1372,7 +1384,12 @@  static int img_convert(int argc, char **argv)
 
     if (!skip_create) {
         /* Create the new image */
-        ret = bdrv_create(drv, out_filename, param, &local_err);
+        if (drv->bdrv_create2) {
+            ret = bdrv_create(drv, out_filename, NULL, opts, &local_err);
+        } else {
+            param = opts_to_params(opts);
+            ret = bdrv_create(drv, out_filename, param, NULL, &local_err);
+        }
         if (ret < 0) {
             error_report("%s: error while converting %s: %s",
                          out_filename, out_fmt, error_get_pretty(local_err));
@@ -1638,6 +1655,8 @@  out:
     qemu_progress_end();
     free_option_parameters(create_options);
     free_option_parameters(param);
+    qemu_opts_free(create_opts);
+    qemu_opts_del(opts);
     qemu_vfree(buf);
     if (sn_opts) {
         qemu_opts_del(sn_opts);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index c51c55d..be9ee13 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1396,3 +1396,114 @@  void qemu_opts_print_help(QemuOptsList *list)
                list->desc[i].help : "");
     }
 }
+
+/* convert QEMUOptionParameter to QemuOpts */
+QemuOptsList *params_to_opts(QEMUOptionParameter *list)
+{
+    QemuOptsList *opts = NULL;
+    size_t num_opts, i = 0;
+
+    if (!list) {
+        return NULL;
+    }
+
+    num_opts = count_option_parameters(list);
+    opts = g_malloc0(sizeof(QemuOptsList) +
+                              (num_opts + 1) * sizeof(QemuOptDesc));
+    QTAILQ_INIT(&opts->head);
+    opts->desc[i].name = NULL;
+
+    while (list && list->name) {
+        opts->desc[i].name = strdup(list->name);
+        opts->desc[i].help = strdup(list->help);
+        switch (list->type) {
+        case OPT_FLAG:
+            opts->desc[i].type = QEMU_OPT_BOOL;
+            opts->desc[i].def_value_str = list->value.n ? "on" : "off";
+            break;
+
+        case OPT_NUMBER:
+            opts->desc[i].type = QEMU_OPT_NUMBER;
+            if (list->value.n) {
+                char tmp[100];
+                sprintf(tmp, "%" PRIu64, list->value.n);
+                opts->desc[i].def_value_str = strdup(tmp);
+            }
+            break;
+
+        case OPT_SIZE:
+            opts->desc[i].type = QEMU_OPT_SIZE;
+            if (list->value.n) {
+                char tmp[100];
+                sprintf(tmp, "%" PRIu64, list->value.n);
+                opts->desc[i].def_value_str = strdup(tmp);
+            }
+            break;
+
+        case OPT_STRING:
+            opts->desc[i].type = QEMU_OPT_STRING;
+            if (list->value.s) {
+                opts->desc[i].def_value_str = strdup(list->value.s);
+            }
+            break;
+        }
+
+        i++;
+        list++;
+        opts->desc[i].name = NULL;
+    }
+
+    return opts;
+}
+
+QEMUOptionParameter *opts_to_params(QemuOpts *opts)
+{
+    QEMUOptionParameter *dest = NULL;
+    QemuOptDesc *desc;
+    size_t num_opts, i = 0;
+    const char *tmp;
+
+    if (!opts || !opts->list || !opts->list->desc) {
+        return NULL;
+    }
+
+    num_opts = count_opts_list(opts->list);
+    dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
+    dest[i].name = NULL;
+
+    desc = opts->list->desc;
+    while (desc && desc->name) {
+        dest[i].name = strdup(desc->name);
+        dest[i].help = strdup(desc->help);
+        switch (desc->type) {
+        case QEMU_OPT_STRING:
+            dest[i].type = OPT_STRING;
+            tmp = qemu_opt_get(opts, desc->name);
+            if (tmp) {
+                dest[i].value.s = strdup(tmp);
+            }
+            break;
+
+        case QEMU_OPT_BOOL:
+            dest[i].type = OPT_FLAG;
+            dest[i].value.n = qemu_opt_get_bool(opts, desc->name, 0) ? 1 : 0;
+            break;
+
+        case QEMU_OPT_NUMBER:
+            dest[i].type = OPT_NUMBER;
+            dest[i].value.n = qemu_opt_get_number(opts, desc->name, 0);
+            break;
+
+        case QEMU_OPT_SIZE:
+            dest[i].type = OPT_SIZE;
+            dest[i].value.n = qemu_opt_get_size(opts, desc->name, 0);
+            break;
+        }
+
+        i++;
+        desc++;
+        dest[i].name = NULL;
+    }
+
+    return dest;
+}