| Message ID | 1362043236-4635-4-git-send-email-wdongxu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 28, 2013 at 5:20 PM, Dong Xu Wang <wdongxu@linux.vnet.ibm.com> wrote: > This patch will use QemuOpts related functions in block layer, add > a member bdrv_create_opts 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> > --- > > v11->v12: > 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. > These functions works like origin code. > 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. > 3) in bdrv_create, if opts is NULL, will create an empty one, so can > discard if(opts) code safely. > > 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/iscsi.c | 8 +-- > block/qcow.c | 61 ++++++++-------- > block/qcow2.c | 173 +++++++++++++++++++++++----------------------- > block/qed.c | 86 +++++++++++------------ > block/qed.h | 2 +- > block/raw-posix.c | 59 +++++++--------- > block/raw-win32.c | 31 +++++---- > block/raw.c | 30 ++++---- > block/rbd.c | 62 ++++++++--------- > block/sheepdog.c | 75 ++++++++++---------- > block/vdi.c | 70 +++++++++---------- > block/vmdk.c | 90 ++++++++++++------------ > block/vpc.c | 57 +++++++-------- > block/vvfat.c | 11 +-- > include/block/block.h | 4 +- > include/block/block_int.h | 6 +- > include/qemu/option.h | 13 +++- > qemu-img.c | 61 ++++++++-------- > util/qemu-option.c | 93 +++++++++++++++++++++++-- > 22 files changed, 613 insertions(+), 553 deletions(-) > > diff --git a/block.c b/block.c > index 4582961..975c3d8 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 ?: qemu_opts_create_nofail(drv->bdrv_create_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); > } > > /* > @@ -814,7 +814,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 > @@ -847,17 +847,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_opts); > > - 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; > } > @@ -4502,8 +4501,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > char *options, uint64_t img_size, int flags, > Error **errp, bool quiet) > { > - QEMUOptionParameter *param = NULL, *create_options = NULL; > - QEMUOptionParameter *backing_fmt, *backing_file, *size; > + QemuOpts *opts = NULL; > + QemuOptsList *create_opts = NULL; > + const char *backing_fmt, *backing_file; > + int64_t size; > BlockDriverState *bs = NULL; > BlockDriver *drv, *proto_drv; > BlockDriver *backing_drv = NULL; > @@ -4521,28 +4522,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_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > /* Create parameter list with default values */ > - param = parse_option_parameters("", create_options, param); > + opts = qemu_opts_create_nofail(create_opts); > > - 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_replace(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_replace_set(opts, BLOCK_OPT_BACKING_FILE, > base_filename)) { > error_setg(errp, "Backing file not supported for file format '%s'", > fmt); > @@ -4551,39 +4547,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_replace_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_size(opts, BLOCK_OPT_SIZE, 0); > + if (size == -1) { > + if (backing_file) { > uint64_t size; > - char buf[32]; > int back_flags; > > /* backing files always opened read-only */ > @@ -4592,17 +4586,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; > @@ -4611,10 +4604,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > > if (!quiet) { > printf("Formatting '%s', fmt=%s ", filename, fmt); > - print_option_parameters(param); > + qemu_opts_print(opts, 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 " > @@ -4629,8 +4622,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > } > > out: > - free_option_parameters(create_options); > - free_option_parameters(param); > + qemu_opts_free(create_opts); > + if (opts) { > + qemu_opts_del(opts); > + } > > if (bs) { > bdrv_delete(bs); > diff --git a/block/cow.c b/block/cow.c > index 4baf904..135fa33 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,11 @@ 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 */ > + image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; > + image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -318,18 +312,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_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 +343,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_opts = &cow_create_opts, > }; > > static void bdrv_cow_init(void) > diff --git a/block/gluster.c b/block/gluster.c > index ccd684d..a219201 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,12 +349,8 @@ 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++; > - } > + total_size = > + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > > fd = glfs_creat(glfs, gconf->image, > O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR); > @@ -544,13 +539,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_SIZE, > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_gluster = { > @@ -565,7 +564,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_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_tcp = { > @@ -580,7 +579,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_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_unix = { > @@ -595,7 +594,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_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_rdma = { > @@ -610,7 +609,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_opts = &qemu_gluster_create_opts, > }; > > static void bdrv_gluster_init(void) > diff --git a/block/iscsi.c b/block/iscsi.c > index deb3b68..def377a 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -789,9 +789,7 @@ static char *parse_initiator_name(const char *target) > if (!opts) { > opts = QTAILQ_FIRST(&list->head); > } > - if (opts) { > - name = qemu_opt_get(opts, "initiator-name"); > - } > + name = qemu_opt_get(opts, "initiator-name"); > } > > if (name) { > @@ -1073,7 +1071,7 @@ out: > return ret; > } > > -static QEMUOptionParameter iscsi_create_options[] = { > +static QEMUOptionParameter iscsi_create_opts[] = { > { > .name = BLOCK_OPT_SIZE, > .type = OPT_SIZE, > @@ -1090,7 +1088,7 @@ static BlockDriver bdrv_iscsi = { > .bdrv_file_open = iscsi_open, > .bdrv_close = iscsi_close, > .bdrv_create = iscsi_create, > - .create_options = iscsi_create_options, > + .create_opts = iscsi_create_opts, > > .bdrv_getlength = iscsi_getlength, > > diff --git a/block/qcow.c b/block/qcow.c > index a7135ee..f0c4c38 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,14 @@ 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; > - } > - options++; > + /* Read out opts */ > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, 0)) { > + flags |= BLOCK_FLAG_ENCRYPT; > } > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -851,24 +846,28 @@ 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_SIZE, > + .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", > + .def_value_str = "off" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_qcow = { > @@ -889,7 +888,7 @@ static BlockDriver bdrv_qcow = { > .bdrv_write_compressed = qcow_write_compressed, > .bdrv_get_info = qcow_get_info, > > - .create_options = qcow_create_options, > + .bdrv_create_opts = &qcow_create_opts, > }; > > static void bdrv_qcow_init(void) > diff --git a/block/qcow2.c b/block/qcow2.c > index 7610e56..83288a8 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,41 @@ 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; > - } > - options++; > + sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, 0)) { > + flags |= BLOCK_FLAG_ENCRYPT; > + } > + cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, > + DEFAULT_CLUSTER_SIZE); > + buf = qemu_opt_get_del(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_del(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_del(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) { > + flags |= BLOCK_FLAG_LAZY_REFCOUNTS; > } > > if (backing_file && prealloc) { > @@ -1374,7 +1370,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 +1631,55 @@ 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_SIZE, > + .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", > + .def_value_str = "off" > + }, > + { > + .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", > + .def_value_str = "off" > + }, > + { /* 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_opts = &qcow2_create_opts, > }; > > static void bdrv_qcow2_init(void) > diff --git a/block/qed.c b/block/qed.c > index b8515e5..6032ea6 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,24 +611,14 @@ 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++; > - } > + image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); > + cluster_size = qemu_opt_get_size_del(opts, > + BLOCK_OPT_CLUSTER_SIZE, > + QED_DEFAULT_CLUSTER_SIZE); > + table_size = qemu_opt_get_size_del(opts, BLOCK_OPT_TABLE_SIZE, > + QED_DEFAULT_TABLE_SIZE); > > if (!qed_is_cluster_size_valid(cluster_size)) { > fprintf(stderr, "QED cluster size must be within range [%u, %u] and power of 2\n", > @@ -1537,36 +1527,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_SIZE, > + .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_opts = &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 4dfdf98..18e965f 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; > @@ -1006,19 +1019,14 @@ 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++; > - } > + total_size = > + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > > fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, > 0644); > @@ -1145,15 +1153,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", > @@ -1176,8 +1175,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_opts = &file_proto_create_opts, > }; > > /***********************************************/ > @@ -1459,20 +1457,15 @@ 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++; > - } > + total_size = > + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > > fd = qemu_open(filename, O_WRONLY | O_BINARY); > if (fd < 0) > @@ -1496,7 +1489,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, > @@ -1505,7 +1498,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, > @@ -1514,9 +1506,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_opts = &file_proto_create_opts, > > /* generic scsi device */ > #ifdef __linux__ > @@ -1630,7 +1623,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, > @@ -1646,6 +1638,7 @@ static BlockDriver bdrv_host_floppy = { > .bdrv_is_inserted = floppy_is_inserted, > .bdrv_media_changed = floppy_media_changed, > .bdrv_eject = floppy_eject, > + .bdrv_create_opts = &file_proto_create_opts, > }; > > static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) > @@ -1732,7 +1725,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, > @@ -1752,6 +1744,8 @@ static BlockDriver bdrv_host_cdrom = { > /* generic scsi device */ > .bdrv_ioctl = hdev_ioctl, > .bdrv_aio_ioctl = hdev_aio_ioctl, > + > + .bdrv_create_opts = &file_proto_create_opts, > }; > #endif /* __linux__ */ > > @@ -1854,7 +1848,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..d000f88 100644 > --- a/block/raw-win32.c > +++ b/block/raw-win32.c > @@ -382,18 +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++; > - } > + > + total_size = > + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; > > fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, > 0644); > @@ -405,13 +402,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_SIZE, > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_file = { > @@ -431,7 +432,7 @@ static BlockDriver bdrv_file = { > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > }; > > /***********************************************/ > diff --git a/block/raw.c b/block/raw.c > index 75812db..8cb26c2 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_SIZE, > + .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_opts = &raw_create_opts, > }; > > static void bdrv_raw_init(void) > diff --git a/block/rbd.c b/block/rbd.c > index 8cd10a7..2b56ed7 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,18 @@ 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; > - } > + bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + objsize = qemu_opt_get_size_del(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; > } > - options++; > + obj_order = ffs(objsize) - 1; > } > > clientname = qemu_rbd_parse_clientname(conf, clientname_buf); > @@ -920,20 +914,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_SIZE, > + .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 +939,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_opts = &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 d466b23..56397cc 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,18 @@ 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; > - } > - } > - options++; > + vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + buf = qemu_opt_get_del(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; > } > - > if (vdi_size > SD_MAX_VDI_SIZE) { > error_report("too big image size"); > ret = -EINVAL; > @@ -2043,24 +2035,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_SIZE, > + .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 = { > @@ -2085,7 +2080,7 @@ BlockDriver bdrv_sheepdog = { > .bdrv_save_vmstate = sd_save_vmstate, > .bdrv_load_vmstate = sd_load_vmstate, > > - .create_options = sd_create_options, > + .bdrv_create_opts = &sd_create_opts, > }; > > static void bdrv_sheepdog_init(void) > diff --git a/block/vdi.c b/block/vdi.c > index 87c691b..48b1bec 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -633,7 +633,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; > @@ -648,25 +648,18 @@ 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; > + bytes = qemu_opt_get_size_del(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_del(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 > - } > - options++; > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, 0)) { > + image_type = VDI_TYPE_STATIC; > } > +#endif > > fd = qemu_open(filename, > O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, > @@ -746,29 +739,34 @@ 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_SIZE, > + .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", > + .def_value_str = "off" > + }, > #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 = { > @@ -789,7 +787,7 @@ static BlockDriver bdrv_vdi = { > > .bdrv_get_info = vdi_get_info, > > - .create_options = vdi_create_options, > + .bdrv_create_opts = &vdi_create_opts, > .bdrv_check = vdi_check, > }; > > diff --git a/block/vmdk.c b/block/vmdk.c > index aef1abc..5428b1a 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1429,7 +1429,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]; > @@ -1470,21 +1470,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_ADAPTER_TYPE)) { > - adapter_type = options->value.s; > - } 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; > - } > - options++; > + /* Read out opts */ > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE); > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, 0)) { > + flags |= BLOCK_FLAG_COMPAT6; > } > + fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); > if (!adapter_type) { > adapter_type = "ide"; > } else if (strcmp(adapter_type, "ide") && > @@ -1665,36 +1658,41 @@ 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_ADAPTER_TYPE, > - .type = OPT_STRING, > - .help = "Virtual adapter type, can be one of " > - "ide (default), lsilogic, buslogic or legacyESX" > - }, > - { > - .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_SIZE, > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_ADAPTER_TYPE, > + .type = OPT_STRING, > + .help = "Virtual adapter type, can be one of " > + "ide (default), lsilogic, buslogic or legacyESX" > + }, > + { > + .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", > + .def_value_str = "off" > + }, > + { > + .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 = { > @@ -1711,7 +1709,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_opts = &vmdk_create_opts, > }; > > static void bdrv_vmdk_init(void) > diff --git a/block/vpc.c b/block/vpc.c > index 82229ef..e8c439f 100644 > --- a/block/vpc.c > +++ b/block/vpc.c > @@ -683,34 +683,31 @@ 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")) { > + /* Read out opts */ > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); > + if (disk_type_param) { > + if (!strcmp(disk_type_param, "dynamic")) { > disk_type = VHD_DYNAMIC; > - } else if (!strcmp(disk_type_param->value.s, "fixed")) { > + } else if (!strcmp(disk_type_param, "fixed")) { > disk_type = VHD_FIXED; > } else { > return -EINVAL; > } > - } else { > - disk_type = VHD_DYNAMIC; > } > > /* Create the file */ > @@ -798,20 +795,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_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} " > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vpc = { > @@ -827,7 +828,7 @@ static BlockDriver bdrv_vpc = { > .bdrv_read = vpc_co_read, > .bdrv_write = vpc_co_write, > > - .create_options = vpc_create_options, > + .bdrv_create_opts = &vpc_create_opts, > }; > > static void bdrv_vpc_init(void) > diff --git a/block/vvfat.c b/block/vvfat.c > index 06e6654..26ebe7c 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_opts); > + 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 0f750d7..a477b7a 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -126,8 +126,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 eaad53e..4f942ef 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -90,7 +90,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 */ > @@ -179,9 +179,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_opts; > > /* > * Returns 0 for completed check, -errno for internal errors. > diff --git a/include/qemu/option.h b/include/qemu/option.h > index c58db43..0d5fb66 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -108,6 +108,7 @@ struct QemuOptsList { > }; > > const char *qemu_opt_get(QemuOpts *opts, const char *name); > +const char *qemu_opt_get_del(QemuOpts *opts, const char *name); > /** > * qemu_opt_has_help_opt: > * @opts: options to search for a help request > @@ -121,9 +122,13 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name); > */ > bool qemu_opt_has_help_opt(QemuOpts *opts); > bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); > +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval); > uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval); > uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval); > +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, > + uint64_t defval); > int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); > +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value); > void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, > Error **errp); > int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); > @@ -144,7 +149,10 @@ const char *qemu_opts_id(QemuOpts *opts); > void qemu_opts_del(QemuOpts *opts); > void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); > int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); > -QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); > +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, > + const char *firstname); > +QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, > + int permit_abbrev); > void qemu_opts_set_defaults(QemuOptsList *list, const char *params, > int permit_abbrev); > QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, > @@ -156,8 +164,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, > int abort_on_failure); > > -QemuOptsList *qemu_opts_append(QemuOptsList *first, > - QemuOptsList *second); > +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); > void qemu_opts_free(QemuOptsList *list); > void qemu_opts_print_help(QemuOptsList *list); > #endif > diff --git a/qemu-img.c b/qemu-img.c > index 471de7d..9339957 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -229,7 +229,7 @@ static int read_password(char *buf, int buf_size) > static int print_block_option_help(const char *filename, const char *fmt) > { > BlockDriver *drv, *proto_drv; > - QEMUOptionParameter *create_options = NULL; > + QemuOptsList *create_opts = NULL; > > /* Find driver and parse its options */ > drv = bdrv_find_format(fmt); > @@ -244,12 +244,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_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > + qemu_opts_print_help(create_opts); > + qemu_opts_free(create_opts); > return 0; > } > > @@ -301,19 +299,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; > @@ -1120,8 +1118,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_opts = NULL; > + const char *out_baseimg_param; > char *options = NULL; > const char *snapshot_name = NULL; > float local_progress = 0; > @@ -1265,40 +1264,36 @@ static int img_convert(int argc, char **argv) > goto out; > } > > - create_options = append_option_parameters(create_options, > - drv->create_options); > - create_options = append_option_parameters(create_options, > - proto_drv->create_options); > + create_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > > if (options) { > - param = parse_option_parameters(options, create_options, param); > - if (param == NULL) { > + if (qemu_opts_do_parse_replace(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_opts); > } > - > - 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"); > @@ -1306,15 +1301,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"); > @@ -1532,8 +1527,10 @@ static int img_convert(int argc, char **argv) > } > out: > qemu_progress_end(); > - free_option_parameters(create_options); > - free_option_parameters(param); > + qemu_opts_free(create_opts); > + if (param) { > + qemu_opts_del(param); > + } > qemu_vfree(buf); > if (out_bs) { > bdrv_delete(out_bs); > diff --git a/util/qemu-option.c b/util/qemu-option.c > index 0e42452..dba82b4 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -33,6 +33,8 @@ > #include "qapi/qmp/qerror.h" > #include "qemu/option_int.h" > > +static void qemu_opt_del(QemuOpt *opt); > + > /* > * Extracts the name of an option from the parameter string (p points at the > * first byte of the option name) > @@ -531,6 +533,16 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) > return opt ? opt->str : NULL; > } > > +const char *qemu_opt_get_del(QemuOpts *opts, const char *name) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + const char *str = opt ? opt->str : NULL; > + if (opt) { > + qemu_opt_del(opt); > + } > + return str; > +} Sorry, should use g_strdup,please ignore the mistake and give other comments.Thanks. > + > bool qemu_opt_has_help_opt(QemuOpts *opts) > { > QemuOpt *opt; > @@ -553,6 +565,22 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) > return opt->value.boolean; > } > > +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + bool ret; > + > + if (opt == NULL) { > + return defval; > + } > + ret = opt->value.boolean; > + assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); > + if (opt) { > + qemu_opt_del(opt); > + } > + return ret; > +} > + > uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) > { > QemuOpt *opt = qemu_opt_find(opts, name); > @@ -573,6 +601,23 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) > return opt->value.uint; > } > > +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, > + uint64_t defval) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + uint64_t ret; > + > + if (opt == NULL) { > + return defval; > + } > + ret = opt->value.uint; > + assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); > + if (opt) { > + qemu_opt_del(opt); > + } > + return ret; > +} > + > static void qemu_opt_parse(QemuOpt *opt, Error **errp) > { > if (opt->desc == NULL) > @@ -624,7 +669,7 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, > } > > static void opt_set(QemuOpts *opts, const char *name, const char *value, > - bool prepend, Error **errp) > + bool prepend, bool replace, Error **errp) > { > QemuOpt *opt; > const QemuOptDesc *desc; > @@ -636,6 +681,11 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, > return; > } > > + opt = qemu_opt_find(opts, name); > + if (replace && opt) { > + qemu_opt_del(opt); > + } > + > opt = g_malloc0(sizeof(*opt)); > opt->name = g_strdup(name); > opt->opts = opts; > @@ -658,7 +708,29 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) > { > Error *local_err = NULL; > > - opt_set(opts, name, value, false, &local_err); > + opt_set(opts, name, value, false, false, &local_err); > + if (error_is_set(&local_err)) { > + qerror_report_err(local_err); > + error_free(local_err); > + return -1; > + } > + > + return 0; > +} > + > +/* > + * If name exists, the function will delete the opt first and then add a new > + * one. > + */ > +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value) > +{ > + Error *local_err = NULL; > + QemuOpt *opt = qemu_opt_find(opts, name); > + > + if (opt) { > + qemu_opt_del(opt); > + } > + opt_set(opts, name, value, false, true, &local_err); > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > @@ -671,7 +743,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) > void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, > Error **errp) > { > - opt_set(opts, name, value, false, errp); > + opt_set(opts, name, value, false, false, errp); > } > > int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) > @@ -895,7 +967,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy) > } > > static int opts_do_parse(QemuOpts *opts, const char *params, > - const char *firstname, bool prepend) > + const char *firstname, bool prepend, bool replace) > { > char option[128], value[1024]; > const char *p,*pe,*pc; > @@ -931,7 +1003,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params, > } > if (strcmp(option, "id") != 0) { > /* store and parse */ > - opt_set(opts, option, value, prepend, &local_err); > + opt_set(opts, option, value, prepend, replace, &local_err); > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > @@ -947,9 +1019,16 @@ static int opts_do_parse(QemuOpts *opts, const char *params, > > int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) > { > - return opts_do_parse(opts, params, firstname, false); > + return opts_do_parse(opts, params, firstname, false, false); > +} > + > +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, > + const char *firstname) > +{ > + return opts_do_parse(opts, params, firstname, false, true); > } > > + > static QemuOpts *opts_parse(QemuOptsList *list, const char *params, > int permit_abbrev, bool defaults) > { > @@ -986,7 +1065,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, > return NULL; > } > > - if (opts_do_parse(opts, params, firstname, defaults) != 0) { > + if (opts_do_parse(opts, params, firstname, defaults, false) != 0) { > qemu_opts_del(opts); > return NULL; > } > -- > 1.7.11.7 > >
Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > This patch will use QemuOpts related functions in block layer, add > a member bdrv_create_opts 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> > --- > > v11->v12: > 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. > These functions works like origin code. > 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. > 3) in bdrv_create, if opts is NULL, will create an empty one, so can > discard if(opts) code safely. > > 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/iscsi.c | 8 +-- > block/qcow.c | 61 ++++++++-------- > block/qcow2.c | 173 +++++++++++++++++++++++----------------------- > block/qed.c | 86 +++++++++++------------ > block/qed.h | 2 +- > block/raw-posix.c | 59 +++++++--------- > block/raw-win32.c | 31 +++++---- > block/raw.c | 30 ++++---- > block/rbd.c | 62 ++++++++--------- > block/sheepdog.c | 75 ++++++++++---------- > block/vdi.c | 70 +++++++++---------- > block/vmdk.c | 90 ++++++++++++------------ > block/vpc.c | 57 +++++++-------- > block/vvfat.c | 11 +-- > include/block/block.h | 4 +- > include/block/block_int.h | 6 +- > include/qemu/option.h | 13 +++- > qemu-img.c | 61 ++++++++-------- > util/qemu-option.c | 93 +++++++++++++++++++++++-- > 22 files changed, 613 insertions(+), 553 deletions(-) *Ouch* Any chance to split this patch up some? Its size makes it really hard to review... > diff --git a/block.c b/block.c > index 4582961..975c3d8 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) Since you touch this anyway, consider unbreaking the line: int bdrv_create(BlockDriver *drv, const char* filename, 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 ?: qemu_opts_create_nofail(drv->bdrv_create_opts), > .ret = NOT_DONE, > }; > As discussed during review of v11, this avoids passing null opts to the bdrv_create() method. Good. > @@ -405,7 +405,7 @@ out: out: g_free(cco.filename); > return ret; > } I suspect you need if (!opts) { qemu_opts_del(cco->opts); } to avoid leaking the empty cco->opts you create in the previous hunk. > > -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); > } > > /* > @@ -814,7 +814,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 > @@ -847,17 +847,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_opts); > > - 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; > } > @@ -4502,8 +4501,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > char *options, uint64_t img_size, int flags, > Error **errp, bool quiet) > { > - QEMUOptionParameter *param = NULL, *create_options = NULL; > - QEMUOptionParameter *backing_fmt, *backing_file, *size; > + QemuOpts *opts = NULL; > + QemuOptsList *create_opts = NULL; > + const char *backing_fmt, *backing_file; > + int64_t size; > BlockDriverState *bs = NULL; > BlockDriver *drv, *proto_drv; > BlockDriver *backing_drv = NULL; > @@ -4521,28 +4522,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_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); qemu_opts_append() dereferences its arguments to compute dest->merge_lists = first->merge_lists || second->merge_lists; However, either of drv->create_opts and proto_drv->create_opts may be null, as far as I can see. If I'm correct, you need a few more test cases :) 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? Beyond the scope of this patch. > /* Create parameter list with default values */ > - param = parse_option_parameters("", create_options, param); > + opts = qemu_opts_create_nofail(create_opts); Before: param empty. After: opts empty. Good. > > - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); Before: param contains exactly BLOCK_OPT_SIZE = img_size. After: opts contains exactly BLOCK_OPT_SIZE = img_size. Both old and new code seem to assume BLOCK_OPT_SIZE is always a valid option. Beyond the scope of this patch. Good. > > /* Parse -o options */ > if (options) { > - param = parse_option_parameters(options, create_options, param); > - if (param == NULL) { > + if (qemu_opts_do_parse_replace(opts, options, NULL) != 0) { > error_setg(errp, "Invalid options for file format '%s'.", fmt); > goto out; > } > } Before: values from -o replace whatever is in param already. After: values from -o replace whatever is in opts already. In particular, size from -o replaces img_size before and after. Did you test it does? $ qemu-img create -f raw -o size=1024 t.img 512 Formatting 't.img', fmt=raw size=1024 $ ls -l t.img -rw-r--r--. 1 armbru armbru 1024 Mar 21 15:12 t.img > > if (base_filename) { > - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, > + if (qemu_opt_replace_set(opts, BLOCK_OPT_BACKING_FILE, > base_filename)) { > error_setg(errp, "Backing file not supported for file format '%s'", > fmt); Before: base_filename replaces any backing_file from -o in param. After: base_filename replaces any backing_file from -o in opts. Did you test it does? $ qemu-img create -f qcow2 -b t.img -o backing_file=/dev/null t.qcow2 Formatting 't.qcow2', fmt=qcow2 size=1024 backing_file='t.img' encryption=off cluster_size=65536 lazy_refcounts=off $ qemu-img info t.qcow2image: t.qcow2 file format: qcow2 virtual size: 1.0K (1024 bytes) disk size: 196K cluster_size: 65536 backing file: t.img > @@ -4551,39 +4547,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_replace_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) { > error_setg(errp, "Backing file format not supported for file " > "format '%s'", fmt); > goto out; > } > } Likewise. Did you test? $ qemu-img create -f qcow2 -b t.img -F file -o backing_file=/dev/null,backing_fmt=host_device t.qcow2 Formatting 't.qcow2', fmt=qcow2 size=1024 backing_file='t.img' backing_fmt='file' encryption=off cluster_size=65536 lazy_refcounts=off $ qemu-img info t.qcow2image: t.qcow2 file format: qcow2 virtual size: 1.0K (1024 bytes) disk size: 196K cluster_size: 65536 backing file: t.img backing file format: file > > - 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_size(opts, BLOCK_OPT_SIZE, 0); > + if (size == -1) { > + if (backing_file) { > uint64_t size; > - char buf[32]; > int back_flags; > > /* backing files always opened read-only */ > @@ -4592,17 +4586,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); We get here only when opts doesn't have BLOCK_OPT_SIZE. Good. > } else { > error_setg(errp, "Image creation needs a size parameter"); > goto out; > @@ -4611,10 +4604,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > > if (!quiet) { > printf("Formatting '%s', fmt=%s ", filename, fmt); > - print_option_parameters(param); > + qemu_opts_print(opts, NULL); > puts(""); > } Before: param[] contains all valid options. If the option in param[i] hasn't been set, param[i].value still has the default value defined in the .create_options[] where we got this option. print_option_parameters() iterates over all valid options, and prints NAME=VALUE. Except it prints nothing for an option of type OPT_STRING when the value is null. That's the case when the default is null and the option hasn't been set. After: opts contains only the options that have been set. opts->list->desc[] contains all valid options (or is empty, which means "accept all", but that shouldn't happen here). qemu_opts_print() in master prints only the options that have been set. Differs from print_option_parameters() for unset options. That's why you rewrite qemu_opts_print() in PATCH 1/4. The rewritten qemu_opts_print() prints all all valid options, except unset ones that have a null def_value_str. Therefore: * OPT_STRING parameters need to become QEMU_OPT_STRING options, and their default value needs to go both into def_value_str *and* into every caller's handling of qemu_opt_get() returning null. * OPT_FLAG, OPT_NUMBER, OPT_STRING parameters need to become QEMU_OPT_BOOL, QEMU_OPT_NUMBER, QEMU_OPT_SIZE options, and their default value needs to go both into def_value_str *and* into the last argument of every qemu_opt_bool() getting it. I hate how the default value needs to go in multiple places now. More on that in my forthcoming review of PATCH 1/4. > - 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 " > @@ -4629,8 +4622,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > } > > out: > - free_option_parameters(create_options); > - free_option_parameters(param); > + qemu_opts_free(create_opts); > + if (opts) { > + qemu_opts_del(opts); > + } > > if (bs) { > bdrv_delete(bs); > diff --git a/block/cow.c b/block/cow.c > index 4baf904..135fa33 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,11 @@ 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 */ > + image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; > + image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); Why _del? > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > return ret; > } > @@ -318,18 +312,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_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 +343,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_opts = &cow_create_opts, > }; > > static void bdrv_cow_init(void) [Boatload of drivers snipped; not reviewed in this round] > diff --git a/include/block/block.h b/include/block/block.h > index 0f750d7..a477b7a 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -126,8 +126,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); QemuOpts *opts > 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 eaad53e..4f942ef 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -90,7 +90,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); QemuOpts *opts > int (*bdrv_set_key)(BlockDriverState *bs, const char *key); > int (*bdrv_make_empty)(BlockDriverState *bs); > /* aio */ > @@ -179,9 +179,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_opts; > > /* > * Returns 0 for completed check, -errno for internal errors. > diff --git a/include/qemu/option.h b/include/qemu/option.h > index c58db43..0d5fb66 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -108,6 +108,7 @@ struct QemuOptsList { > }; > > const char *qemu_opt_get(QemuOpts *opts, const char *name); > +const char *qemu_opt_get_del(QemuOpts *opts, const char *name); > /** > * qemu_opt_has_help_opt: > * @opts: options to search for a help request > @@ -121,9 +122,13 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name); > */ > bool qemu_opt_has_help_opt(QemuOpts *opts); > bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); > +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval); > uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval); > uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval); > +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, > + uint64_t defval); > int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); > +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value); > void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, > Error **errp); > int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); > @@ -144,7 +149,10 @@ const char *qemu_opts_id(QemuOpts *opts); > void qemu_opts_del(QemuOpts *opts); > void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); > int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); > -QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); > +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, > + const char *firstname); > +QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, > + int permit_abbrev); > void qemu_opts_set_defaults(QemuOptsList *list, const char *params, > int permit_abbrev); > QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, Putting these new qemu_opt_*() functions in their own patch would reduce this patch's size a little. > @@ -156,8 +164,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, > int abort_on_failure); > > -QemuOptsList *qemu_opts_append(QemuOptsList *first, > - QemuOptsList *second); > +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); > void qemu_opts_free(QemuOptsList *list); > void qemu_opts_print_help(QemuOptsList *list); > #endif > diff --git a/qemu-img.c b/qemu-img.c > index 471de7d..9339957 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -229,7 +229,7 @@ static int read_password(char *buf, int buf_size) > static int print_block_option_help(const char *filename, const char *fmt) > { > BlockDriver *drv, *proto_drv; > - QEMUOptionParameter *create_options = NULL; > + QemuOptsList *create_opts = NULL; > > /* Find driver and parse its options */ > drv = bdrv_find_format(fmt); > @@ -244,12 +244,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_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > + qemu_opts_print_help(create_opts); > + qemu_opts_free(create_opts); > return 0; > } > Must merge options exactly like bdrv_img_create(). It does. Good. I suspect the merge code should be factored out. Beyond the scope of this patch. Running out of steam, review is becoming more superficial and less reliable. > @@ -301,19 +299,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; Looks like this duplicates code in bdrv_img_create(). Cleanup beyond the scope of this patch. > @@ -1120,8 +1118,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_opts = NULL; > + const char *out_baseimg_param; > char *options = NULL; > const char *snapshot_name = NULL; > float local_progress = 0; > @@ -1265,40 +1264,36 @@ static int img_convert(int argc, char **argv) > goto out; > } > > - create_options = append_option_parameters(create_options, > - drv->create_options); > - create_options = append_option_parameters(create_options, > - proto_drv->create_options); > + create_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > More duplicated option merge code. Cleanup beyond the scope of this patch. > if (options) { > - param = parse_option_parameters(options, create_options, param); > - if (param == NULL) { > + if (qemu_opts_do_parse_replace(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_opts); > } > - > - 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; > } More copy/paste coding. Cleanup beyond the scope of this patch. > > /* 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"); > @@ -1306,15 +1301,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"); > @@ -1532,8 +1527,10 @@ static int img_convert(int argc, char **argv) > } > out: > qemu_progress_end(); > - free_option_parameters(create_options); > - free_option_parameters(param); > + qemu_opts_free(create_opts); > + if (param) { > + qemu_opts_del(param); > + } > qemu_vfree(buf); > if (out_bs) { > bdrv_delete(out_bs); > diff --git a/util/qemu-option.c b/util/qemu-option.c > index 0e42452..dba82b4 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c Changes to this file not reviewed in this round. > @@ -33,6 +33,8 @@ > #include "qapi/qmp/qerror.h" > #include "qemu/option_int.h" > > +static void qemu_opt_del(QemuOpt *opt); > + > /* > * Extracts the name of an option from the parameter string (p points at the > * first byte of the option name) > @@ -531,6 +533,16 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) > return opt ? opt->str : NULL; > } > > +const char *qemu_opt_get_del(QemuOpts *opts, const char *name) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + const char *str = opt ? opt->str : NULL; > + if (opt) { > + qemu_opt_del(opt); > + } > + return str; > +} > + > bool qemu_opt_has_help_opt(QemuOpts *opts) > { > QemuOpt *opt; > @@ -553,6 +565,22 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) > return opt->value.boolean; > } > > +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + bool ret; > + > + if (opt == NULL) { > + return defval; > + } > + ret = opt->value.boolean; > + assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); > + if (opt) { > + qemu_opt_del(opt); > + } > + return ret; > +} > + > uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) > { > QemuOpt *opt = qemu_opt_find(opts, name); > @@ -573,6 +601,23 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) > return opt->value.uint; > } > > +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, > + uint64_t defval) > +{ > + QemuOpt *opt = qemu_opt_find(opts, name); > + uint64_t ret; > + > + if (opt == NULL) { > + return defval; > + } > + ret = opt->value.uint; > + assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); > + if (opt) { > + qemu_opt_del(opt); > + } > + return ret; > +} > + > static void qemu_opt_parse(QemuOpt *opt, Error **errp) > { > if (opt->desc == NULL) > @@ -624,7 +669,7 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, > } > > static void opt_set(QemuOpts *opts, const char *name, const char *value, > - bool prepend, Error **errp) > + bool prepend, bool replace, Error **errp) > { > QemuOpt *opt; > const QemuOptDesc *desc; > @@ -636,6 +681,11 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, > return; > } > > + opt = qemu_opt_find(opts, name); > + if (replace && opt) { > + qemu_opt_del(opt); > + } > + > opt = g_malloc0(sizeof(*opt)); > opt->name = g_strdup(name); > opt->opts = opts; > @@ -658,7 +708,29 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) > { > Error *local_err = NULL; > > - opt_set(opts, name, value, false, &local_err); > + opt_set(opts, name, value, false, false, &local_err); > + if (error_is_set(&local_err)) { > + qerror_report_err(local_err); > + error_free(local_err); > + return -1; > + } > + > + return 0; > +} > + > +/* > + * If name exists, the function will delete the opt first and then add a new > + * one. > + */ > +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value) > +{ > + Error *local_err = NULL; > + QemuOpt *opt = qemu_opt_find(opts, name); > + > + if (opt) { > + qemu_opt_del(opt); > + } > + opt_set(opts, name, value, false, true, &local_err); > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > @@ -671,7 +743,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) > void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, > Error **errp) > { > - opt_set(opts, name, value, false, errp); > + opt_set(opts, name, value, false, false, errp); > } > > int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) > @@ -895,7 +967,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy) > } > > static int opts_do_parse(QemuOpts *opts, const char *params, > - const char *firstname, bool prepend) > + const char *firstname, bool prepend, bool replace) > { > char option[128], value[1024]; > const char *p,*pe,*pc; > @@ -931,7 +1003,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params, > } > if (strcmp(option, "id") != 0) { > /* store and parse */ > - opt_set(opts, option, value, prepend, &local_err); > + opt_set(opts, option, value, prepend, replace, &local_err); > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > @@ -947,9 +1019,16 @@ static int opts_do_parse(QemuOpts *opts, const char *params, > > int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) > { > - return opts_do_parse(opts, params, firstname, false); > + return opts_do_parse(opts, params, firstname, false, false); > +} > + > +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, > + const char *firstname) > +{ > + return opts_do_parse(opts, params, firstname, false, true); > } > > + > static QemuOpts *opts_parse(QemuOptsList *list, const char *params, > int permit_abbrev, bool defaults) > { > @@ -986,7 +1065,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, > return NULL; > } > > - if (opts_do_parse(opts, params, firstname, defaults) != 0) { > + if (opts_do_parse(opts, params, firstname, defaults, false) != 0) { > qemu_opts_del(opts); > return NULL; > }
On Thu, Mar 21, 2013 at 11:31 PM, Markus Armbruster <armbru@redhat.com> wrote: > Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > >> This patch will use QemuOpts related functions in block layer, add >> a member bdrv_create_opts 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> >> --- >> >> v11->v12: >> 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. >> These functions works like origin code. >> 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. >> 3) in bdrv_create, if opts is NULL, will create an empty one, so can >> discard if(opts) code safely. >> >> 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/iscsi.c | 8 +-- >> block/qcow.c | 61 ++++++++-------- >> block/qcow2.c | 173 +++++++++++++++++++++++----------------------- >> block/qed.c | 86 +++++++++++------------ >> block/qed.h | 2 +- >> block/raw-posix.c | 59 +++++++--------- >> block/raw-win32.c | 31 +++++---- >> block/raw.c | 30 ++++---- >> block/rbd.c | 62 ++++++++--------- >> block/sheepdog.c | 75 ++++++++++---------- >> block/vdi.c | 70 +++++++++---------- >> block/vmdk.c | 90 ++++++++++++------------ >> block/vpc.c | 57 +++++++-------- >> block/vvfat.c | 11 +-- >> include/block/block.h | 4 +- >> include/block/block_int.h | 6 +- >> include/qemu/option.h | 13 +++- >> qemu-img.c | 61 ++++++++-------- >> util/qemu-option.c | 93 +++++++++++++++++++++++-- >> 22 files changed, 613 insertions(+), 553 deletions(-) > > *Ouch* > > Any chance to split this patch up some? Its size makes it really hard > to review... > I will split this patch into some small patches in next version. >> diff --git a/block.c b/block.c >> index 4582961..975c3d8 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) > > Since you touch this anyway, consider unbreaking the line: > > int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts) > Okay. >> { >> 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 ?: qemu_opts_create_nofail(drv->bdrv_create_opts), >> .ret = NOT_DONE, >> }; >> > > As discussed during review of v11, this avoids passing null opts to the > bdrv_create() method. Good. > >> @@ -405,7 +405,7 @@ out: > out: > g_free(cco.filename); >> return ret; >> } > > I suspect you need > > if (!opts) { > qemu_opts_del(cco->opts); > } > > to avoid leaking the empty cco->opts you create in the previous hunk. > Okay. >> >> -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); >> } >> >> /* >> @@ -814,7 +814,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 >> @@ -847,17 +847,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_opts); >> >> - 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; >> } >> @@ -4502,8 +4501,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> char *options, uint64_t img_size, int flags, >> Error **errp, bool quiet) >> { >> - QEMUOptionParameter *param = NULL, *create_options = NULL; >> - QEMUOptionParameter *backing_fmt, *backing_file, *size; >> + QemuOpts *opts = NULL; >> + QemuOptsList *create_opts = NULL; >> + const char *backing_fmt, *backing_file; >> + int64_t size; >> BlockDriverState *bs = NULL; >> BlockDriver *drv, *proto_drv; >> BlockDriver *backing_drv = NULL; >> @@ -4521,28 +4522,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_opts = qemu_opts_append(drv->bdrv_create_opts, >> + proto_drv->bdrv_create_opts); > > qemu_opts_append() dereferences its arguments to compute > > dest->merge_lists = first->merge_lists || second->merge_lists; > > However, either of drv->create_opts and proto_drv->create_opts may be > null, as far as I can see. If I'm correct, you need a few more test > cases :) > Okay, will check first and second. > 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? Beyond the > scope of this patch. > >> /* Create parameter list with default values */ >> - param = parse_option_parameters("", create_options, param); >> + opts = qemu_opts_create_nofail(create_opts); > > Before: param empty. > > After: opts empty. > > Good. > >> >> - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); > > Before: param contains exactly BLOCK_OPT_SIZE = img_size. > > After: opts contains exactly BLOCK_OPT_SIZE = img_size. > > Both old and new code seem to assume BLOCK_OPT_SIZE is always a valid > option. Beyond the scope of this patch. > > Good. > >> >> /* Parse -o options */ >> if (options) { >> - param = parse_option_parameters(options, create_options, param); >> - if (param == NULL) { >> + if (qemu_opts_do_parse_replace(opts, options, NULL) != 0) { >> error_setg(errp, "Invalid options for file format '%s'.", fmt); >> goto out; >> } >> } > > Before: values from -o replace whatever is in param already. > > After: values from -o replace whatever is in opts already. > > In particular, size from -o replaces img_size before and after. > > Did you test it does? > > $ qemu-img create -f raw -o size=1024 t.img 512 > Formatting 't.img', fmt=raw size=1024 > $ ls -l t.img > -rw-r--r--. 1 armbru armbru 1024 Mar 21 15:12 t.img > >> >> if (base_filename) { >> - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, >> + if (qemu_opt_replace_set(opts, BLOCK_OPT_BACKING_FILE, >> base_filename)) { >> error_setg(errp, "Backing file not supported for file format '%s'", >> fmt); > > Before: base_filename replaces any backing_file from -o in param. > > After: base_filename replaces any backing_file from -o in opts. > > Did you test it does? > Oh, I did not test command line like this, my patch will fail here. Will fix in next version. > $ qemu-img create -f qcow2 -b t.img -o backing_file=/dev/null t.qcow2 > Formatting 't.qcow2', fmt=qcow2 size=1024 backing_file='t.img' encryption=off cluster_size=65536 lazy_refcounts=off > $ qemu-img info t.qcow2image: t.qcow2 > file format: qcow2 > virtual size: 1.0K (1024 bytes) > disk size: 196K > cluster_size: 65536 > backing file: t.img > I run this command using upstream QEMU code, but failed. [wdongxu@13@VM:~]$ qemu-img info t.qcow2image: t.qcow2 qemu-img: Could not open 't.qcow2image:': No such file or directory Or did I type something wrong? >> @@ -4551,39 +4547,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_replace_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) { >> error_setg(errp, "Backing file format not supported for file " >> "format '%s'", fmt); >> goto out; >> } >> } > > Likewise. > > Did you test? > > $ qemu-img create -f qcow2 -b t.img -F file -o backing_file=/dev/null,backing_fmt=host_device t.qcow2 > Formatting 't.qcow2', fmt=qcow2 size=1024 backing_file='t.img' backing_fmt='file' encryption=off cluster_size=65536 lazy_refcounts=off > $ qemu-img info t.qcow2image: t.qcow2 > file format: qcow2 > virtual size: 1.0K (1024 bytes) > disk size: 196K > cluster_size: 65536 > backing file: t.img > backing file format: file > >> >> - 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_size(opts, BLOCK_OPT_SIZE, 0); >> + if (size == -1) { >> + if (backing_file) { >> uint64_t size; >> - char buf[32]; >> int back_flags; >> >> /* backing files always opened read-only */ >> @@ -4592,17 +4586,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); > > We get here only when opts doesn't have BLOCK_OPT_SIZE. > > Good. > >> } else { >> error_setg(errp, "Image creation needs a size parameter"); >> goto out; >> @@ -4611,10 +4604,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> >> if (!quiet) { >> printf("Formatting '%s', fmt=%s ", filename, fmt); >> - print_option_parameters(param); >> + qemu_opts_print(opts, NULL); >> puts(""); >> } > > Before: > > param[] contains all valid options. If the option in param[i] hasn't > been set, param[i].value still has the default value defined in the > .create_options[] where we got this option. > > print_option_parameters() iterates over all valid options, and prints > NAME=VALUE. Except it prints nothing for an option of type OPT_STRING > when the value is null. That's the case when the default is null and > the option hasn't been set. > > After: > > opts contains only the options that have been set. opts->list->desc[] > contains all valid options (or is empty, which means "accept all", but > that shouldn't happen here). > > qemu_opts_print() in master prints only the options that have been set. > Differs from print_option_parameters() for unset options. That's why > you rewrite qemu_opts_print() in PATCH 1/4. > > The rewritten qemu_opts_print() prints all all valid options, except > unset ones that have a null def_value_str. > > Therefore: > > * OPT_STRING parameters need to become QEMU_OPT_STRING options, and > their default value needs to go both into def_value_str *and* into > every caller's handling of qemu_opt_get() returning null. > > * OPT_FLAG, OPT_NUMBER, OPT_STRING parameters need to become > QEMU_OPT_BOOL, QEMU_OPT_NUMBER, QEMU_OPT_SIZE options, and their > default value needs to go both into def_value_str *and* into the last > argument of every qemu_opt_bool() getting it. > > I hate how the default value needs to go in multiple places now. More > on that in my forthcoming review of PATCH 1/4. > >> - 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 " >> @@ -4629,8 +4622,10 @@ void bdrv_img_create(const char *filename, const char *fmt, >> } >> >> out: >> - free_option_parameters(create_options); >> - free_option_parameters(param); >> + qemu_opts_free(create_opts); >> + if (opts) { >> + qemu_opts_del(opts); >> + } >> >> if (bs) { >> bdrv_delete(bs); >> diff --git a/block/cow.c b/block/cow.c >> index 4baf904..135fa33 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,11 @@ 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 */ >> + image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; >> + image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > > Why _del? > Before: after getting one parameter, then option++, so in my patches, I want to delete the opt after using it. If I did not delete it, some parameters will be passed to "protocol", it will go wrong. Do you think it is acceptable? >> >> - ret = bdrv_create_file(filename, options); >> + ret = bdrv_create_file(filename, opts); >> if (ret < 0) { >> return ret; >> } >> @@ -318,18 +312,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_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 +343,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_opts = &cow_create_opts, >> }; >> >> static void bdrv_cow_init(void) > [Boatload of drivers snipped; not reviewed in this round] >> diff --git a/include/block/block.h b/include/block/block.h >> index 0f750d7..a477b7a 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -126,8 +126,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); > > QemuOpts *opts > Okay. >> 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 eaad53e..4f942ef 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -90,7 +90,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); > > QemuOpts *opts > >> int (*bdrv_set_key)(BlockDriverState *bs, const char *key); >> int (*bdrv_make_empty)(BlockDriverState *bs); >> /* aio */ >> @@ -179,9 +179,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_opts; >> >> /* >> * Returns 0 for completed check, -errno for internal errors. >> diff --git a/include/qemu/option.h b/include/qemu/option.h >> index c58db43..0d5fb66 100644 >> --- a/include/qemu/option.h >> +++ b/include/qemu/option.h >> @@ -108,6 +108,7 @@ struct QemuOptsList { >> }; >> >> const char *qemu_opt_get(QemuOpts *opts, const char *name); >> +const char *qemu_opt_get_del(QemuOpts *opts, const char *name); >> /** >> * qemu_opt_has_help_opt: >> * @opts: options to search for a help request >> @@ -121,9 +122,13 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name); >> */ >> bool qemu_opt_has_help_opt(QemuOpts *opts); >> bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); >> +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval); >> uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval); >> uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval); >> +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, >> + uint64_t defval); >> int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); >> +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value); >> void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, >> Error **errp); >> int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); >> @@ -144,7 +149,10 @@ const char *qemu_opts_id(QemuOpts *opts); >> void qemu_opts_del(QemuOpts *opts); >> void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); >> int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); >> -QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); >> +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, >> + const char *firstname); >> +QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, >> + int permit_abbrev); >> void qemu_opts_set_defaults(QemuOptsList *list, const char *params, >> int permit_abbrev); >> QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, > > Putting these new qemu_opt_*() functions in their own patch would reduce > this patch's size a little. > >> @@ -156,8 +164,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); >> int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >> int abort_on_failure); >> >> -QemuOptsList *qemu_opts_append(QemuOptsList *first, >> - QemuOptsList *second); >> +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); >> void qemu_opts_free(QemuOptsList *list); >> void qemu_opts_print_help(QemuOptsList *list); >> #endif >> diff --git a/qemu-img.c b/qemu-img.c >> index 471de7d..9339957 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -229,7 +229,7 @@ static int read_password(char *buf, int buf_size) >> static int print_block_option_help(const char *filename, const char *fmt) >> { >> BlockDriver *drv, *proto_drv; >> - QEMUOptionParameter *create_options = NULL; >> + QemuOptsList *create_opts = NULL; >> >> /* Find driver and parse its options */ >> drv = bdrv_find_format(fmt); >> @@ -244,12 +244,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_opts = qemu_opts_append(drv->bdrv_create_opts, >> + proto_drv->bdrv_create_opts); >> + qemu_opts_print_help(create_opts); >> + qemu_opts_free(create_opts); >> return 0; >> } >> > > Must merge options exactly like bdrv_img_create(). It does. Good. > > I suspect the merge code should be factored out. Beyond the scope of > this patch. > > Running out of steam, review is becoming more superficial and less > reliable. > >> @@ -301,19 +299,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; > > Looks like this duplicates code in bdrv_img_create(). Cleanup beyond > the scope of this patch. > >> @@ -1120,8 +1118,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_opts = NULL; >> + const char *out_baseimg_param; >> char *options = NULL; >> const char *snapshot_name = NULL; >> float local_progress = 0; >> @@ -1265,40 +1264,36 @@ static int img_convert(int argc, char **argv) >> goto out; >> } >> >> - create_options = append_option_parameters(create_options, >> - drv->create_options); >> - create_options = append_option_parameters(create_options, >> - proto_drv->create_options); >> + create_opts = qemu_opts_append(drv->bdrv_create_opts, >> + proto_drv->bdrv_create_opts); >> > > More duplicated option merge code. Cleanup beyond the scope of this > patch. > >> if (options) { >> - param = parse_option_parameters(options, create_options, param); >> - if (param == NULL) { >> + if (qemu_opts_do_parse_replace(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_opts); >> } >> - >> - 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; >> } > > More copy/paste coding. Cleanup beyond the scope of this patch. > >> >> /* 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"); >> @@ -1306,15 +1301,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"); >> @@ -1532,8 +1527,10 @@ static int img_convert(int argc, char **argv) >> } >> out: >> qemu_progress_end(); >> - free_option_parameters(create_options); >> - free_option_parameters(param); >> + qemu_opts_free(create_opts); >> + if (param) { >> + qemu_opts_del(param); >> + } >> qemu_vfree(buf); >> if (out_bs) { >> bdrv_delete(out_bs); >> diff --git a/util/qemu-option.c b/util/qemu-option.c >> index 0e42452..dba82b4 100644 >> --- a/util/qemu-option.c >> +++ b/util/qemu-option.c > > Changes to this file not reviewed in this round. > >> @@ -33,6 +33,8 @@ >> #include "qapi/qmp/qerror.h" >> #include "qemu/option_int.h" >> >> +static void qemu_opt_del(QemuOpt *opt); >> + >> /* >> * Extracts the name of an option from the parameter string (p points at the >> * first byte of the option name) >> @@ -531,6 +533,16 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) >> return opt ? opt->str : NULL; >> } >> >> +const char *qemu_opt_get_del(QemuOpts *opts, const char *name) >> +{ >> + QemuOpt *opt = qemu_opt_find(opts, name); >> + const char *str = opt ? opt->str : NULL; >> + if (opt) { >> + qemu_opt_del(opt); >> + } >> + return str; >> +} >> + >> bool qemu_opt_has_help_opt(QemuOpts *opts) >> { >> QemuOpt *opt; >> @@ -553,6 +565,22 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) >> return opt->value.boolean; >> } >> >> +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval) >> +{ >> + QemuOpt *opt = qemu_opt_find(opts, name); >> + bool ret; >> + >> + if (opt == NULL) { >> + return defval; >> + } >> + ret = opt->value.boolean; >> + assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); >> + if (opt) { >> + qemu_opt_del(opt); >> + } >> + return ret; >> +} >> + >> uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) >> { >> QemuOpt *opt = qemu_opt_find(opts, name); >> @@ -573,6 +601,23 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) >> return opt->value.uint; >> } >> >> +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, >> + uint64_t defval) >> +{ >> + QemuOpt *opt = qemu_opt_find(opts, name); >> + uint64_t ret; >> + >> + if (opt == NULL) { >> + return defval; >> + } >> + ret = opt->value.uint; >> + assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); >> + if (opt) { >> + qemu_opt_del(opt); >> + } >> + return ret; >> +} >> + >> static void qemu_opt_parse(QemuOpt *opt, Error **errp) >> { >> if (opt->desc == NULL) >> @@ -624,7 +669,7 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, >> } >> >> static void opt_set(QemuOpts *opts, const char *name, const char *value, >> - bool prepend, Error **errp) >> + bool prepend, bool replace, Error **errp) >> { >> QemuOpt *opt; >> const QemuOptDesc *desc; >> @@ -636,6 +681,11 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, >> return; >> } >> >> + opt = qemu_opt_find(opts, name); >> + if (replace && opt) { >> + qemu_opt_del(opt); >> + } >> + >> opt = g_malloc0(sizeof(*opt)); >> opt->name = g_strdup(name); >> opt->opts = opts; >> @@ -658,7 +708,29 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) >> { >> Error *local_err = NULL; >> >> - opt_set(opts, name, value, false, &local_err); >> + opt_set(opts, name, value, false, false, &local_err); >> + if (error_is_set(&local_err)) { >> + qerror_report_err(local_err); >> + error_free(local_err); >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> +/* >> + * If name exists, the function will delete the opt first and then add a new >> + * one. >> + */ >> +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value) >> +{ >> + Error *local_err = NULL; >> + QemuOpt *opt = qemu_opt_find(opts, name); >> + >> + if (opt) { >> + qemu_opt_del(opt); >> + } >> + opt_set(opts, name, value, false, true, &local_err); >> if (error_is_set(&local_err)) { >> qerror_report_err(local_err); >> error_free(local_err); >> @@ -671,7 +743,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) >> void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, >> Error **errp) >> { >> - opt_set(opts, name, value, false, errp); >> + opt_set(opts, name, value, false, false, errp); >> } >> >> int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) >> @@ -895,7 +967,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy) >> } >> >> static int opts_do_parse(QemuOpts *opts, const char *params, >> - const char *firstname, bool prepend) >> + const char *firstname, bool prepend, bool replace) >> { >> char option[128], value[1024]; >> const char *p,*pe,*pc; >> @@ -931,7 +1003,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params, >> } >> if (strcmp(option, "id") != 0) { >> /* store and parse */ >> - opt_set(opts, option, value, prepend, &local_err); >> + opt_set(opts, option, value, prepend, replace, &local_err); >> if (error_is_set(&local_err)) { >> qerror_report_err(local_err); >> error_free(local_err); >> @@ -947,9 +1019,16 @@ static int opts_do_parse(QemuOpts *opts, const char *params, >> >> int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) >> { >> - return opts_do_parse(opts, params, firstname, false); >> + return opts_do_parse(opts, params, firstname, false, false); >> +} >> + >> +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, >> + const char *firstname) >> +{ >> + return opts_do_parse(opts, params, firstname, false, true); >> } >> >> + >> static QemuOpts *opts_parse(QemuOptsList *list, const char *params, >> int permit_abbrev, bool defaults) >> { >> @@ -986,7 +1065,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, >> return NULL; >> } >> >> - if (opts_do_parse(opts, params, firstname, defaults) != 0) { >> + if (opts_do_parse(opts, params, firstname, defaults, false) != 0) { >> qemu_opts_del(opts); >> return NULL; >> } > Really thanks for your reviewing, Markus.
Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: > On Thu, Mar 21, 2013 at 11:31 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Dong Xu Wang <wdongxu@linux.vnet.ibm.com> writes: [...] >>> diff --git a/block.c b/block.c >>> index 4582961..975c3d8 100644 >>> --- a/block.c >>> +++ b/block.c [...] >>> @@ -4521,28 +4522,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_opts = qemu_opts_append(drv->bdrv_create_opts, >>> + proto_drv->bdrv_create_opts); >> >> qemu_opts_append() dereferences its arguments to compute >> >> dest->merge_lists = first->merge_lists || second->merge_lists; >> >> However, either of drv->create_opts and proto_drv->create_opts may be >> null, as far as I can see. If I'm correct, you need a few more test >> cases :) >> > Okay, will check first and second. >> 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? Beyond the >> scope of this patch. >> >>> /* Create parameter list with default values */ >>> - param = parse_option_parameters("", create_options, param); >>> + opts = qemu_opts_create_nofail(create_opts); >> >> Before: param empty. >> >> After: opts empty. >> >> Good. >> >>> >>> - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >>> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); >> >> Before: param contains exactly BLOCK_OPT_SIZE = img_size. >> >> After: opts contains exactly BLOCK_OPT_SIZE = img_size. >> >> Both old and new code seem to assume BLOCK_OPT_SIZE is always a valid >> option. Beyond the scope of this patch. >> >> Good. >> >>> >>> /* Parse -o options */ >>> if (options) { >>> - param = parse_option_parameters(options, create_options, param); >>> - if (param == NULL) { >>> + if (qemu_opts_do_parse_replace(opts, options, NULL) != 0) { >>> error_setg(errp, "Invalid options for file format '%s'.", fmt); >>> goto out; >>> } >>> } >> >> Before: values from -o replace whatever is in param already. >> >> After: values from -o replace whatever is in opts already. >> >> In particular, size from -o replaces img_size before and after. >> >> Did you test it does? >> >> $ qemu-img create -f raw -o size=1024 t.img 512 >> Formatting 't.img', fmt=raw size=1024 >> $ ls -l t.img >> -rw-r--r--. 1 armbru armbru 1024 Mar 21 15:12 t.img >> >>> >>> if (base_filename) { >>> - if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, >>> + if (qemu_opt_replace_set(opts, BLOCK_OPT_BACKING_FILE, >>> base_filename)) { >>> error_setg(errp, "Backing file not supported for file format '%s'", >>> fmt); >> >> Before: base_filename replaces any backing_file from -o in param. >> >> After: base_filename replaces any backing_file from -o in opts. >> >> Did you test it does? >> > Oh, I did not test command line like this, my patch will fail here. > Will fix in next version. > >> $ qemu-img create -f qcow2 -b t.img -o backing_file=/dev/null t.qcow2 >> Formatting 't.qcow2', fmt=qcow2 size=1024 backing_file='t.img' encryption=off cluster_size=65536 lazy_refcounts=off >> $ qemu-img info t.qcow2image: t.qcow2 >> file format: qcow2 >> virtual size: 1.0K (1024 bytes) >> disk size: 196K >> cluster_size: 65536 >> backing file: t.img >> > > I run this command using upstream QEMU code, but failed. > [wdongxu@13@VM:~]$ qemu-img info t.qcow2image: t.qcow2 > qemu-img: Could not open 't.qcow2image:': No such file or directory > > Or did I type something wrong? Looks like I pastoed. Please try "qemu-img info t.qcow2". [...] >>> diff --git a/block/cow.c b/block/cow.c >>> index 4baf904..135fa33 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,11 @@ 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 */ >>> + image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; >>> + image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); >> >> Why _del? >> > > Before: after getting one parameter, then option++, so in my patches, I want to > delete the opt after using it. If I did not delete it, some parameters > will be passed > to "protocol", it will go wrong. Do you think it is acceptable? I missed the fact that the old code changes its options parameter before it passes it to bdrv_create_file(). Let's examine how it works. options is either null an array of QEMUOptionParameter terminated by an sentinel element with a null name. The loop steps through options until it hits the sentinel. Postcondition: !options || !options->name. In words: either no options, or an empty array of options. Then: >>> >>> - ret = bdrv_create_file(filename, options); >>> + ret = bdrv_create_file(filename, opts); >>> if (ret < 0) { >>> return ret; >>> } We pass either no options or an empty array of options to bdrv_create_file(). I suspect we could just as well pass NULL. If I'm right, you should pass NULL instead of opts. No reason to change opts then. >>> @@ -318,18 +312,22 @@ exit: >>> return ret; >>> } [...] > > Really thanks for your reviewing, Markus. You're welcome!
diff --git a/block.c b/block.c index 4582961..975c3d8 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 ?: qemu_opts_create_nofail(drv->bdrv_create_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); } /* @@ -814,7 +814,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 @@ -847,17 +847,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_opts); - 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; } @@ -4502,8 +4501,10 @@ void bdrv_img_create(const char *filename, const char *fmt, char *options, uint64_t img_size, int flags, Error **errp, bool quiet) { - QEMUOptionParameter *param = NULL, *create_options = NULL; - QEMUOptionParameter *backing_fmt, *backing_file, *size; + QemuOpts *opts = NULL; + QemuOptsList *create_opts = NULL; + const char *backing_fmt, *backing_file; + int64_t size; BlockDriverState *bs = NULL; BlockDriver *drv, *proto_drv; BlockDriver *backing_drv = NULL; @@ -4521,28 +4522,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_opts = qemu_opts_append(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); /* Create parameter list with default values */ - param = parse_option_parameters("", create_options, param); + opts = qemu_opts_create_nofail(create_opts); - 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_replace(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_replace_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) { error_setg(errp, "Backing file not supported for file format '%s'", fmt); @@ -4551,39 +4547,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_replace_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_size(opts, BLOCK_OPT_SIZE, 0); + if (size == -1) { + if (backing_file) { uint64_t size; - char buf[32]; int back_flags; /* backing files always opened read-only */ @@ -4592,17 +4586,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; @@ -4611,10 +4604,10 @@ void bdrv_img_create(const char *filename, const char *fmt, if (!quiet) { printf("Formatting '%s', fmt=%s ", filename, fmt); - print_option_parameters(param); + qemu_opts_print(opts, 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 " @@ -4629,8 +4622,10 @@ void bdrv_img_create(const char *filename, const char *fmt, } out: - free_option_parameters(create_options); - free_option_parameters(param); + qemu_opts_free(create_opts); + if (opts) { + qemu_opts_del(opts); + } if (bs) { bdrv_delete(bs); diff --git a/block/cow.c b/block/cow.c index 4baf904..135fa33 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,11 @@ 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 */ + image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; + image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { return ret; } @@ -318,18 +312,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_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 +343,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_opts = &cow_create_opts, }; static void bdrv_cow_init(void) diff --git a/block/gluster.c b/block/gluster.c index ccd684d..a219201 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,12 +349,8 @@ 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++; - } + total_size = + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; fd = glfs_creat(glfs, gconf->image, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR); @@ -544,13 +539,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_SIZE, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_gluster = { @@ -565,7 +564,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_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_tcp = { @@ -580,7 +579,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_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_unix = { @@ -595,7 +594,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_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_rdma = { @@ -610,7 +609,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_opts = &qemu_gluster_create_opts, }; static void bdrv_gluster_init(void) diff --git a/block/iscsi.c b/block/iscsi.c index deb3b68..def377a 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -789,9 +789,7 @@ static char *parse_initiator_name(const char *target) if (!opts) { opts = QTAILQ_FIRST(&list->head); } - if (opts) { - name = qemu_opt_get(opts, "initiator-name"); - } + name = qemu_opt_get(opts, "initiator-name"); } if (name) { @@ -1073,7 +1071,7 @@ out: return ret; } -static QEMUOptionParameter iscsi_create_options[] = { +static QEMUOptionParameter iscsi_create_opts[] = { { .name = BLOCK_OPT_SIZE, .type = OPT_SIZE, @@ -1090,7 +1088,7 @@ static BlockDriver bdrv_iscsi = { .bdrv_file_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_create = iscsi_create, - .create_options = iscsi_create_options, + .create_opts = iscsi_create_opts, .bdrv_getlength = iscsi_getlength, diff --git a/block/qcow.c b/block/qcow.c index a7135ee..f0c4c38 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,14 @@ 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; - } - options++; + /* Read out opts */ + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, 0)) { + flags |= BLOCK_FLAG_ENCRYPT; } - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { return ret; } @@ -851,24 +846,28 @@ 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_SIZE, + .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", + .def_value_str = "off" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_qcow = { @@ -889,7 +888,7 @@ static BlockDriver bdrv_qcow = { .bdrv_write_compressed = qcow_write_compressed, .bdrv_get_info = qcow_get_info, - .create_options = qcow_create_options, + .bdrv_create_opts = &qcow_create_opts, }; static void bdrv_qcow_init(void) diff --git a/block/qcow2.c b/block/qcow2.c index 7610e56..83288a8 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,41 @@ 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; - } - options++; + sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, 0)) { + flags |= BLOCK_FLAG_ENCRYPT; + } + cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, + DEFAULT_CLUSTER_SIZE); + buf = qemu_opt_get_del(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_del(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_del(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) { + flags |= BLOCK_FLAG_LAZY_REFCOUNTS; } if (backing_file && prealloc) { @@ -1374,7 +1370,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 +1631,55 @@ 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_SIZE, + .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", + .def_value_str = "off" + }, + { + .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", + .def_value_str = "off" + }, + { /* 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_opts = &qcow2_create_opts, }; static void bdrv_qcow2_init(void) diff --git a/block/qed.c b/block/qed.c index b8515e5..6032ea6 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,24 +611,14 @@ 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++; - } + image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); + cluster_size = qemu_opt_get_size_del(opts, + BLOCK_OPT_CLUSTER_SIZE, + QED_DEFAULT_CLUSTER_SIZE); + table_size = qemu_opt_get_size_del(opts, BLOCK_OPT_TABLE_SIZE, + QED_DEFAULT_TABLE_SIZE); if (!qed_is_cluster_size_valid(cluster_size)) { fprintf(stderr, "QED cluster size must be within range [%u, %u] and power of 2\n", @@ -1537,36 +1527,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_SIZE, + .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_opts = &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 4dfdf98..18e965f 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; @@ -1006,19 +1019,14 @@ 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++; - } + total_size = + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); @@ -1145,15 +1153,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", @@ -1176,8 +1175,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_opts = &file_proto_create_opts, }; /***********************************************/ @@ -1459,20 +1457,15 @@ 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++; - } + total_size = + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; fd = qemu_open(filename, O_WRONLY | O_BINARY); if (fd < 0) @@ -1496,7 +1489,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, @@ -1505,7 +1498,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, @@ -1514,9 +1506,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_opts = &file_proto_create_opts, /* generic scsi device */ #ifdef __linux__ @@ -1630,7 +1623,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, @@ -1646,6 +1638,7 @@ static BlockDriver bdrv_host_floppy = { .bdrv_is_inserted = floppy_is_inserted, .bdrv_media_changed = floppy_media_changed, .bdrv_eject = floppy_eject, + .bdrv_create_opts = &file_proto_create_opts, }; static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) @@ -1732,7 +1725,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, @@ -1752,6 +1744,8 @@ static BlockDriver bdrv_host_cdrom = { /* generic scsi device */ .bdrv_ioctl = hdev_ioctl, .bdrv_aio_ioctl = hdev_aio_ioctl, + + .bdrv_create_opts = &file_proto_create_opts, }; #endif /* __linux__ */ @@ -1854,7 +1848,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..d000f88 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -382,18 +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++; - } + + total_size = + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512; fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); @@ -405,13 +402,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_SIZE, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_file = { @@ -431,7 +432,7 @@ static BlockDriver bdrv_file = { .bdrv_get_allocated_file_size = raw_get_allocated_file_size, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, }; /***********************************************/ diff --git a/block/raw.c b/block/raw.c index 75812db..8cb26c2 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_SIZE, + .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_opts = &raw_create_opts, }; static void bdrv_raw_init(void) diff --git a/block/rbd.c b/block/rbd.c index 8cd10a7..2b56ed7 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,18 @@ 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; - } + bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + objsize = qemu_opt_get_size_del(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; } - options++; + obj_order = ffs(objsize) - 1; } clientname = qemu_rbd_parse_clientname(conf, clientname_buf); @@ -920,20 +914,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_SIZE, + .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 +939,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_opts = &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 d466b23..56397cc 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,18 @@ 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; - } - } - options++; + vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + buf = qemu_opt_get_del(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; } - if (vdi_size > SD_MAX_VDI_SIZE) { error_report("too big image size"); ret = -EINVAL; @@ -2043,24 +2035,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_SIZE, + .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 = { @@ -2085,7 +2080,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_save_vmstate = sd_save_vmstate, .bdrv_load_vmstate = sd_load_vmstate, - .create_options = sd_create_options, + .bdrv_create_opts = &sd_create_opts, }; static void bdrv_sheepdog_init(void) diff --git a/block/vdi.c b/block/vdi.c index 87c691b..48b1bec 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -633,7 +633,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; @@ -648,25 +648,18 @@ 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; + bytes = qemu_opt_get_size_del(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_del(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 - } - options++; + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, 0)) { + image_type = VDI_TYPE_STATIC; } +#endif fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, @@ -746,29 +739,34 @@ 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_SIZE, + .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", + .def_value_str = "off" + }, #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 = { @@ -789,7 +787,7 @@ static BlockDriver bdrv_vdi = { .bdrv_get_info = vdi_get_info, - .create_options = vdi_create_options, + .bdrv_create_opts = &vdi_create_opts, .bdrv_check = vdi_check, }; diff --git a/block/vmdk.c b/block/vmdk.c index aef1abc..5428b1a 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1429,7 +1429,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]; @@ -1470,21 +1470,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_ADAPTER_TYPE)) { - adapter_type = options->value.s; - } 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; - } - options++; + /* Read out opts */ + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE); + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, 0)) { + flags |= BLOCK_FLAG_COMPAT6; } + fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); if (!adapter_type) { adapter_type = "ide"; } else if (strcmp(adapter_type, "ide") && @@ -1665,36 +1658,41 @@ 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_ADAPTER_TYPE, - .type = OPT_STRING, - .help = "Virtual adapter type, can be one of " - "ide (default), lsilogic, buslogic or legacyESX" - }, - { - .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_SIZE, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_ADAPTER_TYPE, + .type = OPT_STRING, + .help = "Virtual adapter type, can be one of " + "ide (default), lsilogic, buslogic or legacyESX" + }, + { + .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", + .def_value_str = "off" + }, + { + .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 = { @@ -1711,7 +1709,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_opts = &vmdk_create_opts, }; static void bdrv_vmdk_init(void) diff --git a/block/vpc.c b/block/vpc.c index 82229ef..e8c439f 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -683,34 +683,31 @@ 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")) { + /* Read out opts */ + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); + if (disk_type_param) { + if (!strcmp(disk_type_param, "dynamic")) { disk_type = VHD_DYNAMIC; - } else if (!strcmp(disk_type_param->value.s, "fixed")) { + } else if (!strcmp(disk_type_param, "fixed")) { disk_type = VHD_FIXED; } else { return -EINVAL; } - } else { - disk_type = VHD_DYNAMIC; } /* Create the file */ @@ -798,20 +795,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_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} " + }, + { /* end of list */ } + } }; static BlockDriver bdrv_vpc = { @@ -827,7 +828,7 @@ static BlockDriver bdrv_vpc = { .bdrv_read = vpc_co_read, .bdrv_write = vpc_co_write, - .create_options = vpc_create_options, + .bdrv_create_opts = &vpc_create_opts, }; static void bdrv_vpc_init(void) diff --git a/block/vvfat.c b/block/vvfat.c index 06e6654..26ebe7c 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_opts); + 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 0f750d7..a477b7a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -126,8 +126,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 eaad53e..4f942ef 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -90,7 +90,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 */ @@ -179,9 +179,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_opts; /* * Returns 0 for completed check, -errno for internal errors. diff --git a/include/qemu/option.h b/include/qemu/option.h index c58db43..0d5fb66 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -108,6 +108,7 @@ struct QemuOptsList { }; const char *qemu_opt_get(QemuOpts *opts, const char *name); +const char *qemu_opt_get_del(QemuOpts *opts, const char *name); /** * qemu_opt_has_help_opt: * @opts: options to search for a help request @@ -121,9 +122,13 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name); */ bool qemu_opt_has_help_opt(QemuOpts *opts); bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval); uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval); uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval); +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, + uint64_t defval); int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value); void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp); int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); @@ -144,7 +149,10 @@ const char *qemu_opts_id(QemuOpts *opts); void qemu_opts_del(QemuOpts *opts); void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); -QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, + const char *firstname); +QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, + int permit_abbrev); void qemu_opts_set_defaults(QemuOptsList *list, const char *params, int permit_abbrev); QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, @@ -156,8 +164,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, int abort_on_failure); -QemuOptsList *qemu_opts_append(QemuOptsList *first, - QemuOptsList *second); +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); void qemu_opts_free(QemuOptsList *list); void qemu_opts_print_help(QemuOptsList *list); #endif diff --git a/qemu-img.c b/qemu-img.c index 471de7d..9339957 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -229,7 +229,7 @@ static int read_password(char *buf, int buf_size) static int print_block_option_help(const char *filename, const char *fmt) { BlockDriver *drv, *proto_drv; - QEMUOptionParameter *create_options = NULL; + QemuOptsList *create_opts = NULL; /* Find driver and parse its options */ drv = bdrv_find_format(fmt); @@ -244,12 +244,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_opts = qemu_opts_append(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); + qemu_opts_print_help(create_opts); + qemu_opts_free(create_opts); return 0; } @@ -301,19 +299,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; @@ -1120,8 +1118,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_opts = NULL; + const char *out_baseimg_param; char *options = NULL; const char *snapshot_name = NULL; float local_progress = 0; @@ -1265,40 +1264,36 @@ static int img_convert(int argc, char **argv) goto out; } - create_options = append_option_parameters(create_options, - drv->create_options); - create_options = append_option_parameters(create_options, - proto_drv->create_options); + create_opts = qemu_opts_append(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); if (options) { - param = parse_option_parameters(options, create_options, param); - if (param == NULL) { + if (qemu_opts_do_parse_replace(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_opts); } - - 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"); @@ -1306,15 +1301,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"); @@ -1532,8 +1527,10 @@ static int img_convert(int argc, char **argv) } out: qemu_progress_end(); - free_option_parameters(create_options); - free_option_parameters(param); + qemu_opts_free(create_opts); + if (param) { + qemu_opts_del(param); + } qemu_vfree(buf); if (out_bs) { bdrv_delete(out_bs); diff --git a/util/qemu-option.c b/util/qemu-option.c index 0e42452..dba82b4 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -33,6 +33,8 @@ #include "qapi/qmp/qerror.h" #include "qemu/option_int.h" +static void qemu_opt_del(QemuOpt *opt); + /* * Extracts the name of an option from the parameter string (p points at the * first byte of the option name) @@ -531,6 +533,16 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) return opt ? opt->str : NULL; } +const char *qemu_opt_get_del(QemuOpts *opts, const char *name) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + const char *str = opt ? opt->str : NULL; + if (opt) { + qemu_opt_del(opt); + } + return str; +} + bool qemu_opt_has_help_opt(QemuOpts *opts) { QemuOpt *opt; @@ -553,6 +565,22 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) return opt->value.boolean; } +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + bool ret; + + if (opt == NULL) { + return defval; + } + ret = opt->value.boolean; + assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); + if (opt) { + qemu_opt_del(opt); + } + return ret; +} + uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) { QemuOpt *opt = qemu_opt_find(opts, name); @@ -573,6 +601,23 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) return opt->value.uint; } +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, + uint64_t defval) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + uint64_t ret; + + if (opt == NULL) { + return defval; + } + ret = opt->value.uint; + assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); + if (opt) { + qemu_opt_del(opt); + } + return ret; +} + static void qemu_opt_parse(QemuOpt *opt, Error **errp) { if (opt->desc == NULL) @@ -624,7 +669,7 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, } static void opt_set(QemuOpts *opts, const char *name, const char *value, - bool prepend, Error **errp) + bool prepend, bool replace, Error **errp) { QemuOpt *opt; const QemuOptDesc *desc; @@ -636,6 +681,11 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, return; } + opt = qemu_opt_find(opts, name); + if (replace && opt) { + qemu_opt_del(opt); + } + opt = g_malloc0(sizeof(*opt)); opt->name = g_strdup(name); opt->opts = opts; @@ -658,7 +708,29 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) { Error *local_err = NULL; - opt_set(opts, name, value, false, &local_err); + opt_set(opts, name, value, false, false, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + return -1; + } + + return 0; +} + +/* + * If name exists, the function will delete the opt first and then add a new + * one. + */ +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value) +{ + Error *local_err = NULL; + QemuOpt *opt = qemu_opt_find(opts, name); + + if (opt) { + qemu_opt_del(opt); + } + opt_set(opts, name, value, false, true, &local_err); if (error_is_set(&local_err)) { qerror_report_err(local_err); error_free(local_err); @@ -671,7 +743,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp) { - opt_set(opts, name, value, false, errp); + opt_set(opts, name, value, false, false, errp); } int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) @@ -895,7 +967,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy) } static int opts_do_parse(QemuOpts *opts, const char *params, - const char *firstname, bool prepend) + const char *firstname, bool prepend, bool replace) { char option[128], value[1024]; const char *p,*pe,*pc; @@ -931,7 +1003,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params, } if (strcmp(option, "id") != 0) { /* store and parse */ - opt_set(opts, option, value, prepend, &local_err); + opt_set(opts, option, value, prepend, replace, &local_err); if (error_is_set(&local_err)) { qerror_report_err(local_err); error_free(local_err); @@ -947,9 +1019,16 @@ static int opts_do_parse(QemuOpts *opts, const char *params, int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) { - return opts_do_parse(opts, params, firstname, false); + return opts_do_parse(opts, params, firstname, false, false); +} + +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, + const char *firstname) +{ + return opts_do_parse(opts, params, firstname, false, true); } + static QemuOpts *opts_parse(QemuOptsList *list, const char *params, int permit_abbrev, bool defaults) { @@ -986,7 +1065,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return NULL; } - if (opts_do_parse(opts, params, firstname, defaults) != 0) { + if (opts_do_parse(opts, params, firstname, defaults, false) != 0) { qemu_opts_del(opts); return NULL; }
This patch will use QemuOpts related functions in block layer, add a member bdrv_create_opts 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> --- v11->v12: 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. These functions works like origin code. 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. 3) in bdrv_create, if opts is NULL, will create an empty one, so can discard if(opts) code safely. 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/iscsi.c | 8 +-- block/qcow.c | 61 ++++++++-------- block/qcow2.c | 173 +++++++++++++++++++++++----------------------- block/qed.c | 86 +++++++++++------------ block/qed.h | 2 +- block/raw-posix.c | 59 +++++++--------- block/raw-win32.c | 31 +++++---- block/raw.c | 30 ++++---- block/rbd.c | 62 ++++++++--------- block/sheepdog.c | 75 ++++++++++---------- block/vdi.c | 70 +++++++++---------- block/vmdk.c | 90 ++++++++++++------------ block/vpc.c | 57 +++++++-------- block/vvfat.c | 11 +-- include/block/block.h | 4 +- include/block/block_int.h | 6 +- include/qemu/option.h | 13 +++- qemu-img.c | 61 ++++++++-------- util/qemu-option.c | 93 +++++++++++++++++++++++-- 22 files changed, 613 insertions(+), 553 deletions(-)