Re: [Qemu-block] [RFC PATCH 04/10] qcow2: Pass BlockdevCreateOptions to qcow2_create2()

[Max Reitz]

On 2018-01-11 20:52, Kevin Wolf wrote:
> All of the simple options are now passed to qcow2_create2() in a
> BlockdevCreateOptions object. Still missing: node-name and the
> encryption options.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block/qcow2.c | 186 
> ++++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 148 insertions(+), 38 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index b02bc39a01..09e567324d 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -2690,12 +2697,11 @@ static uint64_t 
> qcow2_opt_get_refcount_bits_del(QemuOpts *opts, int version,
>      return refcount_bits;
>  }
>  
> -static int qcow2_create2(BlockDriverState *bs, int64_t total_size,
> -                         const char *backing_file, const char 
> *backing_format,
> -                         int flags, size_t cluster_size, PreallocMode 
> prealloc,
> -                         QemuOpts *opts, int version, int refcount_order,
> -                         const char *encryptfmt, Error **errp)
> +static int qcow2_create2(BlockDriverState *bs,
> +                         BlockdevCreateOptions *create_options,

I'd personally really prefer this to be const...

> +                         QemuOpts *opts, const char *encryptfmt, Error 
> **errp)
>  {
> +    BlockdevCreateOptionsQcow2 *qcow2_opts;
>      QDict *options;
>  
>      /*
> @@ -2712,10 +2718,88 @@ static int qcow2_create2(BlockDriverState *bs, 
> int64_t total_size,

[...]

> +
> +    if (!qcow2_opts->has_preallocation) {
> +        qcow2_opts->preallocation = PREALLOC_MODE_OFF;
> +    }
> +    if (qcow2_opts->backing_file &&
> +        qcow2_opts->preallocation != PREALLOC_MODE_OFF)
> +    {
> +        error_setg(errp, "Backing file and preallocation cannot be used at "
> +                   "the same time");
> +        return -EINVAL;
> +    }
> +
> +    if (!qcow2_opts->has_lazy_refcounts) {
> +        qcow2_opts->lazy_refcounts = false;

...because modifying some ideally QMP-provided objects just looks wrong.

[...]

> @@ -2804,18 +2888,26 @@ static int qcow2_create2(BlockDriverState *bs, 
> int64_t total_size,

[...]

>      /* Want a backing file? There you go.*/
> -    if (backing_file) {
> -        ret = bdrv_change_backing_file(blk_bs(blk), backing_file, 
> backing_format);
> +    if (qcow2_opts->has_backing_file) {
> +        const char *backing_format = NULL;
> +
> +        if (qcow2_opts->has_backing_fmt) {
> +            backing_format = BlockdevDriver_str(qcow2_opts->backing_fmt);
> +        }

has_backing_fmt && !has_backing_file should probably be an error.

Max

> +
> +        ret = bdrv_change_backing_file(blk_bs(blk), qcow2_opts->backing_file,
> +                                       backing_format);
>          if (ret < 0) {
>              error_setg_errno(errp, -ret, "Could not assign backing file '%s' 
> "
> -                             "with format '%s'", backing_file, 
> backing_format);
> +                             "with format '%s'", qcow2_opts->backing_file,
> +                             backing_format);
>              goto out;
>          }
>      }

signature.asc
Description: OpenPGP digital signature