| Message ID | 1359022987-6274-4-git-send-email-wdongxu@vnet.linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > This patch will use QemuOpts related functions in block layer, add > a member bdrv_create_options to BlockDriver struct, it will return > a QemuOptsList pointer, which includes the image format's create > options. > > And create options's primary consumer is block creating related functions, > so modify them together. > > > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > --- > v10->v11) > 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or > qemu_opts_print produce un-expanded cluster_size. > 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, > or while using protocol, there will be an error. > > v8->v9) > 1) add qemu_ prefix to gluster_create_opts. > 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be > converted. > > v7->v8) > 1) rebase to upstream source tree. > 2) add gluster.c, raw-win32.c, and rbd.c. > > v6->v7: > 1) use osdep.h:stringify(), not redefining new macro. > 2) preserve TODO comment. > 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. > 4) initialize disk_type even when opts is NULL. > > v5->v6: > 1) judge if opts == NULL in block layer create functions. > 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. > 3) made more readable while using qemu_opt_get_number. > > > block.c | 91 ++++++++++++------------ > block/cow.c | 46 ++++++------- > block/gluster.c | 37 +++++----- > block/qcow.c | 60 ++++++++-------- > block/qcow2.c | 171 +++++++++++++++++++++++----------------------- > block/qed.c | 86 +++++++++++------------ > block/qed.h | 2 +- > block/raw-posix.c | 59 ++++++++-------- > block/raw-win32.c | 30 ++++---- > block/raw.c | 30 ++++---- > block/rbd.c | 62 ++++++++--------- > block/sheepdog.c | 75 ++++++++++---------- > block/vdi.c | 69 +++++++++---------- > block/vmdk.c | 74 ++++++++++---------- > block/vpc.c | 67 +++++++++--------- > block/vvfat.c | 11 +-- > include/block/block.h | 4 +- > include/block/block_int.h | 6 +- > qemu-img.c | 61 ++++++++--------- > 19 files changed, 519 insertions(+), 522 deletions(-) > > diff --git a/block.c b/block.c > index 6fa7c90..56e4613 100644 > --- a/block.c > +++ b/block.c > @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name) > typedef struct CreateCo { > BlockDriver *drv; > char *filename; > - QEMUOptionParameter *options; > + QemuOpts *opts; > int ret; > } CreateCo; > > @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) > CreateCo *cco = opaque; > assert(cco->drv); > > - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); > + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); > } > > int bdrv_create(BlockDriver *drv, const char* filename, > - QEMUOptionParameter *options) > + QemuOpts *opts) > { > int ret; > > @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, > CreateCo cco = { > .drv = drv, > .filename = g_strdup(filename), > - .options = options, > + .opts = opts, > .ret = NOT_DONE, > }; > > @@ -405,7 +405,7 @@ out: > return ret; > } > > -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) > +int bdrv_create_file(const char *filename, QemuOpts *opts) > { > BlockDriver *drv; > > @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) > return -ENOENT; > } > > - return bdrv_create(drv, filename, options); > + return bdrv_create(drv, filename, opts); > } > > /* > @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, > int64_t total_size; > int is_protocol = 0; > BlockDriver *bdrv_qcow2; > - QEMUOptionParameter *options; > + QemuOpts *opts; > char backing_filename[PATH_MAX]; > > /* if snapshot, we create a temporary backing file and open it > @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, > return -errno; > > bdrv_qcow2 = bdrv_find_format("qcow2"); > - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); > + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options); > > - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); > - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); > if (drv) { > - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, > - drv->format_name); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); > } > > - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); > - free_option_parameters(options); > + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); > + qemu_opts_del(opts); > if (ret < 0) { > return ret; > } > @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > const char *base_filename, const char *base_fmt, > char *options, uint64_t img_size, int flags, Error **errp) > { > - QEMUOptionParameter *param = NULL, *create_options = NULL; > - QEMUOptionParameter *backing_fmt, *backing_file, *size; > + QemuOpts *opts = NULL; > + QemuOptsList *create_options = NULL; > + const char *backing_fmt, *backing_file; > + int64_t size; > BlockDriverState *bs = NULL; > BlockDriver *drv, *proto_drv; > BlockDriver *backing_drv = NULL; > @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const char *fmt, > error_setg(errp, "Unknown protocol '%s'", filename); > return; > } > - > - create_options = append_option_parameters(create_options, > - drv->create_options); > - create_options = append_option_parameters(create_options, > - proto_drv->create_options); > - > + create_options = append_opts_list(drv->bdrv_create_options, > + proto_drv->bdrv_create_options); Before: format's options get appended first, then protocol's options. Because get_option_parameter() searches in order, format options shadow protocol options. After: append_opts_list() gives first argument's options precedence. Thus, no change. Good. Should bdrv_create_options option name clashes be avoided? Outside the scope of this patch. > /* Create parameter list with default values */ > - param = parse_option_parameters("", create_options, param); > + opts = qemu_opts_create_nofail(create_options); > > - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > + 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; > } > } Before: size from -o replaces img_size in param. After: size from -o gets appended to opts, is therefore shadowed by img_size. I think. User-visible change, if my reading is correct. Should be avoided. > > if (base_filename) { > - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, > + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, > base_filename)) { > error_setg(errp, "Backing file not supported for file format '%s'", > fmt); Before: base_filename replaces the backing_file from -o. After: base_filename gets appended to opts, shadowed by backing_file from -o. > @@ -4542,39 +4538,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; > } > } Likewise. > > - 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); > + error_setg(errp, "Unknown backing file format '%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_number(opts, BLOCK_OPT_SIZE, -1); > + if (size == -1) { > + if (backing_file) { Code has the same before your patch, so it's not your fault, but here goes anyway: QemuOpts support only *unsigned* numbers. Argument -1 is for an uint64_t parameter, and gets converted to UINT64_MAX. If the option isn't set, it's returned. The assignment to int64_t size converts it back to -1. Using the an honest UINT64_MAX instead of -1 would be clearer. Should make size uint64_t then. However, all other places do qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) Why is this one different? > uint64_t size; > - char buf[32]; > int back_flags; > > /* backing files always opened read-only */ > @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const char *fmt, > > bs = bdrv_new(""); > > - ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); > + ret = bdrv_open(bs, backing_file, back_flags, backing_drv); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not open '%s'", > - backing_file->value.s); > + backing_file); > goto out; > } > bdrv_get_geometry(bs, &size); > 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); > } else { > error_setg(errp, "Image creation needs a size parameter"); > goto out; > @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > } > > printf("Formatting '%s', fmt=%s ", filename, fmt); > - print_option_parameters(param); > + qemu_opts_print(opts, NULL); > puts(""); > > - ret = bdrv_create(drv, filename, param); > + ret = bdrv_create(drv, filename, opts); > if (ret < 0) { > if (ret == -ENOTSUP) { > error_setg(errp,"Formatting or formatting option not supported for " > @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > } > > out: > - free_option_parameters(create_options); > - free_option_parameters(param); > + free_opts_list(create_options); > + if (opts) { > + qemu_opts_del(opts); > + } > > if (bs) { > bdrv_delete(bs); > diff --git a/block/cow.c b/block/cow.c > index a33ce95..5442c9c 100644 > --- a/block/cow.c > +++ b/block/cow.c > @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs) > { > } > > -static int cow_create(const char *filename, QEMUOptionParameter *options) > +static int cow_create(const char *filename, QemuOpts *opts) > { > struct cow_header_v2 cow_header; > struct stat st; > @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) > int ret; > BlockDriverState *cow_bs; > > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - image_sectors = options->value.n / 512; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - image_filename = options->value.s; > - } > - options++; > + /* Read out opts */ > + if (opts) { I suspect you need the if (opts) here and in many other places only because you special-cased "both lists empty" in append_opts_list(). I suspect things become easier if you drop that. > + image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; > + image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > } > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -318,18 +314,22 @@ exit: > return ret; > } > > -static QEMUOptionParameter cow_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { NULL } > +static QemuOptsList cow_create_opts = { > + .name = "cow-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE, or else we lose support for suffixes, don't we? > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_cow = { > @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = { > .bdrv_write = cow_co_write, > .bdrv_co_is_allocated = cow_co_is_allocated, > > - .create_options = cow_create_options, > + .bdrv_create_options = &cow_create_opts, > }; > > static void bdrv_cow_init(void) > diff --git a/block/gluster.c b/block/gluster.c > index 0f2c32a..a41c684 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -335,8 +335,7 @@ out: > return ret; > } > > -static int qemu_gluster_create(const char *filename, > - QEMUOptionParameter *options) > +static int qemu_gluster_create(const char *filename, QemuOpts* opts) Space before the *, not after: QemuOpts *opts Running out of steam for today, rest of patch only skimmed. > { > struct glfs *glfs; > struct glfs_fd *fd; > @@ -350,11 +349,9 @@ static int qemu_gluster_create(const char *filename, > goto out; > } > > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / BDRV_SECTOR_SIZE; > - } > - options++; > + if (opts) { > + total_size = > + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > } > > fd = glfs_creat(glfs, gconf->image, > @@ -544,13 +541,17 @@ static void qemu_gluster_close(BlockDriverState *bs) > glfs_fini(s->glfs); > } > > -static QEMUOptionParameter qemu_gluster_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > +static QemuOptsList qemu_gluster_create_opts = { > + .name = "qemu-gluster-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_gluster = { > @@ -565,7 +566,7 @@ static BlockDriver bdrv_gluster = { > .bdrv_aio_readv = qemu_gluster_aio_readv, > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_options = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_tcp = { > @@ -580,7 +581,7 @@ static BlockDriver bdrv_gluster_tcp = { > .bdrv_aio_readv = qemu_gluster_aio_readv, > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_options = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_unix = { > @@ -595,7 +596,7 @@ static BlockDriver bdrv_gluster_unix = { > .bdrv_aio_readv = qemu_gluster_aio_readv, > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_options = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_rdma = { > @@ -610,7 +611,7 @@ static BlockDriver bdrv_gluster_rdma = { > .bdrv_aio_readv = qemu_gluster_aio_readv, > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_options = &qemu_gluster_create_opts, > }; > > static void bdrv_gluster_init(void) > diff --git a/block/qcow.c b/block/qcow.c > index 4276610..4691a3e 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -651,7 +651,7 @@ static void qcow_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static int qcow_create(const char *filename, QEMUOptionParameter *options) > +static int qcow_create(const char *filename, QemuOpts *opts) > { > int header_size, backing_filename_len, l1_size, shift, i; > QCowHeader header; > @@ -662,19 +662,16 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) > int ret; > BlockDriverState *qcow_bs; > > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / 512; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { > - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; > + /* Read out opts */ > + if (opts) { > + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { > + flags |= BLOCK_FLAG_ENCRYPT; > } > - options++; > } > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -851,24 +848,27 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) > return 0; > } > > - > -static QEMUOptionParameter qcow_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { > - .name = BLOCK_OPT_ENCRYPT, > - .type = OPT_FLAG, > - .help = "Encrypt the image" > - }, > - { NULL } > +static QemuOptsList qcow_create_opts = { > + .name = "qcow-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_ENCRYPT, > + .type = QEMU_OPT_BOOL, > + .help = "Encrypt the image" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_qcow = { > @@ -889,7 +889,7 @@ static BlockDriver bdrv_qcow = { > .bdrv_write_compressed = qcow_write_compressed, > .bdrv_get_info = qcow_get_info, > > - .create_options = qcow_create_options, > + .bdrv_create_options = &qcow_create_opts, > }; > > static void bdrv_qcow_init(void) > diff --git a/block/qcow2.c b/block/qcow2.c > index f6abff6..e3ee4af 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1177,7 +1177,7 @@ static int preallocate(BlockDriverState *bs) > static int qcow2_create2(const char *filename, int64_t total_size, > const char *backing_file, const char *backing_format, > int flags, size_t cluster_size, int prealloc, > - QEMUOptionParameter *options, int version) > + QemuOpts *opts, int version) > { > /* Calculate cluster_bits */ > int cluster_bits; > @@ -1208,7 +1208,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, > uint8_t* refcount_table; > int ret; > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -1311,7 +1311,7 @@ out: > return ret; > } > > -static int qcow2_create(const char *filename, QEMUOptionParameter *options) > +static int qcow2_create(const char *filename, QemuOpts *opts) > { > const char *backing_file = NULL; > const char *backing_fmt = NULL; > @@ -1320,45 +1320,43 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) > size_t cluster_size = DEFAULT_CLUSTER_SIZE; > int prealloc = 0; > int version = 2; > + const char *buf; > > /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - sectors = options->value.n / 512; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { > - backing_fmt = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { > - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; > - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { > - if (options->value.n) { > - cluster_size = options->value.n; > - } > - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { > - if (!options->value.s || !strcmp(options->value.s, "off")) { > - prealloc = 0; > - } else if (!strcmp(options->value.s, "metadata")) { > - prealloc = 1; > - } else { > - fprintf(stderr, "Invalid preallocation mode: '%s'\n", > - options->value.s); > - return -EINVAL; > - } > - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) { > - if (!options->value.s || !strcmp(options->value.s, "0.10")) { > - version = 2; > - } else if (!strcmp(options->value.s, "1.1")) { > - version = 3; > - } else { > - fprintf(stderr, "Invalid compatibility level: '%s'\n", > - options->value.s); > - return -EINVAL; > - } > - } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) { > - flags |= options->value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0; > + if (opts) { > + sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); > + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { > + flags |= BLOCK_FLAG_ENCRYPT; > + } > + cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, > + DEFAULT_CLUSTER_SIZE); > + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); > + if (!buf || !strcmp(buf, "off")) { > + prealloc = 0; > + } else if (!strcmp(buf, "metadata")) { > + prealloc = 1; > + } else { > + fprintf(stderr, "Invalid preallocation mode: '%s'\n", > + buf); > + return -EINVAL; > + } > + > + buf = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL); > + if (!buf || !strcmp(buf, "0.10")) { > + version = 2; > + } else if (!strcmp(buf, "1.1")) { > + version = 3; > + } else { > + fprintf(stderr, "Invalid compatibility level: '%s'\n", > + buf); > + return -EINVAL; > + } > + > + if (qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) { > + flags |= BLOCK_FLAG_LAZY_REFCOUNTS; > } > - options++; > } > > if (backing_file && prealloc) { > @@ -1374,7 +1372,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) > } > > return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, > - cluster_size, prealloc, options, version); > + cluster_size, prealloc, opts, version); > } > > static int qcow2_make_empty(BlockDriverState *bs) > @@ -1635,49 +1633,53 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, > return ret; > } > > -static QEMUOptionParameter qcow2_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_COMPAT_LEVEL, > - .type = OPT_STRING, > - .help = "Compatibility level (0.10 or 1.1)" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { > - .name = BLOCK_OPT_BACKING_FMT, > - .type = OPT_STRING, > - .help = "Image format of the base image" > - }, > - { > - .name = BLOCK_OPT_ENCRYPT, > - .type = OPT_FLAG, > - .help = "Encrypt the image" > - }, > - { > - .name = BLOCK_OPT_CLUSTER_SIZE, > - .type = OPT_SIZE, > - .help = "qcow2 cluster size", > - .value = { .n = DEFAULT_CLUSTER_SIZE }, > - }, > - { > - .name = BLOCK_OPT_PREALLOC, > - .type = OPT_STRING, > - .help = "Preallocation mode (allowed values: off, metadata)" > - }, > - { > - .name = BLOCK_OPT_LAZY_REFCOUNTS, > - .type = OPT_FLAG, > - .help = "Postpone refcount updates", > - }, > - { NULL } > +static QemuOptsList qcow2_create_opts = { > + .name = "qcow2-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_COMPAT_LEVEL, > + .type = QEMU_OPT_STRING, > + .help = "Compatibility level (0.10 or 1.1)" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_BACKING_FMT, > + .type = QEMU_OPT_STRING, > + .help = "Image format of the base image" > + }, > + { > + .name = BLOCK_OPT_ENCRYPT, > + .type = QEMU_OPT_BOOL, > + .help = "Encrypt the image" > + }, > + { > + .name = BLOCK_OPT_CLUSTER_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "qcow2 cluster size", > + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) > + }, > + { > + .name = BLOCK_OPT_PREALLOC, > + .type = QEMU_OPT_STRING, > + .help = "Preallocation mode (allowed values: off, metadata)" > + }, > + { > + .name = BLOCK_OPT_LAZY_REFCOUNTS, > + .type = QEMU_OPT_BOOL, > + .help = "Postpone refcount updates", > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_qcow2 = { > @@ -1715,8 +1717,9 @@ static BlockDriver bdrv_qcow2 = { > > .bdrv_invalidate_cache = qcow2_invalidate_cache, > > - .create_options = qcow2_create_options, > .bdrv_check = qcow2_check, > + > + .bdrv_create_options = &qcow2_create_opts, > }; > > static void bdrv_qcow2_init(void) > diff --git a/block/qed.c b/block/qed.c > index cf85d8f..766ba6f 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -603,7 +603,7 @@ out: > return ret; > } > > -static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) > +static int bdrv_qed_create(const char *filename, QemuOpts *opts) > { > uint64_t image_size = 0; > uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE; > @@ -611,23 +611,15 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) > const char *backing_file = NULL; > const char *backing_fmt = NULL; > > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - image_size = options->value.n; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { > - backing_fmt = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { > - if (options->value.n) { > - cluster_size = options->value.n; > - } > - } else if (!strcmp(options->name, BLOCK_OPT_TABLE_SIZE)) { > - if (options->value.n) { > - table_size = options->value.n; > - } > - } > - options++; > + if (opts) { > + image_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); > + cluster_size = qemu_opt_get_size(opts, > + BLOCK_OPT_CLUSTER_SIZE, > + QED_DEFAULT_CLUSTER_SIZE); > + table_size = qemu_opt_get_size(opts, BLOCK_OPT_TABLE_SIZE, > + QED_DEFAULT_TABLE_SIZE); > } > > if (!qed_is_cluster_size_valid(cluster_size)) { > @@ -1537,36 +1529,44 @@ static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result, > return qed_check(s, result, !!fix); > } > > -static QEMUOptionParameter qed_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size (in bytes)" > - }, { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, { > - .name = BLOCK_OPT_BACKING_FMT, > - .type = OPT_STRING, > - .help = "Image format of the base image" > - }, { > - .name = BLOCK_OPT_CLUSTER_SIZE, > - .type = OPT_SIZE, > - .help = "Cluster size (in bytes)", > - .value = { .n = QED_DEFAULT_CLUSTER_SIZE }, > - }, { > - .name = BLOCK_OPT_TABLE_SIZE, > - .type = OPT_SIZE, > - .help = "L1/L2 table size (in clusters)" > - }, > - { /* end of list */ } > +static QemuOptsList qed_create_opts = { > + .name = "qed-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(qed_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_BACKING_FMT, > + .type = QEMU_OPT_STRING, > + .help = "Image format of the base image" > + }, > + { > + .name = BLOCK_OPT_CLUSTER_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Cluster size (in bytes)", > + .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE), > + }, > + { > + .name = BLOCK_OPT_TABLE_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "L1/L2 table size (in clusters)" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_qed = { > .format_name = "qed", > .instance_size = sizeof(BDRVQEDState), > - .create_options = qed_create_options, > + .bdrv_create_options = &qed_create_opts, > > .bdrv_probe = bdrv_qed_probe, > .bdrv_rebind = bdrv_qed_rebind, > diff --git a/block/qed.h b/block/qed.h > index 2b4dded..99a7726 100644 > --- a/block/qed.h > +++ b/block/qed.h > @@ -43,6 +43,7 @@ > * > * All fields are little-endian on disk. > */ > +#define QED_DEFAULT_CLUSTER_SIZE 65536 > > enum { > QED_MAGIC = 'Q' | 'E' << 8 | 'D' << 16 | '\0' << 24, > @@ -69,7 +70,6 @@ enum { > */ > QED_MIN_CLUSTER_SIZE = 4 * 1024, /* in bytes */ > QED_MAX_CLUSTER_SIZE = 64 * 1024 * 1024, > - QED_DEFAULT_CLUSTER_SIZE = 64 * 1024, > > /* Allocated clusters are tracked using a 2-level pagetable. Table size is > * a multiple of clusters so large maximum image sizes can be supported > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 657af95..890c8a7 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -123,6 +123,19 @@ > > #define MAX_BLOCKSIZE 4096 > > +static QemuOptsList file_proto_create_opts = { > + .name = "file-proto-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(file_proto_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > +}; > + > typedef struct BDRVRawState { > int fd; > int type; > @@ -997,18 +1010,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) > return (int64_t)st.st_blocks * 512; > } > > -static int raw_create(const char *filename, QEMUOptionParameter *options) > +static int raw_create(const char *filename, QemuOpts *opts) > { > int fd; > int result = 0; > int64_t total_size = 0; > > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / BDRV_SECTOR_SIZE; > - } > - options++; > + if (opts) { > + total_size = > + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > } > > fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, > @@ -1136,15 +1146,6 @@ static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs, > cb, opaque, QEMU_AIO_DISCARD); > } > > -static QEMUOptionParameter raw_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > -}; > - > static BlockDriver bdrv_file = { > .format_name = "file", > .protocol_name = "file", > @@ -1167,8 +1168,7 @@ static BlockDriver bdrv_file = { > .bdrv_getlength = raw_getlength, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > - > - .create_options = raw_create_options, > + .bdrv_create_options = &file_proto_create_opts, > }; > > /***********************************************/ > @@ -1403,19 +1403,16 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, > cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); > } > > -static int hdev_create(const char *filename, QEMUOptionParameter *options) > +static int hdev_create(const char *filename, QemuOpts *opts) > { > int fd; > int ret = 0; > struct stat stat_buf; > int64_t total_size = 0; > > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, "size")) { > - total_size = options->value.n / BDRV_SECTOR_SIZE; > - } > - options++; > + if (opts) { > + total_size = > + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > } > > fd = qemu_open(filename, O_WRONLY | O_BINARY); > @@ -1440,7 +1437,7 @@ static int hdev_has_zero_init(BlockDriverState *bs) > > static BlockDriver bdrv_host_device = { > .format_name = "host_device", > - .protocol_name = "host_device", > + .protocol_name = "host_device", > .instance_size = sizeof(BDRVRawState), > .bdrv_probe_device = hdev_probe_device, > .bdrv_file_open = hdev_open, > @@ -1449,7 +1446,6 @@ static BlockDriver bdrv_host_device = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = hdev_has_zero_init, > > .bdrv_aio_readv = raw_aio_readv, > @@ -1458,9 +1454,10 @@ static BlockDriver bdrv_host_device = { > .bdrv_aio_discard = hdev_aio_discard, > > .bdrv_truncate = raw_truncate, > - .bdrv_getlength = raw_getlength, > + .bdrv_getlength = raw_getlength, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > + .bdrv_create_options = &file_proto_create_opts, > > /* generic scsi device */ > #ifdef __linux__ > @@ -1574,7 +1571,6 @@ static BlockDriver bdrv_host_floppy = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = hdev_has_zero_init, > > .bdrv_aio_readv = raw_aio_readv, > @@ -1590,6 +1586,7 @@ static BlockDriver bdrv_host_floppy = { > .bdrv_is_inserted = floppy_is_inserted, > .bdrv_media_changed = floppy_media_changed, > .bdrv_eject = floppy_eject, > + .bdrv_create_options = &file_proto_create_opts, > }; > > static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) > @@ -1676,7 +1673,6 @@ static BlockDriver bdrv_host_cdrom = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = hdev_has_zero_init, > > .bdrv_aio_readv = raw_aio_readv, > @@ -1696,6 +1692,8 @@ static BlockDriver bdrv_host_cdrom = { > /* generic scsi device */ > .bdrv_ioctl = hdev_ioctl, > .bdrv_aio_ioctl = hdev_aio_ioctl, > + > + .bdrv_create_options = &file_proto_create_opts, > }; > #endif /* __linux__ */ > > @@ -1798,7 +1796,6 @@ static BlockDriver bdrv_host_cdrom = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = hdev_has_zero_init, > > .bdrv_aio_readv = raw_aio_readv, > diff --git a/block/raw-win32.c b/block/raw-win32.c > index b89ac19..34826e0 100644 > --- a/block/raw-win32.c > +++ b/block/raw-win32.c > @@ -382,17 +382,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) > return st.st_size; > } > > -static int raw_create(const char *filename, QEMUOptionParameter *options) > +static int raw_create(const char *filename, QemuOpts *opts) > { > int fd; > int64_t total_size = 0; > > /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / 512; > - } > - options++; > + if (opts) { > + total_size = > + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; > } > > fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, > @@ -405,13 +403,17 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) > return 0; > } > > -static QEMUOptionParameter raw_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > +static QemuOptsList raw_create_opts = { > + .name = "raw-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_file = { > @@ -431,7 +433,7 @@ static BlockDriver bdrv_file = { > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > - .create_options = raw_create_options, > + .bdrv_create_options = &raw_create_opts, > }; > > /***********************************************/ > diff --git a/block/raw.c b/block/raw.c > index 75812db..033ffa6 100644 > --- a/block/raw.c > +++ b/block/raw.c > @@ -95,18 +95,22 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, > return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque); > } > > -static int raw_create(const char *filename, QEMUOptionParameter *options) > -{ > - return bdrv_create_file(filename, options); > -} > - > -static QEMUOptionParameter raw_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > +static int raw_create(const char *filename, QemuOpts *opts) > +{ > + return bdrv_create_file(filename, opts); > +} > + > +static QemuOptsList raw_create_opts = { > + .name = "raw-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > static int raw_has_zero_init(BlockDriverState *bs) > @@ -143,8 +147,8 @@ static BlockDriver bdrv_raw = { > .bdrv_aio_ioctl = raw_aio_ioctl, > > .bdrv_create = raw_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = raw_has_zero_init, > + .bdrv_create_options = &raw_create_opts, > }; > > static void bdrv_raw_init(void) > diff --git a/block/rbd.c b/block/rbd.c > index 8cd10a7..a9f8772 100644 > --- a/block/rbd.c > +++ b/block/rbd.c > @@ -287,7 +287,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf) > return ret; > } > > -static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) > +static int qemu_rbd_create(const char *filename, QemuOpts *opts) > { > int64_t bytes = 0; > int64_t objsize; > @@ -310,24 +310,20 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) > } > > /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - bytes = options->value.n; > - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { > - if (options->value.n) { > - objsize = options->value.n; > - if ((objsize - 1) & objsize) { /* not a power of 2? */ > - error_report("obj size needs to be power of 2"); > - return -EINVAL; > - } > - if (objsize < 4096) { > - error_report("obj size too small"); > - return -EINVAL; > - } > - obj_order = ffs(objsize) - 1; > + if (opts) { > + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > + objsize = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, 0); > + if (objsize) { > + if ((objsize - 1) & objsize) { /* not a power of 2? */ > + error_report("obj size needs to be power of 2"); > + return -EINVAL; > } > + if (objsize < 4096) { > + error_report("obj size too small"); > + return -EINVAL; > + } > + obj_order = ffs(objsize) - 1; > } > - options++; > } > > clientname = qemu_rbd_parse_clientname(conf, clientname_buf); > @@ -920,20 +916,24 @@ static BlockDriverAIOCB* qemu_rbd_aio_discard(BlockDriverState *bs, > } > #endif > > -static QEMUOptionParameter qemu_rbd_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_CLUSTER_SIZE, > - .type = OPT_SIZE, > - .help = "RBD object size" > - }, > - {NULL} > +static QemuOptsList rbd_create_opts = { > + .name = "rbd-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(rbd_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_CLUSTER_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "RBD object size", > + .def_value_str = stringify(0), > + }, > + { /* end of list */ } > + } > }; > - > static BlockDriver bdrv_rbd = { > .format_name = "rbd", > .instance_size = sizeof(BDRVRBDState), > @@ -941,7 +941,7 @@ static BlockDriver bdrv_rbd = { > .bdrv_close = qemu_rbd_close, > .bdrv_create = qemu_rbd_create, > .bdrv_get_info = qemu_rbd_getinfo, > - .create_options = qemu_rbd_create_options, > + .bdrv_create_options = &rbd_create_opts, > .bdrv_getlength = qemu_rbd_getlength, > .bdrv_truncate = qemu_rbd_truncate, > .protocol_name = "rbd", > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 3e49bb8..d515e56 100644 > --- a/block/sheepdog.c > +++ b/block/sheepdog.c > @@ -1274,12 +1274,12 @@ out: > return ret; > } > > -static int sd_create(const char *filename, QEMUOptionParameter *options) > +static int sd_create(const char *filename, QemuOpts *opts) > { > int ret = 0; > uint32_t vid = 0, base_vid = 0; > int64_t vdi_size = 0; > - char *backing_file = NULL; > + const char *backing_file = NULL, *buf = NULL; > BDRVSheepdogState *s; > char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; > uint32_t snapid; > @@ -1298,26 +1298,20 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) > goto out; > } > > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - vdi_size = options->value.n; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { > - if (!options->value.s || !strcmp(options->value.s, "off")) { > - prealloc = false; > - } else if (!strcmp(options->value.s, "full")) { > - prealloc = true; > - } else { > - error_report("Invalid preallocation mode: '%s'", > - options->value.s); > - ret = -EINVAL; > - goto out; > - } > + if (opts) { > + vdi_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); > + if (!buf || !strcmp(buf, "off")) { > + prealloc = false; > + } else if (!strcmp(buf, "full")) { > + prealloc = true; > + } else { > + error_report("Invalid preallocation mode: '%s'", buf); > + ret = -EINVAL; > + goto out; > } > - options++; > } > - > if (vdi_size > SD_MAX_VDI_SIZE) { > error_report("too big image size"); > ret = -EINVAL; > @@ -2042,24 +2036,27 @@ static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data, > return do_load_save_vmstate(s, data, pos, size, 1); > } > > - > -static QEMUOptionParameter sd_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { > - .name = BLOCK_OPT_PREALLOC, > - .type = OPT_STRING, > - .help = "Preallocation mode (allowed values: off, full)" > - }, > - { NULL } > +static QemuOptsList sd_create_opts = { > + .name = "sheepdog-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_PREALLOC, > + .type = QEMU_OPT_STRING, > + .help = "Preallocation mode (allowed values: off, full)" > + }, > + { /* end of list */ } > + } > }; > > BlockDriver bdrv_sheepdog = { > @@ -2084,7 +2081,7 @@ BlockDriver bdrv_sheepdog = { > .bdrv_save_vmstate = sd_save_vmstate, > .bdrv_load_vmstate = sd_load_vmstate, > > - .create_options = sd_create_options, > + .bdrv_create_options = &sd_create_opts, > }; > > static void bdrv_sheepdog_init(void) > diff --git a/block/vdi.c b/block/vdi.c > index 021abaa..95f738e 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -620,7 +620,7 @@ static int vdi_co_write(BlockDriverState *bs, > return ret; > } > > -static int vdi_create(const char *filename, QEMUOptionParameter *options) > +static int vdi_create(const char *filename, QemuOpts *opts) > { > int fd; > int result = 0; > @@ -635,24 +635,19 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) > logout("\n"); > > /* Read out options. */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - bytes = options->value.n; > + if (opts) { > + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > #if defined(CONFIG_VDI_BLOCK_SIZE) > - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { > - if (options->value.n) { > - /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ > - block_size = options->value.n; > - } > + /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ > + block_size = qemu_opt_get_size(opts, > + BLOCK_OPT_CLUSTER_SIZE, > + DEFAULT_CLUSTER_SIZE); > #endif > #if defined(CONFIG_VDI_STATIC_IMAGE) > - } else if (!strcmp(options->name, BLOCK_OPT_STATIC)) { > - if (options->value.n) { > - image_type = VDI_TYPE_STATIC; > - } > -#endif > + if (qemu_opt_get_bool(opts, BLOCK_OPT_STATIC, 0)) { > + image_type = VDI_TYPE_STATIC; > } > - options++; > +#endif > } > > fd = qemu_open(filename, > @@ -733,29 +728,33 @@ static void vdi_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static QEMUOptionParameter vdi_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > +static QemuOptsList vdi_create_opts = { > + .name = "vdi-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > #if defined(CONFIG_VDI_BLOCK_SIZE) > - { > - .name = BLOCK_OPT_CLUSTER_SIZE, > - .type = OPT_SIZE, > - .help = "VDI cluster (block) size", > - .value = { .n = DEFAULT_CLUSTER_SIZE }, > - }, > + { > + .name = BLOCK_OPT_CLUSTER_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "VDI cluster (block) size", > + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) > + }, > #endif > #if defined(CONFIG_VDI_STATIC_IMAGE) > - { > - .name = BLOCK_OPT_STATIC, > - .type = OPT_FLAG, > - .help = "VDI static (pre-allocated) image" > - }, > + { > + .name = BLOCK_OPT_STATIC, > + .type = QEMU_OPT_BOOL, > + .help = "VDI static (pre-allocated) image" > + }, > #endif > - /* TODO: An additional option to set UUID values might be useful. */ > - { NULL } > + /* TODO: An additional option to set UUID values might be useful. */ > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vdi = { > @@ -776,7 +775,7 @@ static BlockDriver bdrv_vdi = { > > .bdrv_get_info = vdi_get_info, > > - .create_options = vdi_create_options, > + .bdrv_create_options = &vdi_create_opts, > .bdrv_check = vdi_check, > }; > > diff --git a/block/vmdk.c b/block/vmdk.c > index 19298c2..4c29927 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1437,7 +1437,7 @@ static int relative_path(char *dest, int dest_size, > return 0; > } > > -static int vmdk_create(const char *filename, QEMUOptionParameter *options) > +static int vmdk_create(const char *filename, QemuOpts *opts) > { > int fd, idx = 0; > char desc[BUF_SIZE]; > @@ -1476,18 +1476,14 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { > return -EINVAL; > } > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) { > - flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0; > - } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) { > - fmt = options->value.s; > + /* Read out opts */ > + if (opts) { > + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); > + if (qemu_opt_get_bool(opts, BLOCK_OPT_COMPAT6, 0)) { > + flags |= BLOCK_FLAG_COMPAT6; > } > - options++; > + fmt = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); > } > if (!fmt) { > /* Default format to monolithicSparse */ > @@ -1654,30 +1650,34 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs) > return ret; > } > > -static QEMUOptionParameter vmdk_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { > - .name = BLOCK_OPT_COMPAT6, > - .type = OPT_FLAG, > - .help = "VMDK version 6 image" > - }, > - { > - .name = BLOCK_OPT_SUBFMT, > - .type = OPT_STRING, > - .help = > - "VMDK flat extent format, can be one of " > - "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " > - }, > - { NULL } > +static QemuOptsList vmdk_create_opts = { > + .name = "vmdk-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_COMPAT6, > + .type = QEMU_OPT_BOOL, > + .help = "VMDK version 6 image" > + }, > + { > + .name = BLOCK_OPT_SUBFMT, > + .type = QEMU_OPT_STRING, > + .help = > + "VMDK flat extent format, can be one of " > + "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vmdk = { > @@ -1694,7 +1694,7 @@ static BlockDriver bdrv_vmdk = { > .bdrv_co_is_allocated = vmdk_co_is_allocated, > .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, > > - .create_options = vmdk_create_options, > + .bdrv_create_options = &vmdk_create_opts, > }; > > static void bdrv_vmdk_init(void) > diff --git a/block/vpc.c b/block/vpc.c > index 7948609..fcfdc79 100644 > --- a/block/vpc.c > +++ b/block/vpc.c > @@ -665,34 +665,33 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size) > return ret; > } > > -static int vpc_create(const char *filename, QEMUOptionParameter *options) > +static int vpc_create(const char *filename, QemuOpts *opts) > { > uint8_t buf[1024]; > struct vhd_footer *footer = (struct vhd_footer *) buf; > - QEMUOptionParameter *disk_type_param; > + const char *disk_type_param = NULL; > int fd, i; > uint16_t cyls = 0; > uint8_t heads = 0; > uint8_t secs_per_cyl = 0; > int64_t total_sectors; > - int64_t total_size; > - int disk_type; > + int64_t total_size = 0; > + int disk_type = VHD_DYNAMIC; > int ret = -EIO; > > - /* Read out options */ > - total_size = get_option_parameter(options, BLOCK_OPT_SIZE)->value.n; > - > - disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT); > - if (disk_type_param && disk_type_param->value.s) { > - if (!strcmp(disk_type_param->value.s, "dynamic")) { > - disk_type = VHD_DYNAMIC; > - } else if (!strcmp(disk_type_param->value.s, "fixed")) { > - disk_type = VHD_FIXED; > - } else { > - return -EINVAL; > + /* Read out opts */ > + if (opts) { > + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); > + disk_type_param = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); > + if (disk_type_param) { > + if (!strcmp(disk_type_param, "dynamic")) { > + disk_type = VHD_DYNAMIC; > + } else if (!strcmp(disk_type_param, "fixed")) { > + disk_type = VHD_FIXED; > + } else { > + return -EINVAL; > + } > } > - } else { > - disk_type = VHD_DYNAMIC; > } > > /* Create the file */ > @@ -780,20 +779,24 @@ static void vpc_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static QEMUOptionParameter vpc_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_SUBFMT, > - .type = OPT_STRING, > - .help = > - "Type of virtual hard disk format. Supported formats are " > - "{dynamic (default) | fixed} " > - }, > - { NULL } > +static QemuOptsList vpc_create_opts = { > + .name = "vpc-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_NUMBER, QEMU_OPT_SIZE again. > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_SUBFMT, > + .type = OPT_STRING, > + .help = > + "Type of virtual hard disk format. Supported formats are " > + "{dynamic (default) | fixed} " > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vpc = { > @@ -809,7 +812,7 @@ static BlockDriver bdrv_vpc = { > .bdrv_read = vpc_co_read, > .bdrv_write = vpc_co_write, > > - .create_options = vpc_create_options, > + .bdrv_create_options = &vpc_create_opts, > }; > > static void bdrv_vpc_init(void) > diff --git a/block/vvfat.c b/block/vvfat.c > index 06e6654..2343cd4 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -2802,7 +2802,7 @@ static BlockDriver vvfat_write_target = { > static int enable_write_target(BDRVVVFATState *s) > { > BlockDriver *bdrv_qcow; > - QEMUOptionParameter *options; > + QemuOpts *opts; > int ret; > int size = sector2cluster(s, s->sector_count); > s->used_clusters = calloc(size, 1); > @@ -2818,12 +2818,13 @@ static int enable_write_target(BDRVVVFATState *s) > } > > bdrv_qcow = bdrv_find_format("qcow"); > - options = parse_option_parameters("", bdrv_qcow->create_options, NULL); > - set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512); > - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:"); > + opts = qemu_opts_create_nofail(bdrv_qcow->bdrv_create_options); > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); > > - if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0) > + if (bdrv_create(bdrv_qcow, s->qcow_filename, opts) < 0) { > return -1; > + } > > s->qcow = bdrv_new(""); > if (s->qcow == NULL) { > diff --git a/include/block/block.h b/include/block/block.h > index ffd1936..c99e8bf 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -124,8 +124,8 @@ BlockDriver *bdrv_find_protocol(const char *filename); > BlockDriver *bdrv_find_format(const char *format_name); > BlockDriver *bdrv_find_whitelisted_format(const char *format_name); > int bdrv_create(BlockDriver *drv, const char* filename, > - QEMUOptionParameter *options); > -int bdrv_create_file(const char* filename, QEMUOptionParameter *options); > + QemuOpts *options); > +int bdrv_create_file(const char *filename, QemuOpts *options); > 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 f83ffb8..71f7670 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -88,7 +88,7 @@ struct BlockDriver { > const uint8_t *buf, int nb_sectors); > void (*bdrv_close)(BlockDriverState *bs); > void (*bdrv_rebind)(BlockDriverState *bs); > - int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); > + int (*bdrv_create)(const char *filename, QemuOpts *options); > int (*bdrv_set_key)(BlockDriverState *bs, const char *key); > int (*bdrv_make_empty)(BlockDriverState *bs); > /* aio */ > @@ -177,9 +177,7 @@ struct BlockDriver { > unsigned long int req, void *buf, > BlockDriverCompletionFunc *cb, void *opaque); > > - /* List of options for creating images, terminated by name == NULL */ > - QEMUOptionParameter *create_options; > - > + QemuOptsList *bdrv_create_options; Since you're renaming this anyway, consider renaming to bdrv_create_opts, to match the names used elsewhere. > > /* > * Returns 0 for completed check, -errno for internal errors. > diff --git a/qemu-img.c b/qemu-img.c > index 85d3740..b4b3ce8 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -200,7 +200,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_options = NULL; > > /* Find driver and parse its options */ > drv = bdrv_find_format(fmt); > @@ -215,12 +215,10 @@ 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); > - free_option_parameters(create_options); > + create_options = append_opts_list(drv->bdrv_create_options, > + proto_drv->bdrv_create_options); > + print_opts_list(create_options); > + free_opts_list(create_options); > return 0; > } > > @@ -271,19 +269,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 *list, > 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(list, 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(list, BLOCK_OPT_BACKING_FMT, base_fmt)) { > error_report("Backing file format not supported for file " > "format '%s'", fmt); > return -1; > @@ -675,8 +673,9 @@ static int img_convert(int argc, char **argv) static int img_convert(int argc, char **argv) { int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors; int progress = 0, flags; const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; BlockDriver *drv, *proto_drv; BlockDriverState **bs = NULL, *out_bs = NULL; int64_t total_sectors, nb_sectors, sector_num, bs_offset; Some or all of these should probably be uint64_t. Not your patch's fault, feel free to ignore. uint64_t bs_sectors; > uint8_t * buf = NULL; > const uint8_t *buf1; > BlockDriverInfo bdi; > - QEMUOptionParameter *param = NULL, *create_options = NULL; > - QEMUOptionParameter *out_baseimg_param; > + QemuOpts *param = NULL; > + QemuOptsList *create_options = NULL; > + const char *out_baseimg_param; > char *options = NULL; > const char *snapshot_name = NULL; > float local_progress = 0; > @@ -811,40 +810,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_options = append_opts_list(drv->bdrv_create_options, > + proto_drv->bdrv_create_options); > > if (options) { > - param = parse_option_parameters(options, create_options, param); > - if (param == NULL) { > + if (qemu_opts_do_parse(param, options, NULL) != 0) { > error_report("Invalid options for file format '%s'.", out_fmt); > ret = -1; > goto out; > } > } else { > - param = parse_option_parameters("", create_options, param); > + param = qemu_opts_create_nofail(create_options); > } > - > - set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512); > + qemu_opt_set_number(param, BLOCK_OPT_SIZE, total_sectors * 512); > ret = add_old_style_options(out_fmt, param, 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(param, 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(param, BLOCK_OPT_ENCRYPT, false); > + const char *preallocation = > + qemu_opt_get(param, BLOCK_OPT_PREALLOC); > > if (!drv->bdrv_write_compressed) { > error_report("Compression not supported for this file format"); > @@ -852,15 +847,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"); > @@ -1078,8 +1073,10 @@ static int img_convert(int argc, char **argv) > } > out: > qemu_progress_end(); > - free_option_parameters(create_options); > - free_option_parameters(param); > + free_opts_list(create_options); > + if (param) { > + qemu_opts_del(param); > + } > qemu_vfree(buf); > if (out_bs) { > bdrv_delete(out_bs);
于 2013-1-25 4:08, Markus Armbruster 写道: > Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > >> This patch will use QemuOpts related functions in block layer, add >> a member bdrv_create_options to BlockDriver struct, it will return >> a QemuOptsList pointer, which includes the image format's create >> options. >> >> And create options's primary consumer is block creating related functions, >> so modify them together. >> >> >> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >> --- >> v10->v11) >> 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or >> qemu_opts_print produce un-expanded cluster_size. >> 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, >> or while using protocol, there will be an error. >> >> v8->v9) >> 1) add qemu_ prefix to gluster_create_opts. >> 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be >> converted. >> >> v7->v8) >> 1) rebase to upstream source tree. >> 2) add gluster.c, raw-win32.c, and rbd.c. >> >> v6->v7: >> 1) use osdep.h:stringify(), not redefining new macro. >> 2) preserve TODO comment. >> 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. >> 4) initialize disk_type even when opts is NULL. >> >> v5->v6: >> 1) judge if opts == NULL in block layer create functions. >> 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. >> 3) made more readable while using qemu_opt_get_number. >> >> >> block.c | 91 ++++++++++++------------ >> block/cow.c | 46 ++++++------- >> block/gluster.c | 37 +++++----- >> block/qcow.c | 60 ++++++++-------- >> block/qcow2.c | 171 +++++++++++++++++++++++----------------------- >> block/qed.c | 86 +++++++++++------------ >> block/qed.h | 2 +- >> block/raw-posix.c | 59 ++++++++-------- >> block/raw-win32.c | 30 ++++---- >> block/raw.c | 30 ++++---- >> block/rbd.c | 62 ++++++++--------- >> block/sheepdog.c | 75 ++++++++++---------- >> block/vdi.c | 69 +++++++++---------- >> block/vmdk.c | 74 ++++++++++---------- >> block/vpc.c | 67 +++++++++--------- >> block/vvfat.c | 11 +-- >> include/block/block.h | 4 +- >> include/block/block_int.h | 6 +- >> qemu-img.c | 61 ++++++++--------- >> 19 files changed, 519 insertions(+), 522 deletions(-) >> >> diff --git a/block.c b/block.c >> index 6fa7c90..56e4613 100644 >> --- a/block.c >> +++ b/block.c >> @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name) >> typedef struct CreateCo { >> BlockDriver *drv; >> char *filename; >> - QEMUOptionParameter *options; >> + QemuOpts *opts; >> int ret; >> } CreateCo; >> >> @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) >> CreateCo *cco = opaque; >> assert(cco->drv); >> >> - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); >> + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); >> } >> >> int bdrv_create(BlockDriver *drv, const char* filename, >> - QEMUOptionParameter *options) >> + QemuOpts *opts) >> { >> int ret; >> >> @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, >> CreateCo cco = { >> .drv = drv, >> .filename = g_strdup(filename), >> - .options = options, >> + .opts = opts, >> .ret = NOT_DONE, >> }; >> >> @@ -405,7 +405,7 @@ out: >> return ret; >> } >> >> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >> +int bdrv_create_file(const char *filename, QemuOpts *opts) >> { >> BlockDriver *drv; >> >> @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >> return -ENOENT; >> } >> >> - return bdrv_create(drv, filename, options); >> + return bdrv_create(drv, filename, opts); >> } >> >> /* >> @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >> int64_t total_size; >> int is_protocol = 0; >> BlockDriver *bdrv_qcow2; >> - QEMUOptionParameter *options; >> + QemuOpts *opts; >> char backing_filename[PATH_MAX]; >> >> /* if snapshot, we create a temporary backing file and open it >> @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >> return -errno; >> >> bdrv_qcow2 = bdrv_find_format("qcow2"); >> - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); >> + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options); >> >> - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); >> - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); >> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); >> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); >> if (drv) { >> - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, >> - drv->format_name); >> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); >> } >> >> - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); >> - free_option_parameters(options); >> + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); >> + qemu_opts_del(opts); >> if (ret < 0) { >> return ret; >> } >> @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> const char *base_filename, const char *base_fmt, >> char *options, uint64_t img_size, int flags, Error **errp) >> { >> - QEMUOptionParameter *param = NULL, *create_options = NULL; >> - QEMUOptionParameter *backing_fmt, *backing_file, *size; >> + QemuOpts *opts = NULL; >> + QemuOptsList *create_options = NULL; >> + const char *backing_fmt, *backing_file; >> + int64_t size; >> BlockDriverState *bs = NULL; >> BlockDriver *drv, *proto_drv; >> BlockDriver *backing_drv = NULL; >> @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const char *fmt, >> error_setg(errp, "Unknown protocol '%s'", filename); >> return; >> } >> - >> - create_options = append_option_parameters(create_options, >> - drv->create_options); >> - create_options = append_option_parameters(create_options, >> - proto_drv->create_options); >> - >> + create_options = append_opts_list(drv->bdrv_create_options, >> + proto_drv->bdrv_create_options); > > Before: format's options get appended first, then protocol's options. > Because get_option_parameter() searches in order, format options shadow > protocol options. > > After: append_opts_list() gives first argument's options precedence. > > Thus, no change. Good. > > Should bdrv_create_options option name clashes be avoided? Outside the > scope of this patch. Sorry, I do not understand this line, clash? Can you explain some more? Do you mean bdrv_create_options should be renamed such as bdrv_create_opts? > >> /* Create parameter list with default values */ >> - param = parse_option_parameters("", create_options, param); >> + opts = qemu_opts_create_nofail(create_options); >> >> - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >> + 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; >> } >> } > > Before: size from -o replaces img_size in param. > > After: size from -o gets appended to opts, is therefore shadowed by > img_size. I think. > > User-visible change, if my reading is correct. Should be avoided. > Okay. will also "replace", not "append". >> >> if (base_filename) { >> - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, >> + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, >> base_filename)) { >> error_setg(errp, "Backing file not supported for file format '%s'", >> fmt); > > Before: base_filename replaces the backing_file from -o. > > After: base_filename gets appended to opts, shadowed by backing_file > from -o. > >> @@ -4542,39 +4538,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; >> } >> } > > Likewise. > >> >> - 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); >> + error_setg(errp, "Unknown backing file format '%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_number(opts, BLOCK_OPT_SIZE, -1); >> + if (size == -1) { >> + if (backing_file) { > > Code has the same before your patch, so it's not your fault, but here > goes anyway: > > QemuOpts support only *unsigned* numbers. Argument -1 is for an > uint64_t parameter, and gets converted to UINT64_MAX. If the option > isn't set, it's returned. The assignment to int64_t size converts it > back to -1. > > Using the an honest UINT64_MAX instead of -1 would be clearer. Should > make size uint64_t then. > > However, all other places do > > qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) > > Why is this one different? > Because previous use -1, so, I passed default value -1. >> uint64_t size; >> - char buf[32]; >> int back_flags; >> >> /* backing files always opened read-only */ >> @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const char *fmt, >> >> bs = bdrv_new(""); >> >> - ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); >> + ret = bdrv_open(bs, backing_file, back_flags, backing_drv); >> if (ret < 0) { >> error_setg_errno(errp, -ret, "Could not open '%s'", >> - backing_file->value.s); >> + backing_file); >> goto out; >> } >> bdrv_get_geometry(bs, &size); >> 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); >> } else { >> error_setg(errp, "Image creation needs a size parameter"); >> goto out; >> @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> } >> >> printf("Formatting '%s', fmt=%s ", filename, fmt); >> - print_option_parameters(param); >> + qemu_opts_print(opts, NULL); >> puts(""); >> >> - ret = bdrv_create(drv, filename, param); >> + ret = bdrv_create(drv, filename, opts); >> if (ret < 0) { >> if (ret == -ENOTSUP) { >> error_setg(errp,"Formatting or formatting option not supported for " >> @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> } >> >> out: >> - free_option_parameters(create_options); >> - free_option_parameters(param); >> + free_opts_list(create_options); >> + if (opts) { >> + qemu_opts_del(opts); >> + } >> >> if (bs) { >> bdrv_delete(bs); >> diff --git a/block/cow.c b/block/cow.c >> index a33ce95..5442c9c 100644 >> --- a/block/cow.c >> +++ b/block/cow.c >> @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs) >> { >> } >> >> -static int cow_create(const char *filename, QEMUOptionParameter *options) >> +static int cow_create(const char *filename, QemuOpts *opts) >> { >> struct cow_header_v2 cow_header; >> struct stat st; >> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) >> int ret; >> BlockDriverState *cow_bs; >> >> - /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - image_sectors = options->value.n / 512; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - image_filename = options->value.s; >> - } >> - options++; >> + /* Read out opts */ >> + if (opts) { > > I suspect you need the if (opts) here and in many other places only > because you special-cased "both lists empty" in append_opts_list(). I > suspect things become easier if you drop that. No, in this version, if(opt) is needed in "protocol", not needed in "format", I want to have the same code style, so I also judged if opts is NULL in "format" _create functions. Do you think is it acceptable? > >> + image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >> + image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> } >> >> - ret = bdrv_create_file(filename, options); >> + ret = bdrv_create_file(filename, opts); >> if (ret < 0) { >> return ret; >> } >> @@ -318,18 +314,22 @@ exit: >> return ret; >> } >> >> -static QEMUOptionParameter cow_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, >> - { NULL } >> +static QemuOptsList cow_create_opts = { >> + .name = "cow-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE, or else we lose support for suffixes, don't we? > In qemu-img.c, strtosz_suffix convert suffixes to regular digitals. Okay. I will change to QEMU_OPT_SIZE. >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_cow = { >> @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = { >> .bdrv_write = cow_co_write, >> .bdrv_co_is_allocated = cow_co_is_allocated, >> >> - .create_options = cow_create_options, >> + .bdrv_create_options = &cow_create_opts, >> }; >> >> static void bdrv_cow_init(void) >> diff --git a/block/gluster.c b/block/gluster.c >> index 0f2c32a..a41c684 100644 >> --- a/block/gluster.c >> +++ b/block/gluster.c >> @@ -335,8 +335,7 @@ out: >> return ret; >> } >> >> -static int qemu_gluster_create(const char *filename, >> - QEMUOptionParameter *options) >> +static int qemu_gluster_create(const char *filename, QemuOpts* opts) > > Space before the *, not after: QemuOpts *opts > Okay. > Running out of steam for today, rest of patch only skimmed. > >> { >> struct glfs *glfs; >> struct glfs_fd *fd; >> @@ -350,11 +349,9 @@ static int qemu_gluster_create(const char *filename, >> goto out; >> } >> >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - total_size = options->value.n / BDRV_SECTOR_SIZE; >> - } >> - options++; >> + if (opts) { >> + total_size = >> + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; >> } >> >> fd = glfs_creat(glfs, gconf->image, >> @@ -544,13 +541,17 @@ static void qemu_gluster_close(BlockDriverState *bs) >> glfs_fini(s->glfs); >> } >> >> -static QEMUOptionParameter qemu_gluster_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { NULL } >> +static QemuOptsList qemu_gluster_create_opts = { >> + .name = "qemu-gluster-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_gluster = { >> @@ -565,7 +566,7 @@ static BlockDriver bdrv_gluster = { >> .bdrv_aio_readv = qemu_gluster_aio_readv, >> .bdrv_aio_writev = qemu_gluster_aio_writev, >> .bdrv_aio_flush = qemu_gluster_aio_flush, >> - .create_options = qemu_gluster_create_options, >> + .bdrv_create_options = &qemu_gluster_create_opts, >> }; >> >> static BlockDriver bdrv_gluster_tcp = { >> @@ -580,7 +581,7 @@ static BlockDriver bdrv_gluster_tcp = { >> .bdrv_aio_readv = qemu_gluster_aio_readv, >> .bdrv_aio_writev = qemu_gluster_aio_writev, >> .bdrv_aio_flush = qemu_gluster_aio_flush, >> - .create_options = qemu_gluster_create_options, >> + .bdrv_create_options = &qemu_gluster_create_opts, >> }; >> >> static BlockDriver bdrv_gluster_unix = { >> @@ -595,7 +596,7 @@ static BlockDriver bdrv_gluster_unix = { >> .bdrv_aio_readv = qemu_gluster_aio_readv, >> .bdrv_aio_writev = qemu_gluster_aio_writev, >> .bdrv_aio_flush = qemu_gluster_aio_flush, >> - .create_options = qemu_gluster_create_options, >> + .bdrv_create_options = &qemu_gluster_create_opts, >> }; >> >> static BlockDriver bdrv_gluster_rdma = { >> @@ -610,7 +611,7 @@ static BlockDriver bdrv_gluster_rdma = { >> .bdrv_aio_readv = qemu_gluster_aio_readv, >> .bdrv_aio_writev = qemu_gluster_aio_writev, >> .bdrv_aio_flush = qemu_gluster_aio_flush, >> - .create_options = qemu_gluster_create_options, >> + .bdrv_create_options = &qemu_gluster_create_opts, >> }; >> >> static void bdrv_gluster_init(void) >> diff --git a/block/qcow.c b/block/qcow.c >> index 4276610..4691a3e 100644 >> --- a/block/qcow.c >> +++ b/block/qcow.c >> @@ -651,7 +651,7 @@ static void qcow_close(BlockDriverState *bs) >> error_free(s->migration_blocker); >> } >> >> -static int qcow_create(const char *filename, QEMUOptionParameter *options) >> +static int qcow_create(const char *filename, QemuOpts *opts) >> { >> int header_size, backing_filename_len, l1_size, shift, i; >> QCowHeader header; >> @@ -662,19 +662,16 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) >> int ret; >> BlockDriverState *qcow_bs; >> >> - /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - total_size = options->value.n / 512; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - backing_file = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { >> - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; >> + /* Read out opts */ >> + if (opts) { >> + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >> + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { >> + flags |= BLOCK_FLAG_ENCRYPT; >> } >> - options++; >> } >> >> - ret = bdrv_create_file(filename, options); >> + ret = bdrv_create_file(filename, opts); >> if (ret < 0) { >> return ret; >> } >> @@ -851,24 +848,27 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) >> return 0; >> } >> >> - >> -static QEMUOptionParameter qcow_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, >> - { >> - .name = BLOCK_OPT_ENCRYPT, >> - .type = OPT_FLAG, >> - .help = "Encrypt the image" >> - }, >> - { NULL } >> +static QemuOptsList qcow_create_opts = { >> + .name = "qcow-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { >> + .name = BLOCK_OPT_ENCRYPT, >> + .type = QEMU_OPT_BOOL, >> + .help = "Encrypt the image" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_qcow = { >> @@ -889,7 +889,7 @@ static BlockDriver bdrv_qcow = { >> .bdrv_write_compressed = qcow_write_compressed, >> .bdrv_get_info = qcow_get_info, >> >> - .create_options = qcow_create_options, >> + .bdrv_create_options = &qcow_create_opts, >> }; >> >> static void bdrv_qcow_init(void) >> diff --git a/block/qcow2.c b/block/qcow2.c >> index f6abff6..e3ee4af 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -1177,7 +1177,7 @@ static int preallocate(BlockDriverState *bs) >> static int qcow2_create2(const char *filename, int64_t total_size, >> const char *backing_file, const char *backing_format, >> int flags, size_t cluster_size, int prealloc, >> - QEMUOptionParameter *options, int version) >> + QemuOpts *opts, int version) >> { >> /* Calculate cluster_bits */ >> int cluster_bits; >> @@ -1208,7 +1208,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, >> uint8_t* refcount_table; >> int ret; >> >> - ret = bdrv_create_file(filename, options); >> + ret = bdrv_create_file(filename, opts); >> if (ret < 0) { >> return ret; >> } >> @@ -1311,7 +1311,7 @@ out: >> return ret; >> } >> >> -static int qcow2_create(const char *filename, QEMUOptionParameter *options) >> +static int qcow2_create(const char *filename, QemuOpts *opts) >> { >> const char *backing_file = NULL; >> const char *backing_fmt = NULL; >> @@ -1320,45 +1320,43 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) >> size_t cluster_size = DEFAULT_CLUSTER_SIZE; >> int prealloc = 0; >> int version = 2; >> + const char *buf; >> >> /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - sectors = options->value.n / 512; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - backing_file = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { >> - backing_fmt = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { >> - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; >> - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { >> - if (options->value.n) { >> - cluster_size = options->value.n; >> - } >> - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { >> - if (!options->value.s || !strcmp(options->value.s, "off")) { >> - prealloc = 0; >> - } else if (!strcmp(options->value.s, "metadata")) { >> - prealloc = 1; >> - } else { >> - fprintf(stderr, "Invalid preallocation mode: '%s'\n", >> - options->value.s); >> - return -EINVAL; >> - } >> - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) { >> - if (!options->value.s || !strcmp(options->value.s, "0.10")) { >> - version = 2; >> - } else if (!strcmp(options->value.s, "1.1")) { >> - version = 3; >> - } else { >> - fprintf(stderr, "Invalid compatibility level: '%s'\n", >> - options->value.s); >> - return -EINVAL; >> - } >> - } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) { >> - flags |= options->value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0; >> + if (opts) { >> + sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >> + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); >> + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { >> + flags |= BLOCK_FLAG_ENCRYPT; >> + } >> + cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, >> + DEFAULT_CLUSTER_SIZE); >> + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); >> + if (!buf || !strcmp(buf, "off")) { >> + prealloc = 0; >> + } else if (!strcmp(buf, "metadata")) { >> + prealloc = 1; >> + } else { >> + fprintf(stderr, "Invalid preallocation mode: '%s'\n", >> + buf); >> + return -EINVAL; >> + } >> + >> + buf = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL); >> + if (!buf || !strcmp(buf, "0.10")) { >> + version = 2; >> + } else if (!strcmp(buf, "1.1")) { >> + version = 3; >> + } else { >> + fprintf(stderr, "Invalid compatibility level: '%s'\n", >> + buf); >> + return -EINVAL; >> + } >> + >> + if (qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) { >> + flags |= BLOCK_FLAG_LAZY_REFCOUNTS; >> } >> - options++; >> } >> >> if (backing_file && prealloc) { >> @@ -1374,7 +1372,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) >> } >> >> return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, >> - cluster_size, prealloc, options, version); >> + cluster_size, prealloc, opts, version); >> } >> >> static int qcow2_make_empty(BlockDriverState *bs) >> @@ -1635,49 +1633,53 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, >> return ret; >> } >> >> -static QEMUOptionParameter qcow2_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_COMPAT_LEVEL, >> - .type = OPT_STRING, >> - .help = "Compatibility level (0.10 or 1.1)" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FMT, >> - .type = OPT_STRING, >> - .help = "Image format of the base image" >> - }, >> - { >> - .name = BLOCK_OPT_ENCRYPT, >> - .type = OPT_FLAG, >> - .help = "Encrypt the image" >> - }, >> - { >> - .name = BLOCK_OPT_CLUSTER_SIZE, >> - .type = OPT_SIZE, >> - .help = "qcow2 cluster size", >> - .value = { .n = DEFAULT_CLUSTER_SIZE }, >> - }, >> - { >> - .name = BLOCK_OPT_PREALLOC, >> - .type = OPT_STRING, >> - .help = "Preallocation mode (allowed values: off, metadata)" >> - }, >> - { >> - .name = BLOCK_OPT_LAZY_REFCOUNTS, >> - .type = OPT_FLAG, >> - .help = "Postpone refcount updates", >> - }, >> - { NULL } >> +static QemuOptsList qcow2_create_opts = { >> + .name = "qcow2-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_COMPAT_LEVEL, >> + .type = QEMU_OPT_STRING, >> + .help = "Compatibility level (0.10 or 1.1)" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FMT, >> + .type = QEMU_OPT_STRING, >> + .help = "Image format of the base image" >> + }, >> + { >> + .name = BLOCK_OPT_ENCRYPT, >> + .type = QEMU_OPT_BOOL, >> + .help = "Encrypt the image" >> + }, >> + { >> + .name = BLOCK_OPT_CLUSTER_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "qcow2 cluster size", >> + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) >> + }, >> + { >> + .name = BLOCK_OPT_PREALLOC, >> + .type = QEMU_OPT_STRING, >> + .help = "Preallocation mode (allowed values: off, metadata)" >> + }, >> + { >> + .name = BLOCK_OPT_LAZY_REFCOUNTS, >> + .type = QEMU_OPT_BOOL, >> + .help = "Postpone refcount updates", >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_qcow2 = { >> @@ -1715,8 +1717,9 @@ static BlockDriver bdrv_qcow2 = { >> >> .bdrv_invalidate_cache = qcow2_invalidate_cache, >> >> - .create_options = qcow2_create_options, >> .bdrv_check = qcow2_check, >> + >> + .bdrv_create_options = &qcow2_create_opts, >> }; >> >> static void bdrv_qcow2_init(void) >> diff --git a/block/qed.c b/block/qed.c >> index cf85d8f..766ba6f 100644 >> --- a/block/qed.c >> +++ b/block/qed.c >> @@ -603,7 +603,7 @@ out: >> return ret; >> } >> >> -static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) >> +static int bdrv_qed_create(const char *filename, QemuOpts *opts) >> { >> uint64_t image_size = 0; >> uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE; >> @@ -611,23 +611,15 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) >> const char *backing_file = NULL; >> const char *backing_fmt = NULL; >> >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - image_size = options->value.n; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - backing_file = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { >> - backing_fmt = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { >> - if (options->value.n) { >> - cluster_size = options->value.n; >> - } >> - } else if (!strcmp(options->name, BLOCK_OPT_TABLE_SIZE)) { >> - if (options->value.n) { >> - table_size = options->value.n; >> - } >> - } >> - options++; >> + if (opts) { >> + image_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); >> + cluster_size = qemu_opt_get_size(opts, >> + BLOCK_OPT_CLUSTER_SIZE, >> + QED_DEFAULT_CLUSTER_SIZE); >> + table_size = qemu_opt_get_size(opts, BLOCK_OPT_TABLE_SIZE, >> + QED_DEFAULT_TABLE_SIZE); >> } >> >> if (!qed_is_cluster_size_valid(cluster_size)) { >> @@ -1537,36 +1529,44 @@ static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result, >> return qed_check(s, result, !!fix); >> } >> >> -static QEMUOptionParameter qed_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size (in bytes)" >> - }, { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, { >> - .name = BLOCK_OPT_BACKING_FMT, >> - .type = OPT_STRING, >> - .help = "Image format of the base image" >> - }, { >> - .name = BLOCK_OPT_CLUSTER_SIZE, >> - .type = OPT_SIZE, >> - .help = "Cluster size (in bytes)", >> - .value = { .n = QED_DEFAULT_CLUSTER_SIZE }, >> - }, { >> - .name = BLOCK_OPT_TABLE_SIZE, >> - .type = OPT_SIZE, >> - .help = "L1/L2 table size (in clusters)" >> - }, >> - { /* end of list */ } >> +static QemuOptsList qed_create_opts = { >> + .name = "qed-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(qed_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FMT, >> + .type = QEMU_OPT_STRING, >> + .help = "Image format of the base image" >> + }, >> + { >> + .name = BLOCK_OPT_CLUSTER_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "Cluster size (in bytes)", >> + .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE), >> + }, >> + { >> + .name = BLOCK_OPT_TABLE_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "L1/L2 table size (in clusters)" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_qed = { >> .format_name = "qed", >> .instance_size = sizeof(BDRVQEDState), >> - .create_options = qed_create_options, >> + .bdrv_create_options = &qed_create_opts, >> >> .bdrv_probe = bdrv_qed_probe, >> .bdrv_rebind = bdrv_qed_rebind, >> diff --git a/block/qed.h b/block/qed.h >> index 2b4dded..99a7726 100644 >> --- a/block/qed.h >> +++ b/block/qed.h >> @@ -43,6 +43,7 @@ >> * >> * All fields are little-endian on disk. >> */ >> +#define QED_DEFAULT_CLUSTER_SIZE 65536 >> >> enum { >> QED_MAGIC = 'Q' | 'E' << 8 | 'D' << 16 | '\0' << 24, >> @@ -69,7 +70,6 @@ enum { >> */ >> QED_MIN_CLUSTER_SIZE = 4 * 1024, /* in bytes */ >> QED_MAX_CLUSTER_SIZE = 64 * 1024 * 1024, >> - QED_DEFAULT_CLUSTER_SIZE = 64 * 1024, >> >> /* Allocated clusters are tracked using a 2-level pagetable. Table size is >> * a multiple of clusters so large maximum image sizes can be supported >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 657af95..890c8a7 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -123,6 +123,19 @@ >> >> #define MAX_BLOCKSIZE 4096 >> >> +static QemuOptsList file_proto_create_opts = { >> + .name = "file-proto-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(file_proto_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "Virtual disk size" >> + }, >> + { /* end of list */ } >> + } >> +}; >> + >> typedef struct BDRVRawState { >> int fd; >> int type; >> @@ -997,18 +1010,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) >> return (int64_t)st.st_blocks * 512; >> } >> >> -static int raw_create(const char *filename, QEMUOptionParameter *options) >> +static int raw_create(const char *filename, QemuOpts *opts) >> { >> int fd; >> int result = 0; >> int64_t total_size = 0; >> >> - /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - total_size = options->value.n / BDRV_SECTOR_SIZE; >> - } >> - options++; >> + if (opts) { >> + total_size = >> + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; >> } >> >> fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, >> @@ -1136,15 +1146,6 @@ static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs, >> cb, opaque, QEMU_AIO_DISCARD); >> } >> >> -static QEMUOptionParameter raw_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { NULL } >> -}; >> - >> static BlockDriver bdrv_file = { >> .format_name = "file", >> .protocol_name = "file", >> @@ -1167,8 +1168,7 @@ static BlockDriver bdrv_file = { >> .bdrv_getlength = raw_getlength, >> .bdrv_get_allocated_file_size >> = raw_get_allocated_file_size, >> - >> - .create_options = raw_create_options, >> + .bdrv_create_options = &file_proto_create_opts, >> }; >> >> /***********************************************/ >> @@ -1403,19 +1403,16 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, >> cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); >> } >> >> -static int hdev_create(const char *filename, QEMUOptionParameter *options) >> +static int hdev_create(const char *filename, QemuOpts *opts) >> { >> int fd; >> int ret = 0; >> struct stat stat_buf; >> int64_t total_size = 0; >> >> - /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, "size")) { >> - total_size = options->value.n / BDRV_SECTOR_SIZE; >> - } >> - options++; >> + if (opts) { >> + total_size = >> + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; >> } >> >> fd = qemu_open(filename, O_WRONLY | O_BINARY); >> @@ -1440,7 +1437,7 @@ static int hdev_has_zero_init(BlockDriverState *bs) >> >> static BlockDriver bdrv_host_device = { >> .format_name = "host_device", >> - .protocol_name = "host_device", >> + .protocol_name = "host_device", >> .instance_size = sizeof(BDRVRawState), >> .bdrv_probe_device = hdev_probe_device, >> .bdrv_file_open = hdev_open, >> @@ -1449,7 +1446,6 @@ static BlockDriver bdrv_host_device = { >> .bdrv_reopen_commit = raw_reopen_commit, >> .bdrv_reopen_abort = raw_reopen_abort, >> .bdrv_create = hdev_create, >> - .create_options = raw_create_options, >> .bdrv_has_zero_init = hdev_has_zero_init, >> >> .bdrv_aio_readv = raw_aio_readv, >> @@ -1458,9 +1454,10 @@ static BlockDriver bdrv_host_device = { >> .bdrv_aio_discard = hdev_aio_discard, >> >> .bdrv_truncate = raw_truncate, >> - .bdrv_getlength = raw_getlength, >> + .bdrv_getlength = raw_getlength, >> .bdrv_get_allocated_file_size >> = raw_get_allocated_file_size, >> + .bdrv_create_options = &file_proto_create_opts, >> >> /* generic scsi device */ >> #ifdef __linux__ >> @@ -1574,7 +1571,6 @@ static BlockDriver bdrv_host_floppy = { >> .bdrv_reopen_commit = raw_reopen_commit, >> .bdrv_reopen_abort = raw_reopen_abort, >> .bdrv_create = hdev_create, >> - .create_options = raw_create_options, >> .bdrv_has_zero_init = hdev_has_zero_init, >> >> .bdrv_aio_readv = raw_aio_readv, >> @@ -1590,6 +1586,7 @@ static BlockDriver bdrv_host_floppy = { >> .bdrv_is_inserted = floppy_is_inserted, >> .bdrv_media_changed = floppy_media_changed, >> .bdrv_eject = floppy_eject, >> + .bdrv_create_options = &file_proto_create_opts, >> }; >> >> static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) >> @@ -1676,7 +1673,6 @@ static BlockDriver bdrv_host_cdrom = { >> .bdrv_reopen_commit = raw_reopen_commit, >> .bdrv_reopen_abort = raw_reopen_abort, >> .bdrv_create = hdev_create, >> - .create_options = raw_create_options, >> .bdrv_has_zero_init = hdev_has_zero_init, >> >> .bdrv_aio_readv = raw_aio_readv, >> @@ -1696,6 +1692,8 @@ static BlockDriver bdrv_host_cdrom = { >> /* generic scsi device */ >> .bdrv_ioctl = hdev_ioctl, >> .bdrv_aio_ioctl = hdev_aio_ioctl, >> + >> + .bdrv_create_options = &file_proto_create_opts, >> }; >> #endif /* __linux__ */ >> >> @@ -1798,7 +1796,6 @@ static BlockDriver bdrv_host_cdrom = { >> .bdrv_reopen_commit = raw_reopen_commit, >> .bdrv_reopen_abort = raw_reopen_abort, >> .bdrv_create = hdev_create, >> - .create_options = raw_create_options, >> .bdrv_has_zero_init = hdev_has_zero_init, >> >> .bdrv_aio_readv = raw_aio_readv, >> diff --git a/block/raw-win32.c b/block/raw-win32.c >> index b89ac19..34826e0 100644 >> --- a/block/raw-win32.c >> +++ b/block/raw-win32.c >> @@ -382,17 +382,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) >> return st.st_size; >> } >> >> -static int raw_create(const char *filename, QEMUOptionParameter *options) >> +static int raw_create(const char *filename, QemuOpts *opts) >> { >> int fd; >> int64_t total_size = 0; >> >> /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - total_size = options->value.n / 512; >> - } >> - options++; >> + if (opts) { >> + total_size = >> + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >> } >> >> fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, >> @@ -405,13 +403,17 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) >> return 0; >> } >> >> -static QEMUOptionParameter raw_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { NULL } >> +static QemuOptsList raw_create_opts = { >> + .name = "raw-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_file = { >> @@ -431,7 +433,7 @@ static BlockDriver bdrv_file = { >> .bdrv_get_allocated_file_size >> = raw_get_allocated_file_size, >> >> - .create_options = raw_create_options, >> + .bdrv_create_options = &raw_create_opts, >> }; >> >> /***********************************************/ >> diff --git a/block/raw.c b/block/raw.c >> index 75812db..033ffa6 100644 >> --- a/block/raw.c >> +++ b/block/raw.c >> @@ -95,18 +95,22 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, >> return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque); >> } >> >> -static int raw_create(const char *filename, QEMUOptionParameter *options) >> -{ >> - return bdrv_create_file(filename, options); >> -} >> - >> -static QEMUOptionParameter raw_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { NULL } >> +static int raw_create(const char *filename, QemuOpts *opts) >> +{ >> + return bdrv_create_file(filename, opts); >> +} >> + >> +static QemuOptsList raw_create_opts = { >> + .name = "raw-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static int raw_has_zero_init(BlockDriverState *bs) >> @@ -143,8 +147,8 @@ static BlockDriver bdrv_raw = { >> .bdrv_aio_ioctl = raw_aio_ioctl, >> >> .bdrv_create = raw_create, >> - .create_options = raw_create_options, >> .bdrv_has_zero_init = raw_has_zero_init, >> + .bdrv_create_options = &raw_create_opts, >> }; >> >> static void bdrv_raw_init(void) >> diff --git a/block/rbd.c b/block/rbd.c >> index 8cd10a7..a9f8772 100644 >> --- a/block/rbd.c >> +++ b/block/rbd.c >> @@ -287,7 +287,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf) >> return ret; >> } >> >> -static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) >> +static int qemu_rbd_create(const char *filename, QemuOpts *opts) >> { >> int64_t bytes = 0; >> int64_t objsize; >> @@ -310,24 +310,20 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) >> } >> >> /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - bytes = options->value.n; >> - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { >> - if (options->value.n) { >> - objsize = options->value.n; >> - if ((objsize - 1) & objsize) { /* not a power of 2? */ >> - error_report("obj size needs to be power of 2"); >> - return -EINVAL; >> - } >> - if (objsize < 4096) { >> - error_report("obj size too small"); >> - return -EINVAL; >> - } >> - obj_order = ffs(objsize) - 1; >> + if (opts) { >> + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> + objsize = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, 0); >> + if (objsize) { >> + if ((objsize - 1) & objsize) { /* not a power of 2? */ >> + error_report("obj size needs to be power of 2"); >> + return -EINVAL; >> } >> + if (objsize < 4096) { >> + error_report("obj size too small"); >> + return -EINVAL; >> + } >> + obj_order = ffs(objsize) - 1; >> } >> - options++; >> } >> >> clientname = qemu_rbd_parse_clientname(conf, clientname_buf); >> @@ -920,20 +916,24 @@ static BlockDriverAIOCB* qemu_rbd_aio_discard(BlockDriverState *bs, >> } >> #endif >> >> -static QEMUOptionParameter qemu_rbd_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_CLUSTER_SIZE, >> - .type = OPT_SIZE, >> - .help = "RBD object size" >> - }, >> - {NULL} >> +static QemuOptsList rbd_create_opts = { >> + .name = "rbd-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(rbd_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_CLUSTER_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "RBD object size", >> + .def_value_str = stringify(0), >> + }, >> + { /* end of list */ } >> + } >> }; >> - >> static BlockDriver bdrv_rbd = { >> .format_name = "rbd", >> .instance_size = sizeof(BDRVRBDState), >> @@ -941,7 +941,7 @@ static BlockDriver bdrv_rbd = { >> .bdrv_close = qemu_rbd_close, >> .bdrv_create = qemu_rbd_create, >> .bdrv_get_info = qemu_rbd_getinfo, >> - .create_options = qemu_rbd_create_options, >> + .bdrv_create_options = &rbd_create_opts, >> .bdrv_getlength = qemu_rbd_getlength, >> .bdrv_truncate = qemu_rbd_truncate, >> .protocol_name = "rbd", >> diff --git a/block/sheepdog.c b/block/sheepdog.c >> index 3e49bb8..d515e56 100644 >> --- a/block/sheepdog.c >> +++ b/block/sheepdog.c >> @@ -1274,12 +1274,12 @@ out: >> return ret; >> } >> >> -static int sd_create(const char *filename, QEMUOptionParameter *options) >> +static int sd_create(const char *filename, QemuOpts *opts) >> { >> int ret = 0; >> uint32_t vid = 0, base_vid = 0; >> int64_t vdi_size = 0; >> - char *backing_file = NULL; >> + const char *backing_file = NULL, *buf = NULL; >> BDRVSheepdogState *s; >> char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; >> uint32_t snapid; >> @@ -1298,26 +1298,20 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) >> goto out; >> } >> >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - vdi_size = options->value.n; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - backing_file = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { >> - if (!options->value.s || !strcmp(options->value.s, "off")) { >> - prealloc = false; >> - } else if (!strcmp(options->value.s, "full")) { >> - prealloc = true; >> - } else { >> - error_report("Invalid preallocation mode: '%s'", >> - options->value.s); >> - ret = -EINVAL; >> - goto out; >> - } >> + if (opts) { >> + vdi_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); >> + if (!buf || !strcmp(buf, "off")) { >> + prealloc = false; >> + } else if (!strcmp(buf, "full")) { >> + prealloc = true; >> + } else { >> + error_report("Invalid preallocation mode: '%s'", buf); >> + ret = -EINVAL; >> + goto out; >> } >> - options++; >> } >> - >> if (vdi_size > SD_MAX_VDI_SIZE) { >> error_report("too big image size"); >> ret = -EINVAL; >> @@ -2042,24 +2036,27 @@ static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data, >> return do_load_save_vmstate(s, data, pos, size, 1); >> } >> >> - >> -static QEMUOptionParameter sd_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, >> - { >> - .name = BLOCK_OPT_PREALLOC, >> - .type = OPT_STRING, >> - .help = "Preallocation mode (allowed values: off, full)" >> - }, >> - { NULL } >> +static QemuOptsList sd_create_opts = { >> + .name = "sheepdog-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { >> + .name = BLOCK_OPT_PREALLOC, >> + .type = QEMU_OPT_STRING, >> + .help = "Preallocation mode (allowed values: off, full)" >> + }, >> + { /* end of list */ } >> + } >> }; >> >> BlockDriver bdrv_sheepdog = { >> @@ -2084,7 +2081,7 @@ BlockDriver bdrv_sheepdog = { >> .bdrv_save_vmstate = sd_save_vmstate, >> .bdrv_load_vmstate = sd_load_vmstate, >> >> - .create_options = sd_create_options, >> + .bdrv_create_options = &sd_create_opts, >> }; >> >> static void bdrv_sheepdog_init(void) >> diff --git a/block/vdi.c b/block/vdi.c >> index 021abaa..95f738e 100644 >> --- a/block/vdi.c >> +++ b/block/vdi.c >> @@ -620,7 +620,7 @@ static int vdi_co_write(BlockDriverState *bs, >> return ret; >> } >> >> -static int vdi_create(const char *filename, QEMUOptionParameter *options) >> +static int vdi_create(const char *filename, QemuOpts *opts) >> { >> int fd; >> int result = 0; >> @@ -635,24 +635,19 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) >> logout("\n"); >> >> /* Read out options. */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - bytes = options->value.n; >> + if (opts) { >> + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> #if defined(CONFIG_VDI_BLOCK_SIZE) >> - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { >> - if (options->value.n) { >> - /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ >> - block_size = options->value.n; >> - } >> + /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ >> + block_size = qemu_opt_get_size(opts, >> + BLOCK_OPT_CLUSTER_SIZE, >> + DEFAULT_CLUSTER_SIZE); >> #endif >> #if defined(CONFIG_VDI_STATIC_IMAGE) >> - } else if (!strcmp(options->name, BLOCK_OPT_STATIC)) { >> - if (options->value.n) { >> - image_type = VDI_TYPE_STATIC; >> - } >> -#endif >> + if (qemu_opt_get_bool(opts, BLOCK_OPT_STATIC, 0)) { >> + image_type = VDI_TYPE_STATIC; >> } >> - options++; >> +#endif >> } >> >> fd = qemu_open(filename, >> @@ -733,29 +728,33 @@ static void vdi_close(BlockDriverState *bs) >> error_free(s->migration_blocker); >> } >> >> -static QEMUOptionParameter vdi_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> +static QemuOptsList vdi_create_opts = { >> + .name = "vdi-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> #if defined(CONFIG_VDI_BLOCK_SIZE) >> - { >> - .name = BLOCK_OPT_CLUSTER_SIZE, >> - .type = OPT_SIZE, >> - .help = "VDI cluster (block) size", >> - .value = { .n = DEFAULT_CLUSTER_SIZE }, >> - }, >> + { >> + .name = BLOCK_OPT_CLUSTER_SIZE, >> + .type = QEMU_OPT_SIZE, >> + .help = "VDI cluster (block) size", >> + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) >> + }, >> #endif >> #if defined(CONFIG_VDI_STATIC_IMAGE) >> - { >> - .name = BLOCK_OPT_STATIC, >> - .type = OPT_FLAG, >> - .help = "VDI static (pre-allocated) image" >> - }, >> + { >> + .name = BLOCK_OPT_STATIC, >> + .type = QEMU_OPT_BOOL, >> + .help = "VDI static (pre-allocated) image" >> + }, >> #endif >> - /* TODO: An additional option to set UUID values might be useful. */ >> - { NULL } >> + /* TODO: An additional option to set UUID values might be useful. */ >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_vdi = { >> @@ -776,7 +775,7 @@ static BlockDriver bdrv_vdi = { >> >> .bdrv_get_info = vdi_get_info, >> >> - .create_options = vdi_create_options, >> + .bdrv_create_options = &vdi_create_opts, >> .bdrv_check = vdi_check, >> }; >> >> diff --git a/block/vmdk.c b/block/vmdk.c >> index 19298c2..4c29927 100644 >> --- a/block/vmdk.c >> +++ b/block/vmdk.c >> @@ -1437,7 +1437,7 @@ static int relative_path(char *dest, int dest_size, >> return 0; >> } >> >> -static int vmdk_create(const char *filename, QEMUOptionParameter *options) >> +static int vmdk_create(const char *filename, QemuOpts *opts) >> { >> int fd, idx = 0; >> char desc[BUF_SIZE]; >> @@ -1476,18 +1476,14 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) >> if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { >> return -EINVAL; >> } >> - /* Read out options */ >> - while (options && options->name) { >> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >> - total_size = options->value.n; >> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >> - backing_file = options->value.s; >> - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) { >> - flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0; >> - } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) { >> - fmt = options->value.s; >> + /* Read out opts */ >> + if (opts) { >> + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >> + if (qemu_opt_get_bool(opts, BLOCK_OPT_COMPAT6, 0)) { >> + flags |= BLOCK_FLAG_COMPAT6; >> } >> - options++; >> + fmt = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); >> } >> if (!fmt) { >> /* Default format to monolithicSparse */ >> @@ -1654,30 +1650,34 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs) >> return ret; >> } >> >> -static QEMUOptionParameter vmdk_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_BACKING_FILE, >> - .type = OPT_STRING, >> - .help = "File name of a base image" >> - }, >> - { >> - .name = BLOCK_OPT_COMPAT6, >> - .type = OPT_FLAG, >> - .help = "VMDK version 6 image" >> - }, >> - { >> - .name = BLOCK_OPT_SUBFMT, >> - .type = OPT_STRING, >> - .help = >> - "VMDK flat extent format, can be one of " >> - "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " >> - }, >> - { NULL } >> +static QemuOptsList vmdk_create_opts = { >> + .name = "vmdk-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_BACKING_FILE, >> + .type = QEMU_OPT_STRING, >> + .help = "File name of a base image" >> + }, >> + { >> + .name = BLOCK_OPT_COMPAT6, >> + .type = QEMU_OPT_BOOL, >> + .help = "VMDK version 6 image" >> + }, >> + { >> + .name = BLOCK_OPT_SUBFMT, >> + .type = QEMU_OPT_STRING, >> + .help = >> + "VMDK flat extent format, can be one of " >> + "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_vmdk = { >> @@ -1694,7 +1694,7 @@ static BlockDriver bdrv_vmdk = { >> .bdrv_co_is_allocated = vmdk_co_is_allocated, >> .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, >> >> - .create_options = vmdk_create_options, >> + .bdrv_create_options = &vmdk_create_opts, >> }; >> >> static void bdrv_vmdk_init(void) >> diff --git a/block/vpc.c b/block/vpc.c >> index 7948609..fcfdc79 100644 >> --- a/block/vpc.c >> +++ b/block/vpc.c >> @@ -665,34 +665,33 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size) >> return ret; >> } >> >> -static int vpc_create(const char *filename, QEMUOptionParameter *options) >> +static int vpc_create(const char *filename, QemuOpts *opts) >> { >> uint8_t buf[1024]; >> struct vhd_footer *footer = (struct vhd_footer *) buf; >> - QEMUOptionParameter *disk_type_param; >> + const char *disk_type_param = NULL; >> int fd, i; >> uint16_t cyls = 0; >> uint8_t heads = 0; >> uint8_t secs_per_cyl = 0; >> int64_t total_sectors; >> - int64_t total_size; >> - int disk_type; >> + int64_t total_size = 0; >> + int disk_type = VHD_DYNAMIC; >> int ret = -EIO; >> >> - /* Read out options */ >> - total_size = get_option_parameter(options, BLOCK_OPT_SIZE)->value.n; >> - >> - disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT); >> - if (disk_type_param && disk_type_param->value.s) { >> - if (!strcmp(disk_type_param->value.s, "dynamic")) { >> - disk_type = VHD_DYNAMIC; >> - } else if (!strcmp(disk_type_param->value.s, "fixed")) { >> - disk_type = VHD_FIXED; >> - } else { >> - return -EINVAL; >> + /* Read out opts */ >> + if (opts) { >> + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); >> + disk_type_param = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); >> + if (disk_type_param) { >> + if (!strcmp(disk_type_param, "dynamic")) { >> + disk_type = VHD_DYNAMIC; >> + } else if (!strcmp(disk_type_param, "fixed")) { >> + disk_type = VHD_FIXED; >> + } else { >> + return -EINVAL; >> + } >> } >> - } else { >> - disk_type = VHD_DYNAMIC; >> } >> >> /* Create the file */ >> @@ -780,20 +779,24 @@ static void vpc_close(BlockDriverState *bs) >> error_free(s->migration_blocker); >> } >> >> -static QEMUOptionParameter vpc_create_options[] = { >> - { >> - .name = BLOCK_OPT_SIZE, >> - .type = OPT_SIZE, >> - .help = "Virtual disk size" >> - }, >> - { >> - .name = BLOCK_OPT_SUBFMT, >> - .type = OPT_STRING, >> - .help = >> - "Type of virtual hard disk format. Supported formats are " >> - "{dynamic (default) | fixed} " >> - }, >> - { NULL } >> +static QemuOptsList vpc_create_opts = { >> + .name = "vpc-create-opts", >> + .head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head), >> + .desc = { >> + { >> + .name = BLOCK_OPT_SIZE, >> + .type = QEMU_OPT_NUMBER, > > QEMU_OPT_SIZE again. > >> + .help = "Virtual disk size" >> + }, >> + { >> + .name = BLOCK_OPT_SUBFMT, >> + .type = OPT_STRING, >> + .help = >> + "Type of virtual hard disk format. Supported formats are " >> + "{dynamic (default) | fixed} " >> + }, >> + { /* end of list */ } >> + } >> }; >> >> static BlockDriver bdrv_vpc = { >> @@ -809,7 +812,7 @@ static BlockDriver bdrv_vpc = { >> .bdrv_read = vpc_co_read, >> .bdrv_write = vpc_co_write, >> >> - .create_options = vpc_create_options, >> + .bdrv_create_options = &vpc_create_opts, >> }; >> >> static void bdrv_vpc_init(void) >> diff --git a/block/vvfat.c b/block/vvfat.c >> index 06e6654..2343cd4 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -2802,7 +2802,7 @@ static BlockDriver vvfat_write_target = { >> static int enable_write_target(BDRVVVFATState *s) >> { >> BlockDriver *bdrv_qcow; >> - QEMUOptionParameter *options; >> + QemuOpts *opts; >> int ret; >> int size = sector2cluster(s, s->sector_count); >> s->used_clusters = calloc(size, 1); >> @@ -2818,12 +2818,13 @@ static int enable_write_target(BDRVVVFATState *s) >> } >> >> bdrv_qcow = bdrv_find_format("qcow"); >> - options = parse_option_parameters("", bdrv_qcow->create_options, NULL); >> - set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512); >> - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:"); >> + opts = qemu_opts_create_nofail(bdrv_qcow->bdrv_create_options); >> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); >> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); >> >> - if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0) >> + if (bdrv_create(bdrv_qcow, s->qcow_filename, opts) < 0) { >> return -1; >> + } >> >> s->qcow = bdrv_new(""); >> if (s->qcow == NULL) { >> diff --git a/include/block/block.h b/include/block/block.h >> index ffd1936..c99e8bf 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -124,8 +124,8 @@ BlockDriver *bdrv_find_protocol(const char *filename); >> BlockDriver *bdrv_find_format(const char *format_name); >> BlockDriver *bdrv_find_whitelisted_format(const char *format_name); >> int bdrv_create(BlockDriver *drv, const char* filename, >> - QEMUOptionParameter *options); >> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options); >> + QemuOpts *options); >> +int bdrv_create_file(const char *filename, QemuOpts *options); >> 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 f83ffb8..71f7670 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -88,7 +88,7 @@ struct BlockDriver { >> const uint8_t *buf, int nb_sectors); >> void (*bdrv_close)(BlockDriverState *bs); >> void (*bdrv_rebind)(BlockDriverState *bs); >> - int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); >> + int (*bdrv_create)(const char *filename, QemuOpts *options); >> int (*bdrv_set_key)(BlockDriverState *bs, const char *key); >> int (*bdrv_make_empty)(BlockDriverState *bs); >> /* aio */ >> @@ -177,9 +177,7 @@ struct BlockDriver { >> unsigned long int req, void *buf, >> BlockDriverCompletionFunc *cb, void *opaque); >> >> - /* List of options for creating images, terminated by name == NULL */ >> - QEMUOptionParameter *create_options; >> - >> + QemuOptsList *bdrv_create_options; > > Since you're renaming this anyway, consider renaming to > bdrv_create_opts, to match the names used elsewhere. > >> >> /* >> * Returns 0 for completed check, -errno for internal errors. >> diff --git a/qemu-img.c b/qemu-img.c >> index 85d3740..b4b3ce8 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -200,7 +200,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_options = NULL; >> >> /* Find driver and parse its options */ >> drv = bdrv_find_format(fmt); >> @@ -215,12 +215,10 @@ 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); >> - free_option_parameters(create_options); >> + create_options = append_opts_list(drv->bdrv_create_options, >> + proto_drv->bdrv_create_options); >> + print_opts_list(create_options); >> + free_opts_list(create_options); >> return 0; >> } >> >> @@ -271,19 +269,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 *list, >> 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(list, 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(list, BLOCK_OPT_BACKING_FMT, base_fmt)) { >> error_report("Backing file format not supported for file " >> "format '%s'", fmt); >> return -1; >> @@ -675,8 +673,9 @@ static int img_convert(int argc, char **argv) > static int img_convert(int argc, char **argv) > { > int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors; > int progress = 0, flags; > const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; > BlockDriver *drv, *proto_drv; > BlockDriverState **bs = NULL, *out_bs = NULL; > int64_t total_sectors, nb_sectors, sector_num, bs_offset; > > Some or all of these should probably be uint64_t. Not your patch's > fault, feel free to ignore. > > uint64_t bs_sectors; >> uint8_t * buf = NULL; >> const uint8_t *buf1; >> BlockDriverInfo bdi; >> - QEMUOptionParameter *param = NULL, *create_options = NULL; >> - QEMUOptionParameter *out_baseimg_param; >> + QemuOpts *param = NULL; >> + QemuOptsList *create_options = NULL; >> + const char *out_baseimg_param; >> char *options = NULL; >> const char *snapshot_name = NULL; >> float local_progress = 0; >> @@ -811,40 +810,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_options = append_opts_list(drv->bdrv_create_options, >> + proto_drv->bdrv_create_options); >> >> if (options) { >> - param = parse_option_parameters(options, create_options, param); >> - if (param == NULL) { >> + if (qemu_opts_do_parse(param, options, NULL) != 0) { >> error_report("Invalid options for file format '%s'.", out_fmt); >> ret = -1; >> goto out; >> } >> } else { >> - param = parse_option_parameters("", create_options, param); >> + param = qemu_opts_create_nofail(create_options); >> } >> - >> - set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512); >> + qemu_opt_set_number(param, BLOCK_OPT_SIZE, total_sectors * 512); >> ret = add_old_style_options(out_fmt, param, 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(param, 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(param, BLOCK_OPT_ENCRYPT, false); >> + const char *preallocation = >> + qemu_opt_get(param, BLOCK_OPT_PREALLOC); >> >> if (!drv->bdrv_write_compressed) { >> error_report("Compression not supported for this file format"); >> @@ -852,15 +847,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"); >> @@ -1078,8 +1073,10 @@ static int img_convert(int argc, char **argv) >> } >> out: >> qemu_progress_end(); >> - free_option_parameters(create_options); >> - free_option_parameters(param); >> + free_opts_list(create_options); >> + if (param) { >> + qemu_opts_del(param); >> + } >> qemu_vfree(buf); >> if (out_bs) { >> bdrv_delete(out_bs); > >
Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > Markus Armbruster <armbru@redhat.com> writes: >> Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: >> >>> This patch will use QemuOpts related functions in block layer, add >>> a member bdrv_create_options to BlockDriver struct, it will return >>> a QemuOptsList pointer, which includes the image format's create >>> options. >>> >>> And create options's primary consumer is block creating related functions, >>> so modify them together. >>> >>> >>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >>> --- >>> v10->v11) >>> 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or >>> qemu_opts_print produce un-expanded cluster_size. >>> 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, >>> or while using protocol, there will be an error. >>> >>> v8->v9) >>> 1) add qemu_ prefix to gluster_create_opts. >>> 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be >>> converted. >>> >>> v7->v8) >>> 1) rebase to upstream source tree. >>> 2) add gluster.c, raw-win32.c, and rbd.c. >>> >>> v6->v7: >>> 1) use osdep.h:stringify(), not redefining new macro. >>> 2) preserve TODO comment. >>> 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. >>> 4) initialize disk_type even when opts is NULL. >>> >>> v5->v6: >>> 1) judge if opts == NULL in block layer create functions. >>> 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. >>> 3) made more readable while using qemu_opt_get_number. >>> >>> >>> block.c | 91 ++++++++++++------------ >>> block/cow.c | 46 ++++++------- >>> block/gluster.c | 37 +++++----- >>> block/qcow.c | 60 ++++++++-------- >>> block/qcow2.c | 171 +++++++++++++++++++++++----------------------- >>> block/qed.c | 86 +++++++++++------------ >>> block/qed.h | 2 +- >>> block/raw-posix.c | 59 ++++++++-------- >>> block/raw-win32.c | 30 ++++---- >>> block/raw.c | 30 ++++---- >>> block/rbd.c | 62 ++++++++--------- >>> block/sheepdog.c | 75 ++++++++++---------- >>> block/vdi.c | 69 +++++++++---------- >>> block/vmdk.c | 74 ++++++++++---------- >>> block/vpc.c | 67 +++++++++--------- >>> block/vvfat.c | 11 +-- >>> include/block/block.h | 4 +- >>> include/block/block_int.h | 6 +- >>> qemu-img.c | 61 ++++++++--------- >>> 19 files changed, 519 insertions(+), 522 deletions(-) >>> >>> diff --git a/block.c b/block.c >>> index 6fa7c90..56e4613 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name) >>> typedef struct CreateCo { >>> BlockDriver *drv; >>> char *filename; >>> - QEMUOptionParameter *options; >>> + QemuOpts *opts; >>> int ret; >>> } CreateCo; >>> >>> @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) >>> CreateCo *cco = opaque; >>> assert(cco->drv); >>> >>> - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); >>> + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); >>> } >>> >>> int bdrv_create(BlockDriver *drv, const char* filename, >>> - QEMUOptionParameter *options) >>> + QemuOpts *opts) >>> { >>> int ret; >>> >>> @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, >>> CreateCo cco = { >>> .drv = drv, >>> .filename = g_strdup(filename), >>> - .options = options, >>> + .opts = opts, >>> .ret = NOT_DONE, >>> }; >>> >>> @@ -405,7 +405,7 @@ out: >>> return ret; >>> } >>> >>> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >>> +int bdrv_create_file(const char *filename, QemuOpts *opts) >>> { >>> BlockDriver *drv; >>> >>> @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >>> return -ENOENT; >>> } >>> >>> - return bdrv_create(drv, filename, options); >>> + return bdrv_create(drv, filename, opts); >>> } >>> >>> /* >>> @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >>> int64_t total_size; >>> int is_protocol = 0; >>> BlockDriver *bdrv_qcow2; >>> - QEMUOptionParameter *options; >>> + QemuOpts *opts; >>> char backing_filename[PATH_MAX]; >>> >>> /* if snapshot, we create a temporary backing file and open it >>> @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >>> return -errno; >>> >>> bdrv_qcow2 = bdrv_find_format("qcow2"); >>> - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); >>> + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options); >>> >>> - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); >>> - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); >>> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); >>> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); >>> if (drv) { >>> - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, >>> - drv->format_name); >>> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); >>> } >>> >>> - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); >>> - free_option_parameters(options); >>> + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); >>> + qemu_opts_del(opts); >>> if (ret < 0) { >>> return ret; >>> } >>> @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>> const char *base_filename, const char *base_fmt, >>> char *options, uint64_t img_size, int flags, Error **errp) >>> { >>> - QEMUOptionParameter *param = NULL, *create_options = NULL; >>> - QEMUOptionParameter *backing_fmt, *backing_file, *size; >>> + QemuOpts *opts = NULL; >>> + QemuOptsList *create_options = NULL; >>> + const char *backing_fmt, *backing_file; >>> + int64_t size; >>> BlockDriverState *bs = NULL; >>> BlockDriver *drv, *proto_drv; >>> BlockDriver *backing_drv = NULL; >>> @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const char *fmt, >>> error_setg(errp, "Unknown protocol '%s'", filename); >>> return; >>> } >>> - >>> - create_options = append_option_parameters(create_options, >>> - drv->create_options); >>> - create_options = append_option_parameters(create_options, >>> - proto_drv->create_options); >>> - >>> + create_options = append_opts_list(drv->bdrv_create_options, >>> + proto_drv->bdrv_create_options); >> >> Before: format's options get appended first, then protocol's options. >> Because get_option_parameter() searches in order, format options shadow >> protocol options. >> >> After: append_opts_list() gives first argument's options precedence. >> >> Thus, no change. Good. >> >> Should bdrv_create_options option name clashes be avoided? Outside the >> scope of this patch. > Sorry, I do not understand this line, clash? Can you explain some more? > Do you mean bdrv_create_options should be renamed such as bdrv_create_opts? No. I was talking about what happens when drv->create_options and proto->create_options both have an entry with the same name. First, I reasoned why your patch doesn't change the way such a clash is handled. Then I wondered whether we should avoid such clashes, but hastened to add that I'm not asking you to do that now. >>> /* Create parameter list with default values */ >>> - param = parse_option_parameters("", create_options, param); >>> + opts = qemu_opts_create_nofail(create_options); >>> >>> - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >>> + 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; >>> } >>> } >> >> Before: size from -o replaces img_size in param. >> >> After: size from -o gets appended to opts, is therefore shadowed by >> img_size. I think. >> >> User-visible change, if my reading is correct. Should be avoided. >> > Okay. will also "replace", not "append". Prepending could also work. Doing things in a different order could make appending work. Whatever is easiest. >>> >>> if (base_filename) { >>> - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, >>> + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, >>> base_filename)) { >>> error_setg(errp, "Backing file not supported for file format '%s'", >>> fmt); >> >> Before: base_filename replaces the backing_file from -o. >> >> After: base_filename gets appended to opts, shadowed by backing_file >> from -o. >> >>> @@ -4542,39 +4538,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; >>> } >>> } >> >> Likewise. >> >>> >>> - 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); >>> + error_setg(errp, "Unknown backing file format '%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_number(opts, BLOCK_OPT_SIZE, -1); >>> + if (size == -1) { >>> + if (backing_file) { >> >> Code has the same before your patch, so it's not your fault, but here >> goes anyway: >> >> QemuOpts support only *unsigned* numbers. Argument -1 is for an >> uint64_t parameter, and gets converted to UINT64_MAX. If the option >> isn't set, it's returned. The assignment to int64_t size converts it >> back to -1. >> >> Using the an honest UINT64_MAX instead of -1 would be clearer. Should >> make size uint64_t then. >> >> However, all other places do >> >> qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) >> >> Why is this one different? >> > Because previous use -1, so, I passed default value -1. I have no idea what you're talking about, and I'm not 100% sure you do :) The last argument to qemu_opt_get_number() is *not* used here. That's because a few lines above, you do qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); So, opts always has the BLOCK_OPT_SIZE option, and the default value never applies. Let's take a step back and figure out how the current code works before we further review your patch. bdrv_img_create() merges parameters img_size, base_filename, base_fmt with options as follows: 1. Get format and protocol driver from parameters @fmt and @filename. 2. Combine the two drivers' option lists. 3. Copy the resulting option list for no discernible reason. 4. If %BLOCK_OPT_SIZE is a valid option, set its value to parameter @img_size. 5. For all KEY=VAL in parameter @options: KEY must be a valid option (error out if not) set its value to VAL 6. If parameter @base_filename isn't null: %BLOCK_OPT_BACKING_FILE must be a valid option (error out if not) set its value to @base_filename 7. If parameter @base_fmt isn't null: %BLOCK_OPT_BACKING_FMT must be a valid option (error out if not) set its value to @base_fmt In short, @base_filename and @base_fmt take precedence over @options (unless they're null, of course). @options takes precedence over @img_size. What a mess :) Not your fault. Now let's look at the code retrieving BLOCK_OPT_SIZE: // 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) { // compute size, put it into buf set_option_parameter(param, BLOCK_OPT_SIZE, buf); } else { // error out } If %BLOCK_OPT_SIZE is a valid option, and has the special value (uint64_t)-1, then we must have a backing file (error out if not), and we use its size instead. In other words, the special value means "use the backing file's size". The value can come either from @img_size or from @options. Some callers indeed pass (uint64_t)-1 for @img_size. For instance, qemu-img create passes it when run without the optional size argument. Works. qemu-img is the only caller passing @options. With -o size=-1, it actually puts (uint64_t)-1 into @param (parse_option_size() sucks). Weird. The whole bloody mess should be cleaned up. But I'm not asking you to do that now. I do think, however, that you should stick to qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) instead of your voodoo qemu_opt_get_number(opts, BLOCK_OPT_SIZE, -1) >>> uint64_t size; >>> - char buf[32]; >>> int back_flags; >>> >>> /* backing files always opened read-only */ >>> @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const char *fmt, >>> >>> bs = bdrv_new(""); >>> >>> - ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); >>> + ret = bdrv_open(bs, backing_file, back_flags, backing_drv); >>> if (ret < 0) { >>> error_setg_errno(errp, -ret, "Could not open '%s'", >>> - backing_file->value.s); >>> + backing_file); >>> goto out; >>> } >>> bdrv_get_geometry(bs, &size); >>> 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); >>> } else { >>> error_setg(errp, "Image creation needs a size parameter"); >>> goto out; >>> @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>> } >>> >>> printf("Formatting '%s', fmt=%s ", filename, fmt); >>> - print_option_parameters(param); >>> + qemu_opts_print(opts, NULL); >>> puts(""); >>> >>> - ret = bdrv_create(drv, filename, param); >>> + ret = bdrv_create(drv, filename, opts); >>> if (ret < 0) { >>> if (ret == -ENOTSUP) { >>> error_setg(errp,"Formatting or formatting option not supported for " >>> @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>> } >>> >>> out: >>> - free_option_parameters(create_options); >>> - free_option_parameters(param); >>> + free_opts_list(create_options); >>> + if (opts) { >>> + qemu_opts_del(opts); >>> + } >>> >>> if (bs) { >>> bdrv_delete(bs); >>> diff --git a/block/cow.c b/block/cow.c >>> index a33ce95..5442c9c 100644 >>> --- a/block/cow.c >>> +++ b/block/cow.c >>> @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs) >>> { >>> } >>> >>> -static int cow_create(const char *filename, QEMUOptionParameter *options) >>> +static int cow_create(const char *filename, QemuOpts *opts) >>> { >>> struct cow_header_v2 cow_header; >>> struct stat st; >>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) >>> int ret; >>> BlockDriverState *cow_bs; >>> >>> - /* Read out options */ >>> - while (options && options->name) { >>> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >>> - image_sectors = options->value.n / 512; >>> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >>> - image_filename = options->value.s; >>> - } >>> - options++; >>> + /* Read out opts */ >>> + if (opts) { >> >> I suspect you need the if (opts) here and in many other places only >> because you special-cased "both lists empty" in append_opts_list(). I >> suspect things become easier if you drop that. > > No, in this version, if(opt) is needed in "protocol", not needed in > "format", I want to have the same code style, so I also judged if opts > is NULL in "format" _create functions. Do you think is it acceptable? I still don't get it. Can you explain how opts can be null here even when append_opts_list() is changed not to return null? >> >>> + image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >>> + image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >>> } >>> >>> - ret = bdrv_create_file(filename, options); >>> + ret = bdrv_create_file(filename, opts); >>> if (ret < 0) { >>> return ret; >>> } >>> @@ -318,18 +314,22 @@ exit: >>> return ret; >>> } >>> >>> -static QEMUOptionParameter cow_create_options[] = { >>> - { >>> - .name = BLOCK_OPT_SIZE, >>> - .type = OPT_SIZE, >>> - .help = "Virtual disk size" >>> - }, >>> - { >>> - .name = BLOCK_OPT_BACKING_FILE, >>> - .type = OPT_STRING, >>> - .help = "File name of a base image" >>> - }, >>> - { NULL } >>> +static QemuOptsList cow_create_opts = { >>> + .name = "cow-create-opts", >>> + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), >>> + .desc = { >>> + { >>> + .name = BLOCK_OPT_SIZE, >>> + .type = QEMU_OPT_NUMBER, >> >> QEMU_OPT_SIZE, or else we lose support for suffixes, don't we? >> > In qemu-img.c, strtosz_suffix convert suffixes to regular digitals. That's correct for the optional size argument, but not for -o size=... > Okay. I will change to QEMU_OPT_SIZE. Please do, and test that both the size argument and -o size=... works just like before, with and without suffixes. >>> + .help = "Virtual disk size" >>> + }, >>> + { >>> + .name = BLOCK_OPT_BACKING_FILE, >>> + .type = QEMU_OPT_STRING, >>> + .help = "File name of a base image" >>> + }, >>> + { /* end of list */ } >>> + } >>> }; >>> >>> static BlockDriver bdrv_cow = { >>> @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = { >>> .bdrv_write = cow_co_write, >>> .bdrv_co_is_allocated = cow_co_is_allocated, >>> >>> - .create_options = cow_create_options, >>> + .bdrv_create_options = &cow_create_opts, >>> }; >>> >>> static void bdrv_cow_init(void) >>> diff --git a/block/gluster.c b/block/gluster.c >>> index 0f2c32a..a41c684 100644 >>> --- a/block/gluster.c >>> +++ b/block/gluster.c >>> @@ -335,8 +335,7 @@ out: >>> return ret; >>> } >>> >>> -static int qemu_gluster_create(const char *filename, >>> - QEMUOptionParameter *options) >>> +static int qemu_gluster_create(const char *filename, QemuOpts* opts) >> >> Space before the *, not after: QemuOpts *opts >> > Okay. [...]
于 2013-1-29 1:41, Markus Armbruster 写道: > Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > >> Markus Armbruster <armbru@redhat.com> writes: >>> Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: >>> >>>> This patch will use QemuOpts related functions in block layer, add >>>> a member bdrv_create_options to BlockDriver struct, it will return >>>> a QemuOptsList pointer, which includes the image format's create >>>> options. >>>> >>>> And create options's primary consumer is block creating related functions, >>>> so modify them together. >>>> >>>> >>>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >>>> --- >>>> v10->v11) >>>> 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or >>>> qemu_opts_print produce un-expanded cluster_size. >>>> 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, >>>> or while using protocol, there will be an error. >>>> >>>> v8->v9) >>>> 1) add qemu_ prefix to gluster_create_opts. >>>> 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be >>>> converted. >>>> >>>> v7->v8) >>>> 1) rebase to upstream source tree. >>>> 2) add gluster.c, raw-win32.c, and rbd.c. >>>> >>>> v6->v7: >>>> 1) use osdep.h:stringify(), not redefining new macro. >>>> 2) preserve TODO comment. >>>> 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. >>>> 4) initialize disk_type even when opts is NULL. >>>> >>>> v5->v6: >>>> 1) judge if opts == NULL in block layer create functions. >>>> 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. >>>> 3) made more readable while using qemu_opt_get_number. >>>> >>>> >>>> block.c | 91 ++++++++++++------------ >>>> block/cow.c | 46 ++++++------- >>>> block/gluster.c | 37 +++++----- >>>> block/qcow.c | 60 ++++++++-------- >>>> block/qcow2.c | 171 +++++++++++++++++++++++----------------------- >>>> block/qed.c | 86 +++++++++++------------ >>>> block/qed.h | 2 +- >>>> block/raw-posix.c | 59 ++++++++-------- >>>> block/raw-win32.c | 30 ++++---- >>>> block/raw.c | 30 ++++---- >>>> block/rbd.c | 62 ++++++++--------- >>>> block/sheepdog.c | 75 ++++++++++---------- >>>> block/vdi.c | 69 +++++++++---------- >>>> block/vmdk.c | 74 ++++++++++---------- >>>> block/vpc.c | 67 +++++++++--------- >>>> block/vvfat.c | 11 +-- >>>> include/block/block.h | 4 +- >>>> include/block/block_int.h | 6 +- >>>> qemu-img.c | 61 ++++++++--------- >>>> 19 files changed, 519 insertions(+), 522 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index 6fa7c90..56e4613 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name) >>>> typedef struct CreateCo { >>>> BlockDriver *drv; >>>> char *filename; >>>> - QEMUOptionParameter *options; >>>> + QemuOpts *opts; >>>> int ret; >>>> } CreateCo; >>>> >>>> @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) >>>> CreateCo *cco = opaque; >>>> assert(cco->drv); >>>> >>>> - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); >>>> + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); >>>> } >>>> >>>> int bdrv_create(BlockDriver *drv, const char* filename, >>>> - QEMUOptionParameter *options) >>>> + QemuOpts *opts) >>>> { >>>> int ret; >>>> >>>> @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, >>>> CreateCo cco = { >>>> .drv = drv, >>>> .filename = g_strdup(filename), >>>> - .options = options, >>>> + .opts = opts, >>>> .ret = NOT_DONE, >>>> }; >>>> >>>> @@ -405,7 +405,7 @@ out: >>>> return ret; >>>> } >>>> >>>> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >>>> +int bdrv_create_file(const char *filename, QemuOpts *opts) >>>> { >>>> BlockDriver *drv; >>>> >>>> @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) >>>> return -ENOENT; >>>> } >>>> >>>> - return bdrv_create(drv, filename, options); >>>> + return bdrv_create(drv, filename, opts); >>>> } >>>> >>>> /* >>>> @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >>>> int64_t total_size; >>>> int is_protocol = 0; >>>> BlockDriver *bdrv_qcow2; >>>> - QEMUOptionParameter *options; >>>> + QemuOpts *opts; >>>> char backing_filename[PATH_MAX]; >>>> >>>> /* if snapshot, we create a temporary backing file and open it >>>> @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, >>>> return -errno; >>>> >>>> bdrv_qcow2 = bdrv_find_format("qcow2"); >>>> - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); >>>> + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options); >>>> >>>> - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); >>>> - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); >>>> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); >>>> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); >>>> if (drv) { >>>> - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, >>>> - drv->format_name); >>>> + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); >>>> } >>>> >>>> - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); >>>> - free_option_parameters(options); >>>> + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); >>>> + qemu_opts_del(opts); >>>> if (ret < 0) { >>>> return ret; >>>> } >>>> @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>>> const char *base_filename, const char *base_fmt, >>>> char *options, uint64_t img_size, int flags, Error **errp) >>>> { >>>> - QEMUOptionParameter *param = NULL, *create_options = NULL; >>>> - QEMUOptionParameter *backing_fmt, *backing_file, *size; >>>> + QemuOpts *opts = NULL; >>>> + QemuOptsList *create_options = NULL; >>>> + const char *backing_fmt, *backing_file; >>>> + int64_t size; >>>> BlockDriverState *bs = NULL; >>>> BlockDriver *drv, *proto_drv; >>>> BlockDriver *backing_drv = NULL; >>>> @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const char *fmt, >>>> error_setg(errp, "Unknown protocol '%s'", filename); >>>> return; >>>> } >>>> - >>>> - create_options = append_option_parameters(create_options, >>>> - drv->create_options); >>>> - create_options = append_option_parameters(create_options, >>>> - proto_drv->create_options); >>>> - >>>> + create_options = append_opts_list(drv->bdrv_create_options, >>>> + proto_drv->bdrv_create_options); >>> >>> Before: format's options get appended first, then protocol's options. >>> Because get_option_parameter() searches in order, format options shadow >>> protocol options. >>> >>> After: append_opts_list() gives first argument's options precedence. >>> >>> Thus, no change. Good. >>> >>> Should bdrv_create_options option name clashes be avoided? Outside the >>> scope of this patch. >> Sorry, I do not understand this line, clash? Can you explain some more? >> Do you mean bdrv_create_options should be renamed such as bdrv_create_opts? > > No. I was talking about what happens when drv->create_options and > proto->create_options both have an entry with the same name. > > First, I reasoned why your patch doesn't change the way such a clash is > handled. > > Then I wondered whether we should avoid such clashes, but hastened to > add that I'm not asking you to do that now. > >>>> /* Create parameter list with default values */ >>>> - param = parse_option_parameters("", create_options, param); >>>> + opts = qemu_opts_create_nofail(create_options); >>>> >>>> - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >>>> + 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; >>>> } >>>> } >>> >>> Before: size from -o replaces img_size in param. >>> >>> After: size from -o gets appended to opts, is therefore shadowed by >>> img_size. I think. >>> >>> User-visible change, if my reading is correct. Should be avoided. >>> >> Okay. will also "replace", not "append". > > Prepending could also work. > > Doing things in a different order could make appending work. > > Whatever is easiest. > >>>> >>>> if (base_filename) { >>>> - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, >>>> + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, >>>> base_filename)) { >>>> error_setg(errp, "Backing file not supported for file format '%s'", >>>> fmt); >>> >>> Before: base_filename replaces the backing_file from -o. >>> >>> After: base_filename gets appended to opts, shadowed by backing_file >>> from -o. >>> >>>> @@ -4542,39 +4538,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; >>>> } >>>> } >>> >>> Likewise. >>> >>>> >>>> - 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); >>>> + error_setg(errp, "Unknown backing file format '%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_number(opts, BLOCK_OPT_SIZE, -1); >>>> + if (size == -1) { >>>> + if (backing_file) { >>> >>> Code has the same before your patch, so it's not your fault, but here >>> goes anyway: >>> >>> QemuOpts support only *unsigned* numbers. Argument -1 is for an >>> uint64_t parameter, and gets converted to UINT64_MAX. If the option >>> isn't set, it's returned. The assignment to int64_t size converts it >>> back to -1. >>> >>> Using the an honest UINT64_MAX instead of -1 would be clearer. Should >>> make size uint64_t then. >>> >>> However, all other places do >>> >>> qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) >>> >>> Why is this one different? >>> >> Because previous use -1, so, I passed default value -1. > > I have no idea what you're talking about, and I'm not 100% sure you do > :) > > The last argument to qemu_opt_get_number() is *not* used here. That's > because a few lines above, you do > > qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); > > So, opts always has the BLOCK_OPT_SIZE option, and the default value > never applies. > > Let's take a step back and figure out how the current code works before > we further review your patch. > > bdrv_img_create() merges parameters img_size, base_filename, base_fmt > with options as follows: > > 1. Get format and protocol driver from parameters @fmt and @filename. > > 2. Combine the two drivers' option lists. > > 3. Copy the resulting option list for no discernible reason. > > 4. If %BLOCK_OPT_SIZE is a valid option, set its value to parameter > @img_size. > > 5. For all KEY=VAL in parameter @options: > KEY must be a valid option (error out if not) > set its value to VAL > > 6. If parameter @base_filename isn't null: > %BLOCK_OPT_BACKING_FILE must be a valid option (error out if not) > set its value to @base_filename > > 7. If parameter @base_fmt isn't null: > %BLOCK_OPT_BACKING_FMT must be a valid option (error out if not) > set its value to @base_fmt > > In short, @base_filename and @base_fmt take precedence over @options > (unless they're null, of course). @options takes precedence over > @img_size. What a mess :) Not your fault. > > Now let's look at the code retrieving BLOCK_OPT_SIZE: > > // 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) { > // compute size, put it into buf > set_option_parameter(param, BLOCK_OPT_SIZE, buf); > } else { > // error out > } > > If %BLOCK_OPT_SIZE is a valid option, and has the special value > (uint64_t)-1, then we must have a backing file (error out if not), and > we use its size instead. In other words, the special value means "use > the backing file's size". > > The value can come either from @img_size or from @options. > > Some callers indeed pass (uint64_t)-1 for @img_size. For instance, > qemu-img create passes it when run without the optional size argument. > Works. > > qemu-img is the only caller passing @options. With -o size=-1, it > actually puts (uint64_t)-1 into @param (parse_option_size() sucks). > Weird. > > The whole bloody mess should be cleaned up. But I'm not asking you to > do that now. > > I do think, however, that you should stick to > > qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) > > instead of your voodoo > > qemu_opt_get_number(opts, BLOCK_OPT_SIZE, -1) > >>>> uint64_t size; >>>> - char buf[32]; >>>> int back_flags; >>>> >>>> /* backing files always opened read-only */ >>>> @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const char *fmt, >>>> >>>> bs = bdrv_new(""); >>>> >>>> - ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); >>>> + ret = bdrv_open(bs, backing_file, back_flags, backing_drv); >>>> if (ret < 0) { >>>> error_setg_errno(errp, -ret, "Could not open '%s'", >>>> - backing_file->value.s); >>>> + backing_file); >>>> goto out; >>>> } >>>> bdrv_get_geometry(bs, &size); >>>> 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); >>>> } else { >>>> error_setg(errp, "Image creation needs a size parameter"); >>>> goto out; >>>> @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>>> } >>>> >>>> printf("Formatting '%s', fmt=%s ", filename, fmt); >>>> - print_option_parameters(param); >>>> + qemu_opts_print(opts, NULL); >>>> puts(""); >>>> >>>> - ret = bdrv_create(drv, filename, param); >>>> + ret = bdrv_create(drv, filename, opts); >>>> if (ret < 0) { >>>> if (ret == -ENOTSUP) { >>>> error_setg(errp,"Formatting or formatting option not supported for " >>>> @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >>>> } >>>> >>>> out: >>>> - free_option_parameters(create_options); >>>> - free_option_parameters(param); >>>> + free_opts_list(create_options); >>>> + if (opts) { >>>> + qemu_opts_del(opts); >>>> + } >>>> >>>> if (bs) { >>>> bdrv_delete(bs); >>>> diff --git a/block/cow.c b/block/cow.c >>>> index a33ce95..5442c9c 100644 >>>> --- a/block/cow.c >>>> +++ b/block/cow.c >>>> @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs) >>>> { >>>> } >>>> >>>> -static int cow_create(const char *filename, QEMUOptionParameter *options) >>>> +static int cow_create(const char *filename, QemuOpts *opts) >>>> { >>>> struct cow_header_v2 cow_header; >>>> struct stat st; >>>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) >>>> int ret; >>>> BlockDriverState *cow_bs; >>>> >>>> - /* Read out options */ >>>> - while (options && options->name) { >>>> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >>>> - image_sectors = options->value.n / 512; >>>> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >>>> - image_filename = options->value.s; >>>> - } >>>> - options++; >>>> + /* Read out opts */ >>>> + if (opts) { >>> >>> I suspect you need the if (opts) here and in many other places only >>> because you special-cased "both lists empty" in append_opts_list(). I >>> suspect things become easier if you drop that. >> >> No, in this version, if(opt) is needed in "protocol", not needed in >> "format", I want to have the same code style, so I also judged if opts >> is NULL in "format" _create functions. Do you think is it acceptable? > > I still don't get it. Can you explain how opts can be null here even > when append_opts_list() is changed not to return null? > I mean: When I use protocol, such as gluster: /usr/bin/qemu-img create -f qed -o cluster_size=65536 gluster://kvm11/single-volume/12.qed 10M Then opts will be null: qemu_gluster_create (filename=0x555555c11930 "gluster://kvm11/single-volume/12.qed", opts=0x0) at block/gluster.c:339 So it checked if opts is NULL now: 352 if (opts) { 353 total_size = 354 qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; 355 } I want to make these code in only one kind of style, so I judged if opts is NULL in block format code. >>> >>>> + image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; >>>> + image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); >>>> } >>>> >>>> - ret = bdrv_create_file(filename, options); >>>> + ret = bdrv_create_file(filename, opts); >>>> if (ret < 0) { >>>> return ret; >>>> } >>>> @@ -318,18 +314,22 @@ exit: >>>> return ret; >>>> } >>>> >>>> -static QEMUOptionParameter cow_create_options[] = { >>>> - { >>>> - .name = BLOCK_OPT_SIZE, >>>> - .type = OPT_SIZE, >>>> - .help = "Virtual disk size" >>>> - }, >>>> - { >>>> - .name = BLOCK_OPT_BACKING_FILE, >>>> - .type = OPT_STRING, >>>> - .help = "File name of a base image" >>>> - }, >>>> - { NULL } >>>> +static QemuOptsList cow_create_opts = { >>>> + .name = "cow-create-opts", >>>> + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), >>>> + .desc = { >>>> + { >>>> + .name = BLOCK_OPT_SIZE, >>>> + .type = QEMU_OPT_NUMBER, >>> >>> QEMU_OPT_SIZE, or else we lose support for suffixes, don't we? >>> >> In qemu-img.c, strtosz_suffix convert suffixes to regular digitals. > > That's correct for the optional size argument, but not for -o size=... > >> Okay. I will change to QEMU_OPT_SIZE. > > Please do, and test that both the size argument and -o size=... works > just like before, with and without suffixes. > >>>> + .help = "Virtual disk size" >>>> + }, >>>> + { >>>> + .name = BLOCK_OPT_BACKING_FILE, >>>> + .type = QEMU_OPT_STRING, >>>> + .help = "File name of a base image" >>>> + }, >>>> + { /* end of list */ } >>>> + } >>>> }; >>>> >>>> static BlockDriver bdrv_cow = { >>>> @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = { >>>> .bdrv_write = cow_co_write, >>>> .bdrv_co_is_allocated = cow_co_is_allocated, >>>> >>>> - .create_options = cow_create_options, >>>> + .bdrv_create_options = &cow_create_opts, >>>> }; >>>> >>>> static void bdrv_cow_init(void) >>>> diff --git a/block/gluster.c b/block/gluster.c >>>> index 0f2c32a..a41c684 100644 >>>> --- a/block/gluster.c >>>> +++ b/block/gluster.c >>>> @@ -335,8 +335,7 @@ out: >>>> return ret; >>>> } >>>> >>>> -static int qemu_gluster_create(const char *filename, >>>> - QEMUOptionParameter *options) >>>> +static int qemu_gluster_create(const char *filename, QemuOpts* opts) >>> >>> Space before the *, not after: QemuOpts *opts >>> >> Okay. > > [...] > > >
Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > Markus Armbruster <armbru@redhat.com> writes: >> Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: >> >>> Markus Armbruster <armbru@redhat.com> writes: >>>> Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: [...] >>>>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) >>>>> int ret; >>>>> BlockDriverState *cow_bs; >>>>> >>>>> - /* Read out options */ >>>>> - while (options && options->name) { >>>>> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >>>>> - image_sectors = options->value.n / 512; >>>>> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >>>>> - image_filename = options->value.s; >>>>> - } >>>>> - options++; >>>>> + /* Read out opts */ >>>>> + if (opts) { >>>> >>>> I suspect you need the if (opts) here and in many other places only >>>> because you special-cased "both lists empty" in append_opts_list(). I >>>> suspect things become easier if you drop that. >>> >>> No, in this version, if(opt) is needed in "protocol", not needed in >>> "format", I want to have the same code style, so I also judged if opts >>> is NULL in "format" _create functions. Do you think is it acceptable? >> >> I still don't get it. Can you explain how opts can be null here even >> when append_opts_list() is changed not to return null? >> > I mean: When I use protocol, such as gluster: > /usr/bin/qemu-img create -f qed -o cluster_size=65536 > gluster://kvm11/single-volume/12.qed 10M > > Then opts will be null: > qemu_gluster_create (filename=0x555555c11930 > "gluster://kvm11/single-volume/12.qed", opts=0x0) at block/gluster.c:339 > > So it checked if opts is NULL now: > 352 if (opts) { > 353 total_size = > 354 qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / > BDRV_SECTOR_SIZE; > 355 } I see. > I want to make these code in only one kind of style, so I judged if opts > is NULL in block format code. As far as I can tell, there are just two sources of NULL QEMUOptionParameter *: 1. parse_option_parameters() returns NULL on error (documented), but some callers use it like this without checking for errors: param = parse_option_parameters("", create_options, param); The only error that can happen then is null create_options. Depending on that is slightly unclean. Your patch replaces these uses by opts = qemu_opts_create_nofail(create_options); which cannot return NULL. I'd call that an improvement. 2. qed_create() calls bdrv_create_file(filename, NULL), which passes on the null options to bdrv_create(). To get rid of this source, one of the two functions needs to create empty options. Your patch adds a third source: 3. bdrv_img_create() merges drv->create_options and proto_drv->create_options (both possibly null) into create_options. Unlike the current code, your patch merges two empty lists into null. I don't think that's a wise change, and asked you to drop this special case in my review of PATCH 2/4. [...]
于 2013-1-29 17:45, Markus Armbruster 写道: > Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > >> Markus Armbruster <armbru@redhat.com> writes: >>> Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: >>> >>>> Markus Armbruster <armbru@redhat.com> writes: >>>>> Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > [...] >>>>>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) >>>>>> int ret; >>>>>> BlockDriverState *cow_bs; >>>>>> >>>>>> - /* Read out options */ >>>>>> - while (options && options->name) { >>>>>> - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { >>>>>> - image_sectors = options->value.n / 512; >>>>>> - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { >>>>>> - image_filename = options->value.s; >>>>>> - } >>>>>> - options++; >>>>>> + /* Read out opts */ >>>>>> + if (opts) { >>>>> >>>>> I suspect you need the if (opts) here and in many other places only >>>>> because you special-cased "both lists empty" in append_opts_list(). I >>>>> suspect things become easier if you drop that. >>>> >>>> No, in this version, if(opt) is needed in "protocol", not needed in >>>> "format", I want to have the same code style, so I also judged if opts >>>> is NULL in "format" _create functions. Do you think is it acceptable? >>> >>> I still don't get it. Can you explain how opts can be null here even >>> when append_opts_list() is changed not to return null? >>> >> I mean: When I use protocol, such as gluster: >> /usr/bin/qemu-img create -f qed -o cluster_size=65536 >> gluster://kvm11/single-volume/12.qed 10M >> >> Then opts will be null: >> qemu_gluster_create (filename=0x555555c11930 >> "gluster://kvm11/single-volume/12.qed", opts=0x0) at block/gluster.c:339 >> >> So it checked if opts is NULL now: >> 352 if (opts) { >> 353 total_size = >> 354 qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / >> BDRV_SECTOR_SIZE; >> 355 } > > I see. > >> I want to make these code in only one kind of style, so I judged if opts >> is NULL in block format code. > > As far as I can tell, there are just two sources of NULL > QEMUOptionParameter *: > > 1. parse_option_parameters() returns NULL on error (documented), but > some callers use it like this without checking for errors: > > param = parse_option_parameters("", create_options, param); > > The only error that can happen then is null create_options. > Depending on that is slightly unclean. > > Your patch replaces these uses by > > opts = qemu_opts_create_nofail(create_options); > > which cannot return NULL. I'd call that an improvement. > > 2. qed_create() calls bdrv_create_file(filename, NULL), which passes on > the null options to bdrv_create(). To get rid of this source, one of > the two functions needs to create empty options. > > Your patch adds a third source: > > 3. bdrv_img_create() merges drv->create_options and > proto_drv->create_options (both possibly null) into create_options. > Unlike the current code, your patch merges two empty lists into null. > I don't think that's a wise change, and asked you to drop this > special case in my review of PATCH 2/4. > > [...] Got it, thank you Markus, I will consider your comments in next version. > > >
diff --git a/block.c b/block.c index 6fa7c90..56e4613 100644 --- a/block.c +++ b/block.c @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name) typedef struct CreateCo { BlockDriver *drv; char *filename; - QEMUOptionParameter *options; + QemuOpts *opts; int ret; } CreateCo; @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) CreateCo *cco = opaque; assert(cco->drv); - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); } int bdrv_create(BlockDriver *drv, const char* filename, - QEMUOptionParameter *options) + QemuOpts *opts) { int ret; @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, CreateCo cco = { .drv = drv, .filename = g_strdup(filename), - .options = options, + .opts = opts, .ret = NOT_DONE, }; @@ -405,7 +405,7 @@ out: return ret; } -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) +int bdrv_create_file(const char *filename, QemuOpts *opts) { BlockDriver *drv; @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) return -ENOENT; } - return bdrv_create(drv, filename, options); + return bdrv_create(drv, filename, opts); } /* @@ -794,7 +794,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, int64_t total_size; int is_protocol = 0; BlockDriver *bdrv_qcow2; - QEMUOptionParameter *options; + QemuOpts *opts; char backing_filename[PATH_MAX]; /* if snapshot, we create a temporary backing file and open it @@ -827,17 +827,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, return -errno; bdrv_qcow2 = bdrv_find_format("qcow2"); - options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_options); - set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size); - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); if (drv) { - set_option_parameter(options, BLOCK_OPT_BACKING_FMT, - drv->format_name); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); } - ret = bdrv_create(bdrv_qcow2, tmp_filename, options); - free_option_parameters(options); + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); + qemu_opts_del(opts); if (ret < 0) { return ret; } @@ -4493,8 +4492,10 @@ void bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, char *options, uint64_t img_size, int flags, Error **errp) { - QEMUOptionParameter *param = NULL, *create_options = NULL; - QEMUOptionParameter *backing_fmt, *backing_file, *size; + QemuOpts *opts = NULL; + QemuOptsList *create_options = NULL; + const char *backing_fmt, *backing_file; + int64_t size; BlockDriverState *bs = NULL; BlockDriver *drv, *proto_drv; BlockDriver *backing_drv = NULL; @@ -4512,28 +4513,23 @@ void bdrv_img_create(const char *filename, const char *fmt, error_setg(errp, "Unknown protocol '%s'", filename); return; } - - create_options = append_option_parameters(create_options, - drv->create_options); - create_options = append_option_parameters(create_options, - proto_drv->create_options); - + create_options = append_opts_list(drv->bdrv_create_options, + proto_drv->bdrv_create_options); /* Create parameter list with default values */ - param = parse_option_parameters("", create_options, param); + opts = qemu_opts_create_nofail(create_options); - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); + 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, + if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) { error_setg(errp, "Backing file not supported for file format '%s'", fmt); @@ -4542,39 +4538,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); + error_setg(errp, "Unknown backing file format '%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_number(opts, BLOCK_OPT_SIZE, -1); + if (size == -1) { + if (backing_file) { uint64_t size; - char buf[32]; int back_flags; /* backing files always opened read-only */ @@ -4583,17 +4577,16 @@ void bdrv_img_create(const char *filename, const char *fmt, bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); + ret = bdrv_open(bs, backing_file, back_flags, backing_drv); if (ret < 0) { error_setg_errno(errp, -ret, "Could not open '%s'", - backing_file->value.s); + backing_file); goto out; } bdrv_get_geometry(bs, &size); 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); } else { error_setg(errp, "Image creation needs a size parameter"); goto out; @@ -4601,10 +4594,10 @@ void bdrv_img_create(const char *filename, const char *fmt, } printf("Formatting '%s', fmt=%s ", filename, fmt); - print_option_parameters(param); + qemu_opts_print(opts, NULL); puts(""); - ret = bdrv_create(drv, filename, param); + ret = bdrv_create(drv, filename, opts); if (ret < 0) { if (ret == -ENOTSUP) { error_setg(errp,"Formatting or formatting option not supported for " @@ -4619,8 +4612,10 @@ void bdrv_img_create(const char *filename, const char *fmt, } out: - free_option_parameters(create_options); - free_option_parameters(param); + free_opts_list(create_options); + if (opts) { + qemu_opts_del(opts); + } if (bs) { bdrv_delete(bs); diff --git a/block/cow.c b/block/cow.c index a33ce95..5442c9c 100644 --- a/block/cow.c +++ b/block/cow.c @@ -255,7 +255,7 @@ static void cow_close(BlockDriverState *bs) { } -static int cow_create(const char *filename, QEMUOptionParameter *options) +static int cow_create(const char *filename, QemuOpts *opts) { struct cow_header_v2 cow_header; struct stat st; @@ -264,17 +264,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) int ret; BlockDriverState *cow_bs; - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - image_sectors = options->value.n / 512; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - image_filename = options->value.s; - } - options++; + /* Read out opts */ + if (opts) { + image_sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; + image_filename = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); } - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { return ret; } @@ -318,18 +314,22 @@ exit: return ret; } -static QEMUOptionParameter cow_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { NULL } +static QemuOptsList cow_create_opts = { + .name = "cow-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_cow = { @@ -345,7 +345,7 @@ static BlockDriver bdrv_cow = { .bdrv_write = cow_co_write, .bdrv_co_is_allocated = cow_co_is_allocated, - .create_options = cow_create_options, + .bdrv_create_options = &cow_create_opts, }; static void bdrv_cow_init(void) diff --git a/block/gluster.c b/block/gluster.c index 0f2c32a..a41c684 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -335,8 +335,7 @@ out: return ret; } -static int qemu_gluster_create(const char *filename, - QEMUOptionParameter *options) +static int qemu_gluster_create(const char *filename, QemuOpts* opts) { struct glfs *glfs; struct glfs_fd *fd; @@ -350,11 +349,9 @@ static int qemu_gluster_create(const char *filename, goto out; } - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n / BDRV_SECTOR_SIZE; - } - options++; + if (opts) { + total_size = + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; } fd = glfs_creat(glfs, gconf->image, @@ -544,13 +541,17 @@ static void qemu_gluster_close(BlockDriverState *bs) glfs_fini(s->glfs); } -static QEMUOptionParameter qemu_gluster_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } +static QemuOptsList qemu_gluster_create_opts = { + .name = "qemu-gluster-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_gluster = { @@ -565,7 +566,7 @@ static BlockDriver bdrv_gluster = { .bdrv_aio_readv = qemu_gluster_aio_readv, .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, - .create_options = qemu_gluster_create_options, + .bdrv_create_options = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_tcp = { @@ -580,7 +581,7 @@ static BlockDriver bdrv_gluster_tcp = { .bdrv_aio_readv = qemu_gluster_aio_readv, .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, - .create_options = qemu_gluster_create_options, + .bdrv_create_options = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_unix = { @@ -595,7 +596,7 @@ static BlockDriver bdrv_gluster_unix = { .bdrv_aio_readv = qemu_gluster_aio_readv, .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, - .create_options = qemu_gluster_create_options, + .bdrv_create_options = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_rdma = { @@ -610,7 +611,7 @@ static BlockDriver bdrv_gluster_rdma = { .bdrv_aio_readv = qemu_gluster_aio_readv, .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, - .create_options = qemu_gluster_create_options, + .bdrv_create_options = &qemu_gluster_create_opts, }; static void bdrv_gluster_init(void) diff --git a/block/qcow.c b/block/qcow.c index 4276610..4691a3e 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -651,7 +651,7 @@ static void qcow_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static int qcow_create(const char *filename, QEMUOptionParameter *options) +static int qcow_create(const char *filename, QemuOpts *opts) { int header_size, backing_filename_len, l1_size, shift, i; QCowHeader header; @@ -662,19 +662,16 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) int ret; BlockDriverState *qcow_bs; - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n / 512; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; + /* Read out opts */ + if (opts) { + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { + flags |= BLOCK_FLAG_ENCRYPT; } - options++; } - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { return ret; } @@ -851,24 +848,27 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return 0; } - -static QEMUOptionParameter qcow_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { - .name = BLOCK_OPT_ENCRYPT, - .type = OPT_FLAG, - .help = "Encrypt the image" - }, - { NULL } +static QemuOptsList qcow_create_opts = { + .name = "qcow-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_ENCRYPT, + .type = QEMU_OPT_BOOL, + .help = "Encrypt the image" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_qcow = { @@ -889,7 +889,7 @@ static BlockDriver bdrv_qcow = { .bdrv_write_compressed = qcow_write_compressed, .bdrv_get_info = qcow_get_info, - .create_options = qcow_create_options, + .bdrv_create_options = &qcow_create_opts, }; static void bdrv_qcow_init(void) diff --git a/block/qcow2.c b/block/qcow2.c index f6abff6..e3ee4af 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1177,7 +1177,7 @@ static int preallocate(BlockDriverState *bs) static int qcow2_create2(const char *filename, int64_t total_size, const char *backing_file, const char *backing_format, int flags, size_t cluster_size, int prealloc, - QEMUOptionParameter *options, int version) + QemuOpts *opts, int version) { /* Calculate cluster_bits */ int cluster_bits; @@ -1208,7 +1208,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, uint8_t* refcount_table; int ret; - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { return ret; } @@ -1311,7 +1311,7 @@ out: return ret; } -static int qcow2_create(const char *filename, QEMUOptionParameter *options) +static int qcow2_create(const char *filename, QemuOpts *opts) { const char *backing_file = NULL; const char *backing_fmt = NULL; @@ -1320,45 +1320,43 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) size_t cluster_size = DEFAULT_CLUSTER_SIZE; int prealloc = 0; int version = 2; + const char *buf; /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - sectors = options->value.n / 512; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { - backing_fmt = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) { - flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0; - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { - if (options->value.n) { - cluster_size = options->value.n; - } - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { - if (!options->value.s || !strcmp(options->value.s, "off")) { - prealloc = 0; - } else if (!strcmp(options->value.s, "metadata")) { - prealloc = 1; - } else { - fprintf(stderr, "Invalid preallocation mode: '%s'\n", - options->value.s); - return -EINVAL; - } - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) { - if (!options->value.s || !strcmp(options->value.s, "0.10")) { - version = 2; - } else if (!strcmp(options->value.s, "1.1")) { - version = 3; - } else { - fprintf(stderr, "Invalid compatibility level: '%s'\n", - options->value.s); - return -EINVAL; - } - } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) { - flags |= options->value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0; + if (opts) { + sectors = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); + if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) { + flags |= BLOCK_FLAG_ENCRYPT; + } + cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, + DEFAULT_CLUSTER_SIZE); + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); + if (!buf || !strcmp(buf, "off")) { + prealloc = 0; + } else if (!strcmp(buf, "metadata")) { + prealloc = 1; + } else { + fprintf(stderr, "Invalid preallocation mode: '%s'\n", + buf); + return -EINVAL; + } + + buf = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL); + if (!buf || !strcmp(buf, "0.10")) { + version = 2; + } else if (!strcmp(buf, "1.1")) { + version = 3; + } else { + fprintf(stderr, "Invalid compatibility level: '%s'\n", + buf); + return -EINVAL; + } + + if (qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) { + flags |= BLOCK_FLAG_LAZY_REFCOUNTS; } - options++; } if (backing_file && prealloc) { @@ -1374,7 +1372,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options) } return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, - cluster_size, prealloc, options, version); + cluster_size, prealloc, opts, version); } static int qcow2_make_empty(BlockDriverState *bs) @@ -1635,49 +1633,53 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, return ret; } -static QEMUOptionParameter qcow2_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_COMPAT_LEVEL, - .type = OPT_STRING, - .help = "Compatibility level (0.10 or 1.1)" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { - .name = BLOCK_OPT_BACKING_FMT, - .type = OPT_STRING, - .help = "Image format of the base image" - }, - { - .name = BLOCK_OPT_ENCRYPT, - .type = OPT_FLAG, - .help = "Encrypt the image" - }, - { - .name = BLOCK_OPT_CLUSTER_SIZE, - .type = OPT_SIZE, - .help = "qcow2 cluster size", - .value = { .n = DEFAULT_CLUSTER_SIZE }, - }, - { - .name = BLOCK_OPT_PREALLOC, - .type = OPT_STRING, - .help = "Preallocation mode (allowed values: off, metadata)" - }, - { - .name = BLOCK_OPT_LAZY_REFCOUNTS, - .type = OPT_FLAG, - .help = "Postpone refcount updates", - }, - { NULL } +static QemuOptsList qcow2_create_opts = { + .name = "qcow2-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_COMPAT_LEVEL, + .type = QEMU_OPT_STRING, + .help = "Compatibility level (0.10 or 1.1)" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_BACKING_FMT, + .type = QEMU_OPT_STRING, + .help = "Image format of the base image" + }, + { + .name = BLOCK_OPT_ENCRYPT, + .type = QEMU_OPT_BOOL, + .help = "Encrypt the image" + }, + { + .name = BLOCK_OPT_CLUSTER_SIZE, + .type = QEMU_OPT_SIZE, + .help = "qcow2 cluster size", + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) + }, + { + .name = BLOCK_OPT_PREALLOC, + .type = QEMU_OPT_STRING, + .help = "Preallocation mode (allowed values: off, metadata)" + }, + { + .name = BLOCK_OPT_LAZY_REFCOUNTS, + .type = QEMU_OPT_BOOL, + .help = "Postpone refcount updates", + }, + { /* end of list */ } + } }; static BlockDriver bdrv_qcow2 = { @@ -1715,8 +1717,9 @@ static BlockDriver bdrv_qcow2 = { .bdrv_invalidate_cache = qcow2_invalidate_cache, - .create_options = qcow2_create_options, .bdrv_check = qcow2_check, + + .bdrv_create_options = &qcow2_create_opts, }; static void bdrv_qcow2_init(void) diff --git a/block/qed.c b/block/qed.c index cf85d8f..766ba6f 100644 --- a/block/qed.c +++ b/block/qed.c @@ -603,7 +603,7 @@ out: return ret; } -static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) +static int bdrv_qed_create(const char *filename, QemuOpts *opts) { uint64_t image_size = 0; uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE; @@ -611,23 +611,15 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) const char *backing_file = NULL; const char *backing_fmt = NULL; - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - image_size = options->value.n; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) { - backing_fmt = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { - if (options->value.n) { - cluster_size = options->value.n; - } - } else if (!strcmp(options->name, BLOCK_OPT_TABLE_SIZE)) { - if (options->value.n) { - table_size = options->value.n; - } - } - options++; + if (opts) { + image_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); + backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); + cluster_size = qemu_opt_get_size(opts, + BLOCK_OPT_CLUSTER_SIZE, + QED_DEFAULT_CLUSTER_SIZE); + table_size = qemu_opt_get_size(opts, BLOCK_OPT_TABLE_SIZE, + QED_DEFAULT_TABLE_SIZE); } if (!qed_is_cluster_size_valid(cluster_size)) { @@ -1537,36 +1529,44 @@ static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result, return qed_check(s, result, !!fix); } -static QEMUOptionParameter qed_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size (in bytes)" - }, { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, { - .name = BLOCK_OPT_BACKING_FMT, - .type = OPT_STRING, - .help = "Image format of the base image" - }, { - .name = BLOCK_OPT_CLUSTER_SIZE, - .type = OPT_SIZE, - .help = "Cluster size (in bytes)", - .value = { .n = QED_DEFAULT_CLUSTER_SIZE }, - }, { - .name = BLOCK_OPT_TABLE_SIZE, - .type = OPT_SIZE, - .help = "L1/L2 table size (in clusters)" - }, - { /* end of list */ } +static QemuOptsList qed_create_opts = { + .name = "qed-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(qed_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_BACKING_FMT, + .type = QEMU_OPT_STRING, + .help = "Image format of the base image" + }, + { + .name = BLOCK_OPT_CLUSTER_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Cluster size (in bytes)", + .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE), + }, + { + .name = BLOCK_OPT_TABLE_SIZE, + .type = QEMU_OPT_SIZE, + .help = "L1/L2 table size (in clusters)" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_qed = { .format_name = "qed", .instance_size = sizeof(BDRVQEDState), - .create_options = qed_create_options, + .bdrv_create_options = &qed_create_opts, .bdrv_probe = bdrv_qed_probe, .bdrv_rebind = bdrv_qed_rebind, diff --git a/block/qed.h b/block/qed.h index 2b4dded..99a7726 100644 --- a/block/qed.h +++ b/block/qed.h @@ -43,6 +43,7 @@ * * All fields are little-endian on disk. */ +#define QED_DEFAULT_CLUSTER_SIZE 65536 enum { QED_MAGIC = 'Q' | 'E' << 8 | 'D' << 16 | '\0' << 24, @@ -69,7 +70,6 @@ enum { */ QED_MIN_CLUSTER_SIZE = 4 * 1024, /* in bytes */ QED_MAX_CLUSTER_SIZE = 64 * 1024 * 1024, - QED_DEFAULT_CLUSTER_SIZE = 64 * 1024, /* Allocated clusters are tracked using a 2-level pagetable. Table size is * a multiple of clusters so large maximum image sizes can be supported diff --git a/block/raw-posix.c b/block/raw-posix.c index 657af95..890c8a7 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -123,6 +123,19 @@ #define MAX_BLOCKSIZE 4096 +static QemuOptsList file_proto_create_opts = { + .name = "file-proto-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(file_proto_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, + { /* end of list */ } + } +}; + typedef struct BDRVRawState { int fd; int type; @@ -997,18 +1010,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) return (int64_t)st.st_blocks * 512; } -static int raw_create(const char *filename, QEMUOptionParameter *options) +static int raw_create(const char *filename, QemuOpts *opts) { int fd; int result = 0; int64_t total_size = 0; - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n / BDRV_SECTOR_SIZE; - } - options++; + if (opts) { + total_size = + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; } fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, @@ -1136,15 +1146,6 @@ static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs, cb, opaque, QEMU_AIO_DISCARD); } -static QEMUOptionParameter raw_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } -}; - static BlockDriver bdrv_file = { .format_name = "file", .protocol_name = "file", @@ -1167,8 +1168,7 @@ static BlockDriver bdrv_file = { .bdrv_getlength = raw_getlength, .bdrv_get_allocated_file_size = raw_get_allocated_file_size, - - .create_options = raw_create_options, + .bdrv_create_options = &file_proto_create_opts, }; /***********************************************/ @@ -1403,19 +1403,16 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); } -static int hdev_create(const char *filename, QEMUOptionParameter *options) +static int hdev_create(const char *filename, QemuOpts *opts) { int fd; int ret = 0; struct stat stat_buf; int64_t total_size = 0; - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, "size")) { - total_size = options->value.n / BDRV_SECTOR_SIZE; - } - options++; + if (opts) { + total_size = + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; } fd = qemu_open(filename, O_WRONLY | O_BINARY); @@ -1440,7 +1437,7 @@ static int hdev_has_zero_init(BlockDriverState *bs) static BlockDriver bdrv_host_device = { .format_name = "host_device", - .protocol_name = "host_device", + .protocol_name = "host_device", .instance_size = sizeof(BDRVRawState), .bdrv_probe_device = hdev_probe_device, .bdrv_file_open = hdev_open, @@ -1449,7 +1446,6 @@ static BlockDriver bdrv_host_device = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, .bdrv_has_zero_init = hdev_has_zero_init, .bdrv_aio_readv = raw_aio_readv, @@ -1458,9 +1454,10 @@ static BlockDriver bdrv_host_device = { .bdrv_aio_discard = hdev_aio_discard, .bdrv_truncate = raw_truncate, - .bdrv_getlength = raw_getlength, + .bdrv_getlength = raw_getlength, .bdrv_get_allocated_file_size = raw_get_allocated_file_size, + .bdrv_create_options = &file_proto_create_opts, /* generic scsi device */ #ifdef __linux__ @@ -1574,7 +1571,6 @@ static BlockDriver bdrv_host_floppy = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, .bdrv_has_zero_init = hdev_has_zero_init, .bdrv_aio_readv = raw_aio_readv, @@ -1590,6 +1586,7 @@ static BlockDriver bdrv_host_floppy = { .bdrv_is_inserted = floppy_is_inserted, .bdrv_media_changed = floppy_media_changed, .bdrv_eject = floppy_eject, + .bdrv_create_options = &file_proto_create_opts, }; static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) @@ -1676,7 +1673,6 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, .bdrv_has_zero_init = hdev_has_zero_init, .bdrv_aio_readv = raw_aio_readv, @@ -1696,6 +1692,8 @@ static BlockDriver bdrv_host_cdrom = { /* generic scsi device */ .bdrv_ioctl = hdev_ioctl, .bdrv_aio_ioctl = hdev_aio_ioctl, + + .bdrv_create_options = &file_proto_create_opts, }; #endif /* __linux__ */ @@ -1798,7 +1796,6 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, .bdrv_has_zero_init = hdev_has_zero_init, .bdrv_aio_readv = raw_aio_readv, diff --git a/block/raw-win32.c b/block/raw-win32.c index b89ac19..34826e0 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -382,17 +382,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) return st.st_size; } -static int raw_create(const char *filename, QEMUOptionParameter *options) +static int raw_create(const char *filename, QemuOpts *opts) { int fd; int64_t total_size = 0; /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n / 512; - } - options++; + if (opts) { + total_size = + qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) / 512; } fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, @@ -405,13 +403,17 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) return 0; } -static QEMUOptionParameter raw_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } +static QemuOptsList raw_create_opts = { + .name = "raw-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_file = { @@ -431,7 +433,7 @@ static BlockDriver bdrv_file = { .bdrv_get_allocated_file_size = raw_get_allocated_file_size, - .create_options = raw_create_options, + .bdrv_create_options = &raw_create_opts, }; /***********************************************/ diff --git a/block/raw.c b/block/raw.c index 75812db..033ffa6 100644 --- a/block/raw.c +++ b/block/raw.c @@ -95,18 +95,22 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque); } -static int raw_create(const char *filename, QEMUOptionParameter *options) -{ - return bdrv_create_file(filename, options); -} - -static QEMUOptionParameter raw_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } +static int raw_create(const char *filename, QemuOpts *opts) +{ + return bdrv_create_file(filename, opts); +} + +static QemuOptsList raw_create_opts = { + .name = "raw-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; static int raw_has_zero_init(BlockDriverState *bs) @@ -143,8 +147,8 @@ static BlockDriver bdrv_raw = { .bdrv_aio_ioctl = raw_aio_ioctl, .bdrv_create = raw_create, - .create_options = raw_create_options, .bdrv_has_zero_init = raw_has_zero_init, + .bdrv_create_options = &raw_create_opts, }; static void bdrv_raw_init(void) diff --git a/block/rbd.c b/block/rbd.c index 8cd10a7..a9f8772 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -287,7 +287,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf) return ret; } -static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) +static int qemu_rbd_create(const char *filename, QemuOpts *opts) { int64_t bytes = 0; int64_t objsize; @@ -310,24 +310,20 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options) } /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - bytes = options->value.n; - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { - if (options->value.n) { - objsize = options->value.n; - if ((objsize - 1) & objsize) { /* not a power of 2? */ - error_report("obj size needs to be power of 2"); - return -EINVAL; - } - if (objsize < 4096) { - error_report("obj size too small"); - return -EINVAL; - } - obj_order = ffs(objsize) - 1; + if (opts) { + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); + objsize = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, 0); + if (objsize) { + if ((objsize - 1) & objsize) { /* not a power of 2? */ + error_report("obj size needs to be power of 2"); + return -EINVAL; } + if (objsize < 4096) { + error_report("obj size too small"); + return -EINVAL; + } + obj_order = ffs(objsize) - 1; } - options++; } clientname = qemu_rbd_parse_clientname(conf, clientname_buf); @@ -920,20 +916,24 @@ static BlockDriverAIOCB* qemu_rbd_aio_discard(BlockDriverState *bs, } #endif -static QEMUOptionParameter qemu_rbd_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_CLUSTER_SIZE, - .type = OPT_SIZE, - .help = "RBD object size" - }, - {NULL} +static QemuOptsList rbd_create_opts = { + .name = "rbd-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(rbd_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_CLUSTER_SIZE, + .type = QEMU_OPT_SIZE, + .help = "RBD object size", + .def_value_str = stringify(0), + }, + { /* end of list */ } + } }; - static BlockDriver bdrv_rbd = { .format_name = "rbd", .instance_size = sizeof(BDRVRBDState), @@ -941,7 +941,7 @@ static BlockDriver bdrv_rbd = { .bdrv_close = qemu_rbd_close, .bdrv_create = qemu_rbd_create, .bdrv_get_info = qemu_rbd_getinfo, - .create_options = qemu_rbd_create_options, + .bdrv_create_options = &rbd_create_opts, .bdrv_getlength = qemu_rbd_getlength, .bdrv_truncate = qemu_rbd_truncate, .protocol_name = "rbd", diff --git a/block/sheepdog.c b/block/sheepdog.c index 3e49bb8..d515e56 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1274,12 +1274,12 @@ out: return ret; } -static int sd_create(const char *filename, QEMUOptionParameter *options) +static int sd_create(const char *filename, QemuOpts *opts) { int ret = 0; uint32_t vid = 0, base_vid = 0; int64_t vdi_size = 0; - char *backing_file = NULL; + const char *backing_file = NULL, *buf = NULL; BDRVSheepdogState *s; char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; uint32_t snapid; @@ -1298,26 +1298,20 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) goto out; } - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - vdi_size = options->value.n; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { - if (!options->value.s || !strcmp(options->value.s, "off")) { - prealloc = false; - } else if (!strcmp(options->value.s, "full")) { - prealloc = true; - } else { - error_report("Invalid preallocation mode: '%s'", - options->value.s); - ret = -EINVAL; - goto out; - } + if (opts) { + vdi_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); + buf = qemu_opt_get(opts, BLOCK_OPT_PREALLOC); + if (!buf || !strcmp(buf, "off")) { + prealloc = false; + } else if (!strcmp(buf, "full")) { + prealloc = true; + } else { + error_report("Invalid preallocation mode: '%s'", buf); + ret = -EINVAL; + goto out; } - options++; } - if (vdi_size > SD_MAX_VDI_SIZE) { error_report("too big image size"); ret = -EINVAL; @@ -2042,24 +2036,27 @@ static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data, return do_load_save_vmstate(s, data, pos, size, 1); } - -static QEMUOptionParameter sd_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { - .name = BLOCK_OPT_PREALLOC, - .type = OPT_STRING, - .help = "Preallocation mode (allowed values: off, full)" - }, - { NULL } +static QemuOptsList sd_create_opts = { + .name = "sheepdog-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_PREALLOC, + .type = QEMU_OPT_STRING, + .help = "Preallocation mode (allowed values: off, full)" + }, + { /* end of list */ } + } }; BlockDriver bdrv_sheepdog = { @@ -2084,7 +2081,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_save_vmstate = sd_save_vmstate, .bdrv_load_vmstate = sd_load_vmstate, - .create_options = sd_create_options, + .bdrv_create_options = &sd_create_opts, }; static void bdrv_sheepdog_init(void) diff --git a/block/vdi.c b/block/vdi.c index 021abaa..95f738e 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -620,7 +620,7 @@ static int vdi_co_write(BlockDriverState *bs, return ret; } -static int vdi_create(const char *filename, QEMUOptionParameter *options) +static int vdi_create(const char *filename, QemuOpts *opts) { int fd; int result = 0; @@ -635,24 +635,19 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) logout("\n"); /* Read out options. */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - bytes = options->value.n; + if (opts) { + bytes = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); #if defined(CONFIG_VDI_BLOCK_SIZE) - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { - if (options->value.n) { - /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ - block_size = options->value.n; - } + /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ + block_size = qemu_opt_get_size(opts, + BLOCK_OPT_CLUSTER_SIZE, + DEFAULT_CLUSTER_SIZE); #endif #if defined(CONFIG_VDI_STATIC_IMAGE) - } else if (!strcmp(options->name, BLOCK_OPT_STATIC)) { - if (options->value.n) { - image_type = VDI_TYPE_STATIC; - } -#endif + if (qemu_opt_get_bool(opts, BLOCK_OPT_STATIC, 0)) { + image_type = VDI_TYPE_STATIC; } - options++; +#endif } fd = qemu_open(filename, @@ -733,29 +728,33 @@ static void vdi_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static QEMUOptionParameter vdi_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, +static QemuOptsList vdi_create_opts = { + .name = "vdi-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, #if defined(CONFIG_VDI_BLOCK_SIZE) - { - .name = BLOCK_OPT_CLUSTER_SIZE, - .type = OPT_SIZE, - .help = "VDI cluster (block) size", - .value = { .n = DEFAULT_CLUSTER_SIZE }, - }, + { + .name = BLOCK_OPT_CLUSTER_SIZE, + .type = QEMU_OPT_SIZE, + .help = "VDI cluster (block) size", + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) + }, #endif #if defined(CONFIG_VDI_STATIC_IMAGE) - { - .name = BLOCK_OPT_STATIC, - .type = OPT_FLAG, - .help = "VDI static (pre-allocated) image" - }, + { + .name = BLOCK_OPT_STATIC, + .type = QEMU_OPT_BOOL, + .help = "VDI static (pre-allocated) image" + }, #endif - /* TODO: An additional option to set UUID values might be useful. */ - { NULL } + /* TODO: An additional option to set UUID values might be useful. */ + { /* end of list */ } + } }; static BlockDriver bdrv_vdi = { @@ -776,7 +775,7 @@ static BlockDriver bdrv_vdi = { .bdrv_get_info = vdi_get_info, - .create_options = vdi_create_options, + .bdrv_create_options = &vdi_create_opts, .bdrv_check = vdi_check, }; diff --git a/block/vmdk.c b/block/vmdk.c index 19298c2..4c29927 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1437,7 +1437,7 @@ static int relative_path(char *dest, int dest_size, return 0; } -static int vmdk_create(const char *filename, QEMUOptionParameter *options) +static int vmdk_create(const char *filename, QemuOpts *opts) { int fd, idx = 0; char desc[BUF_SIZE]; @@ -1476,18 +1476,14 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { return -EINVAL; } - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) { - flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0; - } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) { - fmt = options->value.s; + /* Read out opts */ + if (opts) { + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE); + if (qemu_opt_get_bool(opts, BLOCK_OPT_COMPAT6, 0)) { + flags |= BLOCK_FLAG_COMPAT6; } - options++; + fmt = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); } if (!fmt) { /* Default format to monolithicSparse */ @@ -1654,30 +1650,34 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs) return ret; } -static QEMUOptionParameter vmdk_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { - .name = BLOCK_OPT_COMPAT6, - .type = OPT_FLAG, - .help = "VMDK version 6 image" - }, - { - .name = BLOCK_OPT_SUBFMT, - .type = OPT_STRING, - .help = - "VMDK flat extent format, can be one of " - "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " - }, - { NULL } +static QemuOptsList vmdk_create_opts = { + .name = "vmdk-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_COMPAT6, + .type = QEMU_OPT_BOOL, + .help = "VMDK version 6 image" + }, + { + .name = BLOCK_OPT_SUBFMT, + .type = QEMU_OPT_STRING, + .help = + "VMDK flat extent format, can be one of " + "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " + }, + { /* end of list */ } + } }; static BlockDriver bdrv_vmdk = { @@ -1694,7 +1694,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_co_is_allocated = vmdk_co_is_allocated, .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, - .create_options = vmdk_create_options, + .bdrv_create_options = &vmdk_create_opts, }; static void bdrv_vmdk_init(void) diff --git a/block/vpc.c b/block/vpc.c index 7948609..fcfdc79 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -665,34 +665,33 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size) return ret; } -static int vpc_create(const char *filename, QEMUOptionParameter *options) +static int vpc_create(const char *filename, QemuOpts *opts) { uint8_t buf[1024]; struct vhd_footer *footer = (struct vhd_footer *) buf; - QEMUOptionParameter *disk_type_param; + const char *disk_type_param = NULL; int fd, i; uint16_t cyls = 0; uint8_t heads = 0; uint8_t secs_per_cyl = 0; int64_t total_sectors; - int64_t total_size; - int disk_type; + int64_t total_size = 0; + int disk_type = VHD_DYNAMIC; int ret = -EIO; - /* Read out options */ - total_size = get_option_parameter(options, BLOCK_OPT_SIZE)->value.n; - - disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT); - if (disk_type_param && disk_type_param->value.s) { - if (!strcmp(disk_type_param->value.s, "dynamic")) { - disk_type = VHD_DYNAMIC; - } else if (!strcmp(disk_type_param->value.s, "fixed")) { - disk_type = VHD_FIXED; - } else { - return -EINVAL; + /* Read out opts */ + if (opts) { + total_size = qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0); + disk_type_param = qemu_opt_get(opts, BLOCK_OPT_SUBFMT); + if (disk_type_param) { + if (!strcmp(disk_type_param, "dynamic")) { + disk_type = VHD_DYNAMIC; + } else if (!strcmp(disk_type_param, "fixed")) { + disk_type = VHD_FIXED; + } else { + return -EINVAL; + } } - } else { - disk_type = VHD_DYNAMIC; } /* Create the file */ @@ -780,20 +779,24 @@ static void vpc_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static QEMUOptionParameter vpc_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_SUBFMT, - .type = OPT_STRING, - .help = - "Type of virtual hard disk format. Supported formats are " - "{dynamic (default) | fixed} " - }, - { NULL } +static QemuOptsList vpc_create_opts = { + .name = "vpc-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_NUMBER, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_SUBFMT, + .type = OPT_STRING, + .help = + "Type of virtual hard disk format. Supported formats are " + "{dynamic (default) | fixed} " + }, + { /* end of list */ } + } }; static BlockDriver bdrv_vpc = { @@ -809,7 +812,7 @@ static BlockDriver bdrv_vpc = { .bdrv_read = vpc_co_read, .bdrv_write = vpc_co_write, - .create_options = vpc_create_options, + .bdrv_create_options = &vpc_create_opts, }; static void bdrv_vpc_init(void) diff --git a/block/vvfat.c b/block/vvfat.c index 06e6654..2343cd4 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2802,7 +2802,7 @@ static BlockDriver vvfat_write_target = { static int enable_write_target(BDRVVVFATState *s) { BlockDriver *bdrv_qcow; - QEMUOptionParameter *options; + QemuOpts *opts; int ret; int size = sector2cluster(s, s->sector_count); s->used_clusters = calloc(size, 1); @@ -2818,12 +2818,13 @@ static int enable_write_target(BDRVVVFATState *s) } bdrv_qcow = bdrv_find_format("qcow"); - options = parse_option_parameters("", bdrv_qcow->create_options, NULL); - set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512); - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:"); + opts = qemu_opts_create_nofail(bdrv_qcow->bdrv_create_options); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); - if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0) + if (bdrv_create(bdrv_qcow, s->qcow_filename, opts) < 0) { return -1; + } s->qcow = bdrv_new(""); if (s->qcow == NULL) { diff --git a/include/block/block.h b/include/block/block.h index ffd1936..c99e8bf 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -124,8 +124,8 @@ BlockDriver *bdrv_find_protocol(const char *filename); BlockDriver *bdrv_find_format(const char *format_name); BlockDriver *bdrv_find_whitelisted_format(const char *format_name); int bdrv_create(BlockDriver *drv, const char* filename, - QEMUOptionParameter *options); -int bdrv_create_file(const char* filename, QEMUOptionParameter *options); + QemuOpts *options); +int bdrv_create_file(const char *filename, QemuOpts *options); 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 f83ffb8..71f7670 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -88,7 +88,7 @@ struct BlockDriver { const uint8_t *buf, int nb_sectors); void (*bdrv_close)(BlockDriverState *bs); void (*bdrv_rebind)(BlockDriverState *bs); - int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); + int (*bdrv_create)(const char *filename, QemuOpts *options); int (*bdrv_set_key)(BlockDriverState *bs, const char *key); int (*bdrv_make_empty)(BlockDriverState *bs); /* aio */ @@ -177,9 +177,7 @@ struct BlockDriver { unsigned long int req, void *buf, BlockDriverCompletionFunc *cb, void *opaque); - /* List of options for creating images, terminated by name == NULL */ - QEMUOptionParameter *create_options; - + QemuOptsList *bdrv_create_options; /* * Returns 0 for completed check, -errno for internal errors. diff --git a/qemu-img.c b/qemu-img.c index 85d3740..b4b3ce8 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -200,7 +200,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_options = NULL; /* Find driver and parse its options */ drv = bdrv_find_format(fmt); @@ -215,12 +215,10 @@ 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); - free_option_parameters(create_options); + create_options = append_opts_list(drv->bdrv_create_options, + proto_drv->bdrv_create_options); + print_opts_list(create_options); + free_opts_list(create_options); return 0; } @@ -271,19 +269,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 *list, 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(list, 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(list, BLOCK_OPT_BACKING_FMT, base_fmt)) { error_report("Backing file format not supported for file " "format '%s'", fmt); return -1; @@ -675,8 +673,9 @@ static int img_convert(int argc, char **argv) uint8_t * buf = NULL; const uint8_t *buf1; BlockDriverInfo bdi; - QEMUOptionParameter *param = NULL, *create_options = NULL; - QEMUOptionParameter *out_baseimg_param; + QemuOpts *param = NULL; + QemuOptsList *create_options = NULL; + const char *out_baseimg_param; char *options = NULL; const char *snapshot_name = NULL; float local_progress = 0; @@ -811,40 +810,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_options = append_opts_list(drv->bdrv_create_options, + proto_drv->bdrv_create_options); if (options) { - param = parse_option_parameters(options, create_options, param); - if (param == NULL) { + if (qemu_opts_do_parse(param, options, NULL) != 0) { error_report("Invalid options for file format '%s'.", out_fmt); ret = -1; goto out; } } else { - param = parse_option_parameters("", create_options, param); + param = qemu_opts_create_nofail(create_options); } - - set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512); + qemu_opt_set_number(param, BLOCK_OPT_SIZE, total_sectors * 512); ret = add_old_style_options(out_fmt, param, 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(param, 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(param, BLOCK_OPT_ENCRYPT, false); + const char *preallocation = + qemu_opt_get(param, BLOCK_OPT_PREALLOC); if (!drv->bdrv_write_compressed) { error_report("Compression not supported for this file format"); @@ -852,15 +847,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"); @@ -1078,8 +1073,10 @@ static int img_convert(int argc, char **argv) } out: qemu_progress_end(); - free_option_parameters(create_options); - free_option_parameters(param); + free_opts_list(create_options); + if (param) { + qemu_opts_del(param); + } qemu_vfree(buf); if (out_bs) { bdrv_delete(out_bs);
This patch will use QemuOpts related functions in block layer, add a member bdrv_create_options to BlockDriver struct, it will return a QemuOptsList pointer, which includes the image format's create options. And create options's primary consumer is block creating related functions, so modify them together. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v10->v11) 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or qemu_opts_print produce un-expanded cluster_size. 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, or while using protocol, there will be an error. v8->v9) 1) add qemu_ prefix to gluster_create_opts. 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be converted. v7->v8) 1) rebase to upstream source tree. 2) add gluster.c, raw-win32.c, and rbd.c. v6->v7: 1) use osdep.h:stringify(), not redefining new macro. 2) preserve TODO comment. 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. 4) initialize disk_type even when opts is NULL. v5->v6: 1) judge if opts == NULL in block layer create functions. 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. 3) made more readable while using qemu_opt_get_number. block.c | 91 ++++++++++++------------ block/cow.c | 46 ++++++------- block/gluster.c | 37 +++++----- block/qcow.c | 60 ++++++++-------- block/qcow2.c | 171 +++++++++++++++++++++++----------------------- block/qed.c | 86 +++++++++++------------ block/qed.h | 2 +- block/raw-posix.c | 59 ++++++++-------- block/raw-win32.c | 30 ++++---- block/raw.c | 30 ++++---- block/rbd.c | 62 ++++++++--------- block/sheepdog.c | 75 ++++++++++---------- block/vdi.c | 69 +++++++++---------- block/vmdk.c | 74 ++++++++++---------- block/vpc.c | 67 +++++++++--------- block/vvfat.c | 11 +-- include/block/block.h | 4 +- include/block/block_int.h | 6 +- qemu-img.c | 61 ++++++++--------- 19 files changed, 519 insertions(+), 522 deletions(-)