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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2538 bytes --]

On 2018-01-29 19:10, Kevin Wolf wrote:
> Am 29.01.2018 um 18:12 hat Max Reitz geschrieben:
>> 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 <kwolf@redhat.com>
>>> ---
>>>  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.
> 
> Isn't this pretty standard, though? Most commands don't use boxed
> options, so there they only modify stack variables, but if you look at
> boxed ones like do_blockdev_backup() or qmp_drive_mirror(), they do the
> same.

:C


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]