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 <wdon...@linux.vnet.ibm.com>
> Signed-off-by: Chunyan Liu <cy...@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?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
