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

2023-12-18 Thread Markus Armbruster
Sam Li  writes:

> Markus Armbruster  于2023年11月30日周四 09:40写道:
>>
>> Sam Li  writes:

[...]

>> > +##
>> > +# @Qcow2ZoneHostManaged:
>> > +#
>> > +# The host-managed zone model.  It only allows sequential writes.
>> > +#
>> > +# @size: Total number of bytes within zones
>> > +#
>> > +# @capacity: The number of usable logical blocks within zones
>> > +# in bytes.  A zone capacity is always smaller or equal to the
>> > +# zone size
>> > +#
>> > +# @conventional-zones: The number of conventional zones of the
>> > +# zoned device
>> > +#
>> > +# @max-open-zones: The maximal number of open zones
>> > +#
>> > +# @max-active-zones: The maximal number of zones in the implicit
>> > +# open, explicit open or closed state
>> > +#
>> > +# @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
>>
>> Missing period at the end.
>>
>> For all the optional members: what's the default?
>
> The default for optional members is 0. When max-open-zones and
> max-active-zones are 0, it implies no limit on zone resources.

Please document the default in the doc comment.  We commonly do it like
this:

  # @max-open-zones: The maximal number of open zones (default 0)

[...]




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

2023-12-16 Thread Sam Li
Markus Armbruster  于2023年11月30日周四 09:40写道:
>
> Sam Li  writes:
>
> > 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 image with zoned format feature, use command like
> > this:
> > $path/to/qemu-img create -f qcow2 zbc.qcow2 -o size=768M
>
> I'd omit $path/to/
>
> > -o zone.size=64M -o zone.capacity=64M -o zone.conventional_zones=0
> > -o zone.max_append_bytes=4096 -o zone.max_open_zones=10
> > -o zone.max_active_zones=12 -o zone.mode=host-managed
>
> Suggest to add \ like this:
>
>   qemu-img create -f qcow2 zbc.qcow2 -o size=768M \
>   -o zone.size=64M -o zone.capacity=64M -o zone.conventional_zones=0 \
>   -o zone.max_append_bytes=4096 -o zone.max_open_zones=10 \
>   -o zone.max_active_zones=12 -o zone.mode=host-managed
>
> >
> > Signed-off-by: Sam Li 
>
> [...]
>
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index ca390c5700..ef98dc83a0 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -5038,6 +5038,64 @@
> >  { 'enum': 'Qcow2CompressionType',
> >'data': [ 'zlib', { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
> >
> > +##
> > +# @Qcow2ZoneModel:
> > +#
> > +# Zoned device model used in qcow2 image file
> > +#
> > +# @host-managed: host-managed model only allows sequential write over the
>
> Suggest "the host-managed model ..."
>
> > +# device zones
> > +#
> > +# Since 8.2
> > +##
> > +{ 'enum': 'Qcow2ZoneModel',
> > +  'data': [ 'host-managed'] }
> > +
> > +##
> > +# @Qcow2ZoneHostManaged:
> > +#
> > +# The host-managed zone model.  It only allows sequential writes.
> > +#
> > +# @size: Total number of bytes within zones
> > +#
> > +# @capacity: The number of usable logical blocks within zones
> > +# in bytes.  A zone capacity is always smaller or equal to the
> > +# zone size
> > +#
> > +# @conventional-zones: The number of conventional zones of the
> > +# zoned device
> > +#
> > +# @max-open-zones: The maximal number of open zones
> > +#
> > +# @max-active-zones: The maximal number of zones in the implicit
> > +# open, explicit open or closed state
> > +#
> > +# @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
>
> Missing period at the end.
>
> For all the optional members: what's the default?

The default for optional members is 0. When max-open-zones and
max-active-zones are 0, it implies no limit on zone resources.

>
> > +#
> > +# Since 8.2
> > +##
> > +{ 'struct': 'Qcow2ZoneHostManaged',
> > +  'data': { '*size':  'size',
> > +'*capacity':  'size',
> > +'*conventional-zones': 'uint32',
> > +'*max-open-zones': 'uint32',
> > +'*max-active-zones':   'uint32',
> > +'*max-append-bytes':   'uint32' } }
>
> Why isn't @max-append-bytes 'size'?  It's a byte count...
>
> > +
> > +##
> > +# @Qcow2ZoneCreateOptions:
> > +#
> > +# The zone device model for the qcow2 image.
> > +#
> > +# Since 8.2
> > +##
> > +{ 'union': 'Qcow2ZoneCreateOptions',
> > +  'base': { 'mode': 'Qcow2ZoneModel' },
> > +  'discriminator': 'mode',
> > +  'data': { 'host-managed': 'Qcow2ZoneHostManaged' } }
> > +
> >  ##
> >  # @BlockdevCreateOptionsQcow2:
> >  #
> > @@ -5080,6 +5138,8 @@
> >  # @compression-type: The image cluster compression method
> >  # (default: zlib, since 5.1)
> >  #
> > +# @zone: @Qcow2ZoneCreateOptions.  The zone device model modes (since 8.2)
>
> Don't put the type into the description like that, because it comes out
> like
>
> "zone": "Qcow2ZoneCreateOptions" (optional)
>"Qcow2ZoneCreateOptions".  The zone device model modes (since 8.2)
>
> in formatted documentation.
>
> Let's spell out the default: the device is not zoned.
>
> > +#
> >  # Since: 2.12
> >  ##
> >  { 'struct': 'BlockdevCreateOptionsQcow2',
> > @@ -5096,7 +5156,8 @@
> >  '*preallocation':   'PreallocMode',
> >  '*lazy-refcounts':  'bool',
> >  '*refcount-bits':   'int',
> > -'*compression-type':'Qcow2CompressionType' } }
> > +'*compression-type':'Qcow2CompressionType',
> > +'*zone':'Qcow2ZoneCreateOptions' } }
> >
> >  ##
> >  # @BlockdevCreateOptionsQed:
>



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

2023-11-30 Thread Markus Armbruster
Sam Li  writes:

> 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 image with zoned format feature, use command like
> this:
> $path/to/qemu-img create -f qcow2 zbc.qcow2 -o size=768M

I'd omit $path/to/

> -o zone.size=64M -o zone.capacity=64M -o zone.conventional_zones=0
> -o zone.max_append_bytes=4096 -o zone.max_open_zones=10
> -o zone.max_active_zones=12 -o zone.mode=host-managed

Suggest to add \ like this:

  qemu-img create -f qcow2 zbc.qcow2 -o size=768M \
  -o zone.size=64M -o zone.capacity=64M -o zone.conventional_zones=0 \
  -o zone.max_append_bytes=4096 -o zone.max_open_zones=10 \
  -o zone.max_active_zones=12 -o zone.mode=host-managed

>
> Signed-off-by: Sam Li 

[...]

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ca390c5700..ef98dc83a0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -5038,6 +5038,64 @@
>  { 'enum': 'Qcow2CompressionType',
>'data': [ 'zlib', { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
>  
> +##
> +# @Qcow2ZoneModel:
> +#
> +# Zoned device model used in qcow2 image file
> +#
> +# @host-managed: host-managed model only allows sequential write over the

Suggest "the host-managed model ..."

> +# device zones
> +#
> +# Since 8.2
> +##
> +{ 'enum': 'Qcow2ZoneModel',
> +  'data': [ 'host-managed'] }
> +
> +##
> +# @Qcow2ZoneHostManaged:
> +#
> +# The host-managed zone model.  It only allows sequential writes.
> +#
> +# @size: Total number of bytes within zones
> +#
> +# @capacity: The number of usable logical blocks within zones
> +# in bytes.  A zone capacity is always smaller or equal to the
> +# zone size
> +#
> +# @conventional-zones: The number of conventional zones of the
> +# zoned device
> +#
> +# @max-open-zones: The maximal number of open zones
> +#
> +# @max-active-zones: The maximal number of zones in the implicit
> +# open, explicit open or closed state
> +#
> +# @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

Missing period at the end.

For all the optional members: what's the default?

> +#
> +# Since 8.2
> +##
> +{ 'struct': 'Qcow2ZoneHostManaged',
> +  'data': { '*size':  'size',
> +'*capacity':  'size',
> +'*conventional-zones': 'uint32',
> +'*max-open-zones': 'uint32',
> +'*max-active-zones':   'uint32',
> +'*max-append-bytes':   'uint32' } }

Why isn't @max-append-bytes 'size'?  It's a byte count...

> +
> +##
> +# @Qcow2ZoneCreateOptions:
> +#
> +# The zone device model for the qcow2 image.
> +#
> +# Since 8.2
> +##
> +{ 'union': 'Qcow2ZoneCreateOptions',
> +  'base': { 'mode': 'Qcow2ZoneModel' },
> +  'discriminator': 'mode',
> +  'data': { 'host-managed': 'Qcow2ZoneHostManaged' } }
> +
>  ##
>  # @BlockdevCreateOptionsQcow2:
>  #
> @@ -5080,6 +5138,8 @@
>  # @compression-type: The image cluster compression method
>  # (default: zlib, since 5.1)
>  #
> +# @zone: @Qcow2ZoneCreateOptions.  The zone device model modes (since 8.2)

Don't put the type into the description like that, because it comes out
like

"zone": "Qcow2ZoneCreateOptions" (optional)
   "Qcow2ZoneCreateOptions".  The zone device model modes (since 8.2)

in formatted documentation.

Let's spell out the default: the device is not zoned.

> +#
>  # Since: 2.12
>  ##
>  { 'struct': 'BlockdevCreateOptionsQcow2',
> @@ -5096,7 +5156,8 @@
>  '*preallocation':   'PreallocMode',
>  '*lazy-refcounts':  'bool',
>  '*refcount-bits':   'int',
> -'*compression-type':'Qcow2CompressionType' } }
> +'*compression-type':'Qcow2CompressionType',
> +'*zone':'Qcow2ZoneCreateOptions' } }
>  
>  ##
>  # @BlockdevCreateOptionsQed:




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

2023-11-26 Thread Sam Li
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 image with zoned format feature, use command like
this:
$path/to/qemu-img create -f qcow2 zbc.qcow2 -o size=768M
-o zone.size=64M -o zone.capacity=64M -o zone.conventional_zones=0
-o zone.max_append_bytes=4096 -o zone.max_open_zones=10
-o zone.max_active_zones=12 -o zone.mode=host-managed

Signed-off-by: Sam Li 
---
 block/qcow2.c| 233 ++-
 block/qcow2.h|  36 -
 docs/interop/qcow2.txt   |  99 -
 include/block/block_int-common.h |  13 ++
 qapi/block-core.json |  63 -
 5 files changed, 440 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 13e032bd5e..9a92cd242c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -73,6 +73,7 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 #define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
 #define  QCOW2_EXT_MAGIC_DATA_FILE 0x44415441
+#define  QCOW2_EXT_MAGIC_ZONED_FORMAT 0x007a6264
 
 static int coroutine_fn
 qcow2_co_preadv_compressed(BlockDriverState *bs,
@@ -194,6 +195,55 @@ qcow2_extract_crypto_opts(QemuOpts *opts, const char *fmt, 
Error **errp)
 return cryptoopts_qdict;
 }
 
+/*
+ * Passing by the zoned device configurations by a zoned_header struct, check
+ * if the zone device options are under constraints. Return false when some
+ * option is invalid
+ */
+static inline bool
+qcow2_check_zone_options(Qcow2ZonedHeaderExtension *zone_opt)
+{
+if (zone_opt) {
+if (zone_opt->zone_size == 0) {
+error_report("Zoned extension header zone_size field "
+ "can not be 0");
+return false;
+}
+
+if (zone_opt->zone_capacity > zone_opt->zone_size) {
+error_report("zone capacity %" PRIu32 "B exceeds zone size "
+ "%" PRIu32"B", zone_opt->zone_capacity,
+ zone_opt->zone_size);
+return false;
+}
+
+if (zone_opt->max_active_zones > zone_opt->nr_zones) {
+error_report("Max_active_zones %" PRIu32 " exceeds "
+ "nr_zones %" PRIu32". Set it to nr_zones.",
+ zone_opt->max_active_zones, zone_opt->nr_zones);
+zone_opt->max_active_zones = zone_opt->nr_zones;
+}
+
+if (zone_opt->max_open_zones > zone_opt->max_active_zones) {
+error_report("Max_open_zones %" PRIu32 " exceeds "
+ "max_active_zones %" PRIu32". Set it to "
+ "max_active_zones.",
+ zone_opt->max_open_zones,
+ zone_opt->max_active_zones);
+zone_opt->max_open_zones = zone_opt->max_active_zones;
+}
+
+if (zone_opt->max_open_zones > zone_opt->nr_zones) {
+error_report("Max_open_zones field can not be larger "
+ "than the number of zones. Set it to nr_zones.");
+zone_opt->max_open_zones = zone_opt->nr_zones;
+}
+
+return true;
+}
+return false;
+}
+
 /*
  * read qcow2 extension and fill bs
  * start reading from start_offset
@@ -211,6 +261,7 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t 
start_offset,
 uint64_t offset;
 int ret;
 Qcow2BitmapHeaderExt bitmaps_ext;
+Qcow2ZonedHeaderExtension zoned_ext;
 
 if (need_update_header != NULL) {
 *need_update_header = false;
@@ -432,6 +483,51 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t 
start_offset,
 break;
 }
 
+case QCOW2_EXT_MAGIC_ZONED_FORMAT:
+{
+if (ext.len < sizeof(zoned_ext)) {
+/* Missing fields */
+error_setg(errp, "zoned_ext: len=%" PRIu32 " too small "
+   "(<%zu)", ext.len, sizeof(zoned_ext));
+return -EINVAL;
+}
+ret = bdrv_pread(bs->file, offset, ext.len, &zoned_ext, 0);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "zoned_ext: "
+ "Could not read ext header");
+return ret;
+}
+
+zoned_ext.zone_size = be32_to_cpu(zoned_ext.zone_size);
+zoned_ext.zone_capacity = be32_to_cpu(zoned_ext.zone_capacity);
+zoned_ext.conventional_zones =
+be32_to_cpu(zoned_ext.conventional_zones);
+zoned_ext.nr_zones = be32_to_cpu(zoned_ext.nr_zones);
+zoned_ext.max_open_zones = be32_to_cpu(zoned_ext.max_open_zones);
+zoned_ext.max_active_zones =
+be32_to_cpu(zoned_ext.max_active_zones);
+zoned_ext.max_append_bytes =
+be32