Re: [PATCH v5 2/4] qcow2: add configurations for zoned format extension

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On Mon, Oct 30, 2023 at 08:18:45PM +0800, Sam Li wrote:
>> To configure the zoned format feature on the qcow2 driver, it
>> requires settings as: the device size, zone model, zone size,
>> zone capacity, number of conventional zones, limits on zone
>> resources (max append bytes, max open zones, and max_active_zones).
>> 
>> To create a qcow2 file with zoned format, use command like this:
>> $ qemu-img create -f qcow2 test.qcow2 -o size=768M -o
>> zone_size=64M -o zone_capacity=64M -o conventional_zones=0 -o
>> max_append_bytes=4096 -o max_open_zones=0 -o max_active_zones=0
>> -o zone_model=host-managed
>> 
>> Signed-off-by: Sam Li <faithilikerun@gmail.com>
>> 
>> fix config?
>
> Is this comment supposed to be part of the commit message?  If not,...
>
>> ---
>
> ...place it here under the divider, so 'git am' won't include it, if there is nothing further to change on this patch.

[...]

>> +++ b/qapi/block-core.json
>> @@ -4981,6 +4981,21 @@
>>  { 'enum': 'Qcow2CompressionType',
>>    'data': [ 'zlib', { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
>>  
>> +##
>> +# @Qcow2ZoneModel:
>> +#
>> +# Zoned device model used in qcow2 image file
>> +#
>> +# @non-zoned: non-zoned model is for regular block devices
>> +#
>> +# @host-managed: host-managed model only allows sequential write over the
>> +#     device zones
>> +#
>> +# Since 8.2
>> +##
>> +{ 'enum': 'Qcow2ZoneModel',
>> +  'data': ['non-zoned', 'host-managed'] }
>> +
>>  ##
>>  # @BlockdevCreateOptionsQcow2:
>>  #
>> @@ -5023,6 +5038,27 @@
>>  # @compression-type: The image cluster compression method
>>  #     (default: zlib, since 5.1)
>>  #
>> +# @zone-model: @Qcow2ZoneModel.  The zone device model.
>> +#     (default: non-zoned, since 8.2)
>> +#
>> +# @zone-size: Total number of bytes within zones (since 8.2)
>
> If @zone-model is "non-zoned", does it make sense to even allow
> @zone-size and friends?  Should this use a QMP union, where you can
> pass in the remaining zone-* fields only when zone-model is set to
> host-managed?

Valid question; needs an answer.

>> +#
>> +# @zone-capacity: The number of usable logical blocks within zones
>> +#     in bytes.  A zone capacity is always smaller or equal to the
>> +#     zone size (since 8.2)
>> +#
>> +# @conventional-zones: The number of conventional zones of the
>> +#     zoned device (since 8.2)
>> +#
>> +# @max-open-zones: The maximal number of open zones (since 8.2)
>> +#
>> +# @max-active-zones: The maximal number of zones in the implicit
>> +#     open, explicit open or closed state (since 8.2)
>> +#
>> +# @max-append-bytes: The maximal number of bytes of a zone
>> +#     append request that can be issued to the device.  It must be
>> +#     512-byte aligned (since 8.2)
>> +#
>>  # Since: 2.12
>>  ##
>>  { 'struct': 'BlockdevCreateOptionsQcow2',
>> @@ -5039,7 +5075,14 @@
>>              '*preallocation':   'PreallocMode',
>>              '*lazy-refcounts':  'bool',
>>              '*refcount-bits':   'int',
>> -            '*compression-type':'Qcow2CompressionType' } }
>> +            '*compression-type':'Qcow2CompressionType',
>> +            '*zone-model':         'Qcow2ZoneModel',
>> +            '*zone-size':          'size',
>> +            '*zone-capacity':      'size',
>> +            '*conventional-zones': 'uint32',
>> +            '*max-open-zones':     'uint32',
>> +            '*max-active-zones':   'uint32',
>> +            '*max-append-bytes':   'uint32' } }
>
> In other words, I'm envisioning something like an optional
> '*zone':'ZoneStruct', where:
>
> { 'struct': 'ZoneHostManaged',
>   'data': { 'size': 'size', '*capacity': 'size', ..., '*max-append-bytes': 'uint32' } }
> { 'union': 'ZoneStruct',
>   'base': { 'model': 'Qcow2ZoneModel' },
>   'discriminator': 'model',
>   'data': { 'non-zoned': {},
>             'host-managed': 'ZoneHostManaged' } }
>
> then over the wire, QMP can use the existing:
> { ..., "compression-type":"zstd" }
>
> as a synonym for the new but explicit non-zoned:
> { ..., "compression-type":"zstd", "zone":{"mode":"non-zoned"} }

I.e. @zone is optional, and defaults to {"mode": "non-zoned"}.

> and when we want to use zones, we pass:
> { ..., "compression-type":"zstd", "zone":{"mode":"host-managed", "size":16777216} }
>
> where you don't have to have zone- prefixing everywhere because it is
> instead contained in the smart union object where it is obvious from
> the 'mode' field what other fields should be present.