On Thu, Oct 25, 2012 at 08:57:27PM +0800, Dong Xu Wang wrote:
> @@ -177,9 +177,7 @@ struct BlockDriver {
> unsigned long int req, void *buf,
> BlockDriverCompletionFunc *cb, void *opaque);
>
> - /* List of options for creating images, terminated by name == NULL */
> - QEMUOptionParameter *create_options;
> -
> + QemuOptsList *(*bdrv_create_options)(void);
Why is .bdrv_create_options() a function? It would be less code for
BlockDrivers to have a pointer: QemuOptsList *.
> /* 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);
> + const char *encryption =
> + qemu_opt_get(param, BLOCK_OPT_ENCRYPT);
> + const char *preallocation =
> + qemu_opt_get(param, BLOCK_OPT_PREALLOC);
>
> if (!drv->bdrv_write_compressed) {
> error_report("Compression not supported for this file format");
> @@ -847,15 +842,15 @@ static int img_convert(int argc, char **argv)
> goto out;
> }
>
> - if (encryption && encryption->value.n) {
> + if (encryption) {
encryption=off will print an error here. We need to use
qemu_opt_get_bool().
> diff --git a/qemu-option.c b/qemu-option.c
> index ab270f0..c391f7d 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -553,10 +553,15 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char
> *name, bool defval)
>
> uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t
> defval)
> {
> - QemuOpt *opt = qemu_opt_find(opts, name);
> + QemuOpt *opt;
> + if (!opts) {
> + return defval;
> + }
> + opt = qemu_opt_find(opts, name);
>
> - if (opt == NULL)
> + if (opt == NULL) {
> return defval;
> + }
> assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER);
> return opt->value.uint;
> }
This should be a separate patch, but there is a problem with the change
anyway: all qemu_opt_*() functions assume QemuOpts *opts is non-NULL.
This change makes qemu_opt_get_number() inconsistent. This will lead to
bugs when people assume qemu_opt_get_bool() also handles opts == NULL,
for example.
Can the qemu-img/block callers make sure opts is never NULL or will that
add a lot of extra checks?
Stefan
