Re: [Qemu-devel] [PATCH 7/7] vpc: Support .bdrv_co_create

[Max Reitz <mreitz@redhat.com>]

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

On 2018-03-09 22:46, Kevin Wolf wrote:
> This adds the .bdrv_co_create driver callback to vpc, which
> enables image creation over QMP.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json |  33 ++++++++++-
>  block/vpc.c          | 152 ++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 147 insertions(+), 38 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 3a65909c47..ca645a0067 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3734,6 +3734,37 @@
>              '*block-state-zero':    'bool' } }
>  
>  ##
> +# @BlockdevVpcSubformat:
> +#
> +# @dynamic: Growing image file
> +# @fixed:   Preallocated fixed-size imge file

s/imge/image/

> +#
> +# Since: 2.12
> +##
> +{ 'enum': 'BlockdevVpcSubformat',
> +  'data': [ 'dynamic', 'fixed' ] }
> +
> +##
> +# @BlockdevCreateOptionsVpc:
> +#
> +# Driver specific image creation options for vpc (VHD).
> +#
> +# @file             Node to create the image format on
> +# @size             Size of the virtual disk in bytes
> +# @subformat        vhdx subformat (default: dynamic)
> +# @force-size       Force use of the exact byte size instead of rounding to the
> +#                   next size that can be represented in CHS geometry
> +#                   (default: false)

Now that's weird, again considering your previous approach of only
rounding things in the legacy path and instead throwing errors from
blockdev-create.  If you think this is OK to have here, than that's OK
with me, but I'm not sure this is the ideal way.

Alternatives:

1. Swap the default, not sure this is such a good idea either.

2. Maybe add an enum instead.  Default: If the given size doesn't fit
CHS, generate an error.  Second choice: Use the given size, even if it
doesn't fit.  Third choice: Round to CHS.

I don't want to be stuck up, but once it's a public interface...

So if you think the bool is OK like it is...

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'BlockdevCreateOptionsVpc',
> +  'data': { 'file':                 'BlockdevRef',
> +            'size':                 'size',
> +            '*subformat':           'BlockdevVpcSubformat',
> +            '*force-size':          'bool' } }
> +
> +##
>  # @BlockdevCreateNotSupported:
>  #
>  # This is used for all drivers that don't support creating images.


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