| Message ID | 20231030121847.4522-3-faithilikerun@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add full zoned storage emulation to qcow2 driver | expand |
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. > block/qcow2.c | 205 ++++++++++++++++++++++++++++++- > block/qcow2.h | 37 +++++- > docs/interop/qcow2.txt | 67 +++++++++- > include/block/block_int-common.h | 13 ++ > qapi/block-core.json | 45 ++++++- > 5 files changed, 362 insertions(+), 5 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index aa01d9e7b5..cd53268ca7 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, > @@ -210,6 +211,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; > @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > break; > } > > + case QCOW2_EXT_MAGIC_ZONED_FORMAT: > + { > + if (ext.len != sizeof(zoned_ext)) { > + error_setg(errp, "zoned_ext: Invalid extension length"); > + return -EINVAL; > + } Do we ever anticipate the struct growing in size in the future to add further features? Forcing the size to be constant, rather than a minimum, may get in the way of clean upgrades to a future version of the extension header. > + 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; > + } > + > + if (s->incompatible_features & QCOW2_INCOMPAT_ZONED_FORMAT) { > + warn_report("A program lacking zoned format support " > + "may modify this file and zoned metadata are " > + "now considered inconsistent"); > + error_printf("The zoned metadata is corrupted.\n"); Why is this mixing warn_report and error_printf at the same time. Also, grammar is inconsistent from the similar QCOW2_AUTOCLEAR_BITMAPS, which used: if (s->qcow_version < 3) { /* Let's be a bit more specific */ warn_report("This qcow2 v2 image contains bitmaps, but " "they may have been modified by a program " "without persistent bitmap support; so now " "they must all be considered inconsistent"); } else { warn_report("a program lacking bitmap support " "modified this file, so all bitmaps are now " "considered inconsistent"); This also raises the question whether we want to ever allow zoned support with a v2 image, or whether it should just be a hard error if it is not a v3 image (my preference would be the latter). > + } > + > + 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_to_cpu(zoned_ext.max_append_bytes); > + s->zoned_header = zoned_ext; > + > + /* refuse to open broken images */ > + if (zoned_ext.zone_size == 0) { > + error_setg(errp, "Zoned extension header zone_size field " > + "can not be 0"); > + return -EINVAL; > + } > + if (zoned_ext.zone_capacity > zoned_ext.zone_size) { > + error_setg(errp, "Zoned extension header zone_capacity field " > + "can not be larger that zone_size field"); > + return -EINVAL; > + } > + if (zoned_ext.nr_zones != DIV_ROUND_UP( > + bs->total_sectors * BDRV_SECTOR_SIZE, zoned_ext.zone_size)) { > + error_setg(errp, "Zoned extension header nr_zones field " > + "is wrong"); > + return -EINVAL; > + } Are there any other sanity checks you should do, such as ensuring max_open_zones <= the number of total possible zones once you divide image size by zone size? Such sanity checks would also be useful when creating new image with zones (and not just when opening a pre-existing image); should you have a helper function that performs all the sanity checks for zone values being self-consistent, and reuse it in each context, rather than repeated open-coding the same checks? > + > +#ifdef DEBUG_EXT > + printf("Qcow2: Got zoned format extension: " > + "offset=%" PRIu32 "\n", offset); > +#endif > + break; > + } > + > default: > /* unknown magic - save it in case we need to rewrite the header */ > /* If you add a new feature, make sure to also update the fast > +++ b/block/qcow2.h > @@ -236,6 +236,27 @@ typedef struct Qcow2CryptoHeaderExtension { > uint64_t length; > } QEMU_PACKED Qcow2CryptoHeaderExtension; > > +typedef struct Qcow2ZonedHeaderExtension { > + /* Zoned device attributes */ > + uint8_t zoned; > + uint8_t reserved[3]; > + uint32_t zone_size; > + uint32_t zone_capacity; > + uint32_t conventional_zones; > + uint32_t nr_zones; > + uint32_t max_active_zones; > + uint32_t max_open_zones; > + uint32_t max_append_bytes; > + uint64_t zonedmeta_size; > + uint64_t zonedmeta_offset; > +} QEMU_PACKED Qcow2ZonedHeaderExtension; Nice that everything is well-aligned so that the struct packs into 6 consecutive 8-byte values. > + > +typedef struct Qcow2ZoneListEntry { > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; > +} Qcow2ZoneListEntry; > + > typedef struct Qcow2UnknownHeaderExtension { > uint32_t magic; > uint32_t len; > @@ -256,17 +277,20 @@ enum { > QCOW2_INCOMPAT_DATA_FILE_BITNR = 2, > QCOW2_INCOMPAT_COMPRESSION_BITNR = 3, > QCOW2_INCOMPAT_EXTL2_BITNR = 4, > + QCOW2_INCOMPAT_ZONED_FORMAT_BITNR = 5, > QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR, > QCOW2_INCOMPAT_CORRUPT = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR, > QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, > QCOW2_INCOMPAT_COMPRESSION = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR, > QCOW2_INCOMPAT_EXTL2 = 1 << QCOW2_INCOMPAT_EXTL2_BITNR, > + QCOW2_INCOMPAT_ZONED_FORMAT = 1 << QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, > > QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY > | QCOW2_INCOMPAT_CORRUPT > | QCOW2_INCOMPAT_DATA_FILE > | QCOW2_INCOMPAT_COMPRESSION > - | QCOW2_INCOMPAT_EXTL2, > + | QCOW2_INCOMPAT_EXTL2 > + | QCOW2_INCOMPAT_ZONED_FORMAT, > }; In the previous version, I had suggested an autoclear bit; but it looks like you went with a full-bore incompatible bit. What about the format of the disk makes it impossible to read an image if you don't know about zoned formats? Other incompat bits have obvious reasons: for example, you can't extract data from an extl2 if you don't know how to access the external data; and you can't uncompress an image with zstd if you don't have the compression header calling out that compression type. But so far, I haven't seen anything about how zone information makes the image unreadable by an earlier version of qemu. Do you have proper sanity checks that the incompat bit and the zone extension header are either both present or both absent? > > /* Compatible feature bits */ > @@ -285,7 +309,6 @@ enum { > QCOW2_AUTOCLEAR_DATA_FILE_RAW = 1 << QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR, > > QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS > - | QCOW2_AUTOCLEAR_DATA_FILE_RAW, > }; Why are you removing a bit from the autoclear mask? Did you experiment with making zoned devices an autoclear bit, and then change your mind to making it an incompatible bit instead? At any rate, this part of the hunk looks wrong. > > enum qcow2_discard_type { > @@ -422,6 +445,16 @@ typedef struct BDRVQcow2State { > * is to convert the image with the desired compression type set. > */ > Qcow2CompressionType compression_type; > + > + /* States of zoned device */ > + Qcow2ZonedHeaderExtension zoned_header; > + QLIST_HEAD(, Qcow2ZoneListEntry) exp_open_zones; > + QLIST_HEAD(, Qcow2ZoneListEntry) imp_open_zones; > + QLIST_HEAD(, Qcow2ZoneListEntry) closed_zones; > + Qcow2ZoneListEntry *zone_list_entries; > + uint32_t nr_zones_exp_open; > + uint32_t nr_zones_imp_open; > + uint32_t nr_zones_closed; > } BDRVQcow2State; > > typedef struct Qcow2COWRegion { > diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt > index 2c4618375a..b210bc4580 100644 > --- a/docs/interop/qcow2.txt > +++ b/docs/interop/qcow2.txt > @@ -125,7 +125,18 @@ the next fields through header_length. > allows subcluster-based allocation. See the > Extended L2 Entries section for more details. > > - Bits 5-63: Reserved (set to 0) > + Bit 5: Zoned extension bit. If this bit is set then > + the file is a zoned device file with > + host-managed model. > + > + It is unsafe when any qcow2 writer which does > + not understand zone information edits a file > + with the zoned extension. The write pointer > + tracking is corrupted between different version > + of qcow2 writes. In such cases, the file can > + no longer be seen as a zoned device. This wording sounds more like you want an autoclear bit than an incompat bit. An incompat bit implies that an image unaware of the zone extension cannot even open the device for reads (making it impossible to write and corrupt the zone information). > + > + Bits 6-63: Reserved (set to 0) > > 80 - 87: compatible_features > Bitmask of compatible features. An implementation can > @@ -249,6 +260,7 @@ be stored. Each extension has a structure like the following: > 0x23852875 - Bitmaps extension > 0x0537be77 - Full disk encryption header pointer > 0x44415441 - External data file name string > + 0x007a6264 - Zoned extension > other - Unknown header extension, can be safely > ignored > > @@ -331,6 +343,59 @@ The fields of the bitmaps extension are: > Offset into the image file at which the bitmap directory > starts. Must be aligned to a cluster boundary. > > +== Zoned extension == > + > +The zoned extension is an optional header extension. It contains fields for > +emulating the zoned stroage model (https://zonedstorage.io/). If you stick with an incompat bit, then this header should be mandatory when the incompat bit is set, and omitted when the incompat bit is clear. > + > +When the device model is not recognized as host-managed, it is regared as regarded > +incompatible and report an error to users. I'm not quite sure what you meant by a 'device model is not recognized as host-managed'. The phrase 'and report an error to users' does not seem to match anywhere else in the spec; I think what you are trying to state is that a given implementation must refuse to open a qcow2 file with the zone extension header containing a 'zoned' byte (the enum at offset 0) with a value it cannot support. > + > +The fields of the zoned extension are: > + Byte 0: zoned > + This bit represents zone model of devices. 1 is for > + host-managed, which only allows sequential writes. > + And 0 is for non-zoned devices without such constraints. Grammar suggestions, and it's nice to list values in order. How about: This bit represents the zone model of the device. 0 is for a non-zoned device (all other information in this header is ignored). 1 is for a host-managed device, which only allows for sequential writes within each zone. Other values may be added later; the implementation must refuse to open a device containing an unknown zone model. > + > + 1 - 3: Reserved, must be zero. > + > + 4 - 7: zone_size > + Total size of each zones, in bytes. It is less than 4GB each zone > + in the contained image for simplicity. Must this be a power of 2, and/or a multiple of the cluster size? Will we ever want to support making zones larger than 4G, in which case we need to plan on sizing this field bigger to begin with? > + > + 8 - 11: zone_capacity > + The number of writable bytes within the zones. > + A zone capacity is always smaller or equal to the > + zone size. I'm still not understanding why we need this separate from zone_size. > + > + 12 - 15: conventional_zones > + The number of conventional zones. Given that this is a spec, it is probably worth defining what a conventional zone is. I'm assuming it is a zone that does not have append-only semantics? > + > + 16 - 19: nr_zones > + The number of zones. It is the sum of conventional zones > + and sequential zones. Does the qcow2 implementation of zones guarantee that all conventional zones appear before any sequential zones? > + > + 20 - 23: max_active_zones > + The number of the zones that are in the implicit open, > + explicit open or closed state. s/are/can be/ > + > + 24 - 27: max_open_zones > + The maximal number of open (implicitly open or explicitly > + open) zones. What other states are there besides open and closed? If a closed zone is active, then when can a zone ever not be active? Is it required that max_open_zones <= max_active_zones <= nr_zones? > + > + 28 - 31: max_append_bytes > + The number of bytes of a zone append request that can be > + issued to the device. It must be 512-byte aligned. Here, you call out bytes; in the docs above you called out sectors. Make sure your patch is self-consistent. > + > + 32 - 39: zonedmeta_size > + The size of zoned metadata in bytes. It contains no more > + than 4GB. The zoned metadata structure is the write > + pointers for each zone. It contains no more than 4G, but yet has an 8-byte value. Why? > + > + 40 - 47: zonedmeta_offset > + The offset of zoned metadata structure in the contained > + image, in bytes. > + > == Full disk encryption header pointer == > > The full disk encryption header must be present if, and only if, the > diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h It feels like you are missing documentation on the zonedmeta cluster(s) - you've described it as being write pointers for each zone, but is it just 8 bytes per zone, taking up as many bytes as needed to cover all zones, with all remaining bytes in the cluster being zero padding? > +++ 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? > +# > +# @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"} } 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.
Hi Eric, Eric Blake <eblake@redhat.com> 于2023年10月30日周一 22:53写道： > > 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,... No... > > > --- > > ...place it here under the divider, so 'git am' won't include it, if there is nothing further to change on this patch. > > > block/qcow2.c | 205 ++++++++++++++++++++++++++++++- > > block/qcow2.h | 37 +++++- > > docs/interop/qcow2.txt | 67 +++++++++- > > include/block/block_int-common.h | 13 ++ > > qapi/block-core.json | 45 ++++++- > > 5 files changed, 362 insertions(+), 5 deletions(-) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index aa01d9e7b5..cd53268ca7 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, > > @@ -210,6 +211,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; > > @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > > break; > > } > > > > + case QCOW2_EXT_MAGIC_ZONED_FORMAT: > > + { > > + if (ext.len != sizeof(zoned_ext)) { > > + error_setg(errp, "zoned_ext: Invalid extension length"); > > + return -EINVAL; > > + } > > Do we ever anticipate the struct growing in size in the future to add > further features? Forcing the size to be constant, rather than a > minimum, may get in the way of clean upgrades to a future version of > the extension header. The zoned extension could grow. So ext.len > sizeof(zoned_ext) -> invalid. > > > + 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; > > + } > > + > > + if (s->incompatible_features & QCOW2_INCOMPAT_ZONED_FORMAT) { > > + warn_report("A program lacking zoned format support " > > + "may modify this file and zoned metadata are " > > + "now considered inconsistent"); > > + error_printf("The zoned metadata is corrupted.\n"); > > Why is this mixing warn_report and error_printf at the same time. > Also, grammar is inconsistent from the similar > QCOW2_AUTOCLEAR_BITMAPS, which used: > > if (s->qcow_version < 3) { > /* Let's be a bit more specific */ > warn_report("This qcow2 v2 image contains bitmaps, but " > "they may have been modified by a program " > "without persistent bitmap support; so now " > "they must all be considered inconsistent"); > } else { > warn_report("a program lacking bitmap support " > "modified this file, so all bitmaps are now " > "considered inconsistent"); > > This also raises the question whether we want to ever allow zoned > support with a v2 image, or whether it should just be a hard error if > it is not a v3 image (my preference would be the latter). > Actually, this part should be gotten rid of after discussion with Stefan. The incompatible feature bit of zoned format does not need to check if the zoned model is host-managed. It's a bit late for me. So I will respond to the rest of the comments later. Thanks for reviewing! Sam > > + } > > + > > + 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_to_cpu(zoned_ext.max_append_bytes); > > + s->zoned_header = zoned_ext; > > + > > + /* refuse to open broken images */ > > + if (zoned_ext.zone_size == 0) { > > + error_setg(errp, "Zoned extension header zone_size field " > > + "can not be 0"); > > + return -EINVAL; > > + } > > + if (zoned_ext.zone_capacity > zoned_ext.zone_size) { > > + error_setg(errp, "Zoned extension header zone_capacity field " > > + "can not be larger that zone_size field"); > > + return -EINVAL; > > + } > > + if (zoned_ext.nr_zones != DIV_ROUND_UP( > > + bs->total_sectors * BDRV_SECTOR_SIZE, zoned_ext.zone_size)) { > > + error_setg(errp, "Zoned extension header nr_zones field " > > + "is wrong"); > > + return -EINVAL; > > + } > > Are there any other sanity checks you should do, such as ensuring > max_open_zones <= the number of total possible zones once you divide > image size by zone size? > > Such sanity checks would also be useful when creating new image with > zones (and not just when opening a pre-existing image); should you > have a helper function that performs all the sanity checks for zone > values being self-consistent, and reuse it in each context, rather > than repeated open-coding the same checks? > > > + > > +#ifdef DEBUG_EXT > > + printf("Qcow2: Got zoned format extension: " > > + "offset=%" PRIu32 "\n", offset); > > +#endif > > + break; > > + } > > + > > default: > > /* unknown magic - save it in case we need to rewrite the header */ > > /* If you add a new feature, make sure to also update the fast > > +++ b/block/qcow2.h > > @@ -236,6 +236,27 @@ typedef struct Qcow2CryptoHeaderExtension { > > uint64_t length; > > } QEMU_PACKED Qcow2CryptoHeaderExtension; > > > > +typedef struct Qcow2ZonedHeaderExtension { > > + /* Zoned device attributes */ > > + uint8_t zoned; > > + uint8_t reserved[3]; > > + uint32_t zone_size; > > + uint32_t zone_capacity; > > + uint32_t conventional_zones; > > + uint32_t nr_zones; > > + uint32_t max_active_zones; > > + uint32_t max_open_zones; > > + uint32_t max_append_bytes; > > + uint64_t zonedmeta_size; > > + uint64_t zonedmeta_offset; > > +} QEMU_PACKED Qcow2ZonedHeaderExtension; > > Nice that everything is well-aligned so that the struct packs into 6 > consecutive 8-byte values. > > > + > > +typedef struct Qcow2ZoneListEntry { > > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; > > +} Qcow2ZoneListEntry; > > + > > typedef struct Qcow2UnknownHeaderExtension { > > uint32_t magic; > > uint32_t len; > > @@ -256,17 +277,20 @@ enum { > > QCOW2_INCOMPAT_DATA_FILE_BITNR = 2, > > QCOW2_INCOMPAT_COMPRESSION_BITNR = 3, > > QCOW2_INCOMPAT_EXTL2_BITNR = 4, > > + QCOW2_INCOMPAT_ZONED_FORMAT_BITNR = 5, > > QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR, > > QCOW2_INCOMPAT_CORRUPT = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR, > > QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, > > QCOW2_INCOMPAT_COMPRESSION = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR, > > QCOW2_INCOMPAT_EXTL2 = 1 << QCOW2_INCOMPAT_EXTL2_BITNR, > > + QCOW2_INCOMPAT_ZONED_FORMAT = 1 << QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, > > > > QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY > > | QCOW2_INCOMPAT_CORRUPT > > | QCOW2_INCOMPAT_DATA_FILE > > | QCOW2_INCOMPAT_COMPRESSION > > - | QCOW2_INCOMPAT_EXTL2, > > + | QCOW2_INCOMPAT_EXTL2 > > + | QCOW2_INCOMPAT_ZONED_FORMAT, > > }; > > In the previous version, I had suggested an autoclear bit; but it > looks like you went with a full-bore incompatible bit. What about the > format of the disk makes it impossible to read an image if you don't > know about zoned formats? Other incompat bits have obvious reasons: > for example, you can't extract data from an extl2 if you don't know > how to access the external data; and you can't uncompress an image > with zstd if you don't have the compression header calling out that > compression type. But so far, I haven't seen anything about how zone > information makes the image unreadable by an earlier version of qemu. > > Do you have proper sanity checks that the incompat bit and the zone > extension header are either both present or both absent? > > > > > /* Compatible feature bits */ > > @@ -285,7 +309,6 @@ enum { > > QCOW2_AUTOCLEAR_DATA_FILE_RAW = 1 << QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR, > > > > QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS > > - | QCOW2_AUTOCLEAR_DATA_FILE_RAW, > > }; > > Why are you removing a bit from the autoclear mask? Did you > experiment with making zoned devices an autoclear bit, and then change > your mind to making it an incompatible bit instead? At any rate, this > part of the hunk looks wrong. > > > > > enum qcow2_discard_type { > > @@ -422,6 +445,16 @@ typedef struct BDRVQcow2State { > > * is to convert the image with the desired compression type set. > > */ > > Qcow2CompressionType compression_type; > > + > > + /* States of zoned device */ > > + Qcow2ZonedHeaderExtension zoned_header; > > + QLIST_HEAD(, Qcow2ZoneListEntry) exp_open_zones; > > + QLIST_HEAD(, Qcow2ZoneListEntry) imp_open_zones; > > + QLIST_HEAD(, Qcow2ZoneListEntry) closed_zones; > > + Qcow2ZoneListEntry *zone_list_entries; > > + uint32_t nr_zones_exp_open; > > + uint32_t nr_zones_imp_open; > > + uint32_t nr_zones_closed; > > } BDRVQcow2State; > > > > typedef struct Qcow2COWRegion { > > diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt > > index 2c4618375a..b210bc4580 100644 > > --- a/docs/interop/qcow2.txt > > +++ b/docs/interop/qcow2.txt > > @@ -125,7 +125,18 @@ the next fields through header_length. > > allows subcluster-based allocation. See the > > Extended L2 Entries section for more details. > > > > - Bits 5-63: Reserved (set to 0) > > + Bit 5: Zoned extension bit. If this bit is set then > > + the file is a zoned device file with > > + host-managed model. > > + > > + It is unsafe when any qcow2 writer which does > > + not understand zone information edits a file > > + with the zoned extension. The write pointer > > + tracking is corrupted between different version > > + of qcow2 writes. In such cases, the file can > > + no longer be seen as a zoned device. > > This wording sounds more like you want an autoclear bit than an > incompat bit. An incompat bit implies that an image unaware of the > zone extension cannot even open the device for reads (making it > impossible to write and corrupt the zone information). > > > + > > + Bits 6-63: Reserved (set to 0) > > > > 80 - 87: compatible_features > > Bitmask of compatible features. An implementation can > > @@ -249,6 +260,7 @@ be stored. Each extension has a structure like the following: > > 0x23852875 - Bitmaps extension > > 0x0537be77 - Full disk encryption header pointer > > 0x44415441 - External data file name string > > + 0x007a6264 - Zoned extension > > other - Unknown header extension, can be safely > > ignored > > > > @@ -331,6 +343,59 @@ The fields of the bitmaps extension are: > > Offset into the image file at which the bitmap directory > > starts. Must be aligned to a cluster boundary. > > > > +== Zoned extension == > > + > > +The zoned extension is an optional header extension. It contains fields for > > +emulating the zoned stroage model (https://zonedstorage.io/). > > If you stick with an incompat bit, then this header should be > mandatory when the incompat bit is set, and omitted when the incompat > bit is clear. > > > + > > +When the device model is not recognized as host-managed, it is regared as > > regarded > > > +incompatible and report an error to users. > > I'm not quite sure what you meant by a 'device model is not recognized > as host-managed'. The phrase 'and report an error to users' does not > seem to match anywhere else in the spec; I think what you are trying > to state is that a given implementation must refuse to open a qcow2 > file with the zone extension header containing a 'zoned' byte (the > enum at offset 0) with a value it cannot support. > > > + > > +The fields of the zoned extension are: > > + Byte 0: zoned > > + This bit represents zone model of devices. 1 is for > > + host-managed, which only allows sequential writes. > > + And 0 is for non-zoned devices without such constraints. > > Grammar suggestions, and it's nice to list values in order. How about: > > This bit represents the zone model of the device. 0 is for a > non-zoned device (all other information in this header is ignored). 1 > is for a host-managed device, which only allows for sequential writes > within each zone. Other values may be added later; the implementation > must refuse to open a device containing an unknown zone model. > > > + > > + 1 - 3: Reserved, must be zero. > > + > > + 4 - 7: zone_size > > + Total size of each zones, in bytes. It is less than 4GB > > each zone > > > + in the contained image for simplicity. > > Must this be a power of 2, and/or a multiple of the cluster size? > Will we ever want to support making zones larger than 4G, in which > case we need to plan on sizing this field bigger to begin with? > > > + > > + 8 - 11: zone_capacity > > + The number of writable bytes within the zones. > > + A zone capacity is always smaller or equal to the > > + zone size. > > I'm still not understanding why we need this separate from zone_size. > > > + > > + 12 - 15: conventional_zones > > + The number of conventional zones. > > Given that this is a spec, it is probably worth defining what a > conventional zone is. I'm assuming it is a zone that does not have > append-only semantics? > > > + > > + 16 - 19: nr_zones > > + The number of zones. It is the sum of conventional zones > > + and sequential zones. > > Does the qcow2 implementation of zones guarantee that all conventional > zones appear before any sequential zones? > > > + > > + 20 - 23: max_active_zones > > + The number of the zones that are in the implicit open, > > + explicit open or closed state. > > s/are/can be/ > > > + > > + 24 - 27: max_open_zones > > + The maximal number of open (implicitly open or explicitly > > + open) zones. > > What other states are there besides open and closed? If a closed zone > is active, then when can a zone ever not be active? Is it required > that max_open_zones <= max_active_zones <= nr_zones? > > > + > > + 28 - 31: max_append_bytes > > + The number of bytes of a zone append request that can be > > + issued to the device. It must be 512-byte aligned. > > Here, you call out bytes; in the docs above you called out sectors. > Make sure your patch is self-consistent. > > > + > > + 32 - 39: zonedmeta_size > > + The size of zoned metadata in bytes. It contains no more > > + than 4GB. The zoned metadata structure is the write > > + pointers for each zone. > > It contains no more than 4G, but yet has an 8-byte value. Why? > > > + > > + 40 - 47: zonedmeta_offset > > + The offset of zoned metadata structure in the contained > > + image, in bytes. > > + > > == Full disk encryption header pointer == > > > > The full disk encryption header must be present if, and only if, the > > diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h > > It feels like you are missing documentation on the zonedmeta > cluster(s) - you've described it as being write pointers for each > zone, but is it just 8 bytes per zone, taking up as many bytes as > needed to cover all zones, with all remaining bytes in the cluster > being zero padding? > > > +++ 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? > > > +# > > +# @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"} } > > 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. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. > Virtualization: qemu.org | libguestfs.org >
On Mon, Oct 30, 2023 at 11:01:26PM +0800, Sam Li wrote: > Hi Eric, > > Eric Blake <eblake@redhat.com> 于2023年10月30日周一 22:53写道： > > > > 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,... > > No... > > > > > > --- > > > > ...place it here under the divider, so 'git am' won't include it, if there is nothing further to change on this patch. > > > > > block/qcow2.c | 205 ++++++++++++++++++++++++++++++- > > > block/qcow2.h | 37 +++++- > > > docs/interop/qcow2.txt | 67 +++++++++- > > > include/block/block_int-common.h | 13 ++ > > > qapi/block-core.json | 45 ++++++- > > > 5 files changed, 362 insertions(+), 5 deletions(-) > > > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > > index aa01d9e7b5..cd53268ca7 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, > > > @@ -210,6 +211,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; > > > @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > > > break; > > > } > > > > > > + case QCOW2_EXT_MAGIC_ZONED_FORMAT: > > > + { > > > + if (ext.len != sizeof(zoned_ext)) { > > > + error_setg(errp, "zoned_ext: Invalid extension length"); > > > + return -EINVAL; > > > + } > > > > Do we ever anticipate the struct growing in size in the future to add > > further features? Forcing the size to be constant, rather than a > > minimum, may get in the way of clean upgrades to a future version of > > the extension header. > > The zoned extension could grow. So ext.len > sizeof(zoned_ext) -> invalid. No, ext.len >= sizeof(zoned_ext) is valid. The program can extract the zoned_ext fields that it knows about. Any additional fields are ignored by the program because it doesn't know how to interpret them. ext.len < sizeof(zoned_ext) is invalid because required fields are missing. When zoned_ext is extended to add a new feature the first time, the code is updated like this: if (ext.len < endof(zoned_ext, last_minimum_field)) { ...invalid... } ...handle minimum zoned_ext fields... if (ext.len >= endof(zoned_ext, additional_field)) { ...handle additional_field... } ...more additional fields... This approach is often used by Linux ioctl(2) interfaces. It allows extensions to the struct without breaking old programs that still use shorter versions of the struct. Only optional features can be added using this approach, so it's often combined with a 'flags' field that indicates which mandatory features userspace wants and the kernel has provided. That way the kernel can reject ioctls that require a new feature that the old kernel doesn't know and the kernel can fill in flags upon returning from ioctl(2) so userspace knows which functionality was provided by the kernel. qcow2 has feature bits, so I don't think a 'flags' field is required. Stefan
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? > --- > block/qcow2.c | 205 ++++++++++++++++++++++++++++++- > block/qcow2.h | 37 +++++- > docs/interop/qcow2.txt | 67 +++++++++- > include/block/block_int-common.h | 13 ++ > qapi/block-core.json | 45 ++++++- > 5 files changed, 362 insertions(+), 5 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index aa01d9e7b5..cd53268ca7 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, > @@ -210,6 +211,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; > @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > break; > } > > + case QCOW2_EXT_MAGIC_ZONED_FORMAT: > + { > + if (ext.len != sizeof(zoned_ext)) { > + error_setg(errp, "zoned_ext: Invalid extension length"); > + 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; > + } > + > + if (s->incompatible_features & QCOW2_INCOMPAT_ZONED_FORMAT) { > + warn_report("A program lacking zoned format support " > + "may modify this file and zoned metadata are " > + "now considered inconsistent"); > + error_printf("The zoned metadata is corrupted.\n"); > + } This check is not necessary: the code already checks for unknown incompatible features (QCOW2_INCOMPAT_MASK) in qcow2_do_open(). > + > + 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_to_cpu(zoned_ext.max_append_bytes); > + s->zoned_header = zoned_ext; > + > + /* refuse to open broken images */ > + if (zoned_ext.zone_size == 0) { > + error_setg(errp, "Zoned extension header zone_size field " > + "can not be 0"); > + return -EINVAL; > + } > + if (zoned_ext.zone_capacity > zoned_ext.zone_size) { > + error_setg(errp, "Zoned extension header zone_capacity field " > + "can not be larger that zone_size field"); > + return -EINVAL; > + } > + if (zoned_ext.nr_zones != DIV_ROUND_UP( > + bs->total_sectors * BDRV_SECTOR_SIZE, zoned_ext.zone_size)) { > + error_setg(errp, "Zoned extension header nr_zones field " > + "is wrong"); > + return -EINVAL; > + } > + > +#ifdef DEBUG_EXT > + printf("Qcow2: Got zoned format extension: " > + "offset=%" PRIu32 "\n", offset); > +#endif > + break; > + } > + > default: > /* unknown magic - save it in case we need to rewrite the header */ > /* If you add a new feature, make sure to also update the fast > @@ -1967,6 +2026,15 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp) > } > bs->bl.pwrite_zeroes_alignment = s->subcluster_size; > bs->bl.pdiscard_alignment = s->cluster_size; > + bs->bl.zoned = s->zoned_header.zoned; > + bs->bl.nr_zones = s->zoned_header.nr_zones; > + bs->bl.max_append_sectors = s->zoned_header.max_append_bytes > + >> BDRV_SECTOR_BITS; > + bs->bl.max_active_zones = s->zoned_header.max_active_zones; > + bs->bl.max_open_zones = s->zoned_header.max_open_zones; > + bs->bl.zone_size = s->zoned_header.zone_size; > + bs->bl.zone_capacity = s->zoned_header.zone_capacity; > + bs->bl.write_granularity = BDRV_SECTOR_SIZE; > } > > static int GRAPH_UNLOCKED > @@ -3062,6 +3130,11 @@ int qcow2_update_header(BlockDriverState *bs) > .bit = QCOW2_INCOMPAT_EXTL2_BITNR, > .name = "extended L2 entries", > }, > + { > + .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, > + .bit = QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, > + .name = "zoned format", > + }, > { > .type = QCOW2_FEAT_TYPE_COMPATIBLE, > .bit = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR, > @@ -3107,6 +3180,31 @@ int qcow2_update_header(BlockDriverState *bs) > buflen -= ret; > } > > + /* Zoned devices header extension */ > + if (s->zoned_header.zoned == BLK_Z_HM) { > + Qcow2ZonedHeaderExtension zoned_header = { > + .zoned = s->zoned_header.zoned, > + .zone_size = cpu_to_be32(s->zoned_header.zone_size), > + .zone_capacity = cpu_to_be32(s->zoned_header.zone_capacity), > + .conventional_zones = > + cpu_to_be32(s->zoned_header.conventional_zones), > + .nr_zones = cpu_to_be32(s->zoned_header.nr_zones), > + .max_open_zones = cpu_to_be32(s->zoned_header.max_open_zones), > + .max_active_zones = > + cpu_to_be32(s->zoned_header.max_active_zones), > + .max_append_bytes = > + cpu_to_be32(s->zoned_header.max_append_bytes) > + }; > + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_ZONED_FORMAT, > + &zoned_header, sizeof(zoned_header), > + buflen); > + if (ret < 0) { > + goto fail; > + } > + buf += ret; > + buflen -= ret; > + } > + > /* Keep unknown header extensions */ > QLIST_FOREACH(uext, &s->unknown_header_ext, next) { > ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen); > @@ -3718,6 +3816,10 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) > header->incompatible_features |= > cpu_to_be64(QCOW2_INCOMPAT_DATA_FILE); > } > + if (qcow2_opts->zone_model != QCOW2_ZONE_MODEL_HOST_MANAGED) { Should != be ==? The incompatible feature bit must be set when the zone model is host-managed. > + header->incompatible_features |= > + cpu_to_be64(QCOW2_INCOMPAT_ZONED_FORMAT); > + } > if (qcow2_opts->data_file_raw) { > header->autoclear_features |= > cpu_to_be64(QCOW2_AUTOCLEAR_DATA_FILE_RAW); > @@ -3786,11 +3888,70 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) > } > > /* Set the external data file if necessary */ > + BDRVQcow2State *s = blk_bs(blk)->opaque; > if (data_bs) { > - BDRVQcow2State *s = blk_bs(blk)->opaque; > s->image_data_file = g_strdup(data_bs->filename); > } > > + if (qcow2_opts->zone_model == QCOW2_ZONE_MODEL_HOST_MANAGED) { > + if (!qcow2_opts->has_zone_size) { > + error_setg(errp, "Missing zone_size parameter"); > + ret = -EINVAL; > + goto out; > + } > + > + if (qcow2_opts->zone_size == 0) { > + s->zoned_header.zoned = BLK_Z_NONE; > + error_setg(errp, "Zoned devices can not allow a larger-than-zero " > + "zone_size"); This error message is confusing. The zone size must be larger than zero when the zone model is host-managed: error_setg(errp, "zone_size must be larger than zero"); > + ret = -EINVAL; > + goto out; > + } > + > + s->zoned_header.zoned = BLK_Z_HM; > + s->zoned_header.zone_size = qcow2_opts->zone_size; > + s->zoned_header.nr_zones = DIV_ROUND_UP(qcow2_opts->size, > + qcow2_opts->zone_size); > + > + if (qcow2_opts->has_zone_capacity) { > + if (qcow2_opts->zone_capacity > qcow2_opts->zone_size) { > + s->zoned_header.zoned = BLK_Z_NONE; > + error_setg(errp, "zone capacity %" PRIu64 "B exceeds zone size " > + "%" PRIu64"B", qcow2_opts->zone_capacity, > + qcow2_opts->zone_size); > + ret = -EINVAL; > + goto out; > + } > + s->zoned_header.zone_capacity = qcow2_opts->zone_capacity; > + } else { > + s->zoned_header.zone_capacity = qcow2_opts->zone_size; > + } > + > + if (qcow2_opts->has_conventional_zones) { > + s->zoned_header.conventional_zones = qcow2_opts->conventional_zones; > + } > + > + if (qcow2_opts->has_max_active_zones) { > + if (qcow2_opts->max_open_zones > qcow2_opts->max_active_zones) { > + s->zoned_header.zoned = BLK_Z_NONE; > + error_setg(errp, "max_open_zones %" PRIu32 " exceeds " > + "max_active_zones %" PRIu32"", > + qcow2_opts->max_open_zones, > + qcow2_opts->max_active_zones); > + ret = -EINVAL; > + goto out; > + } > + if (qcow2_opts->has_max_open_zones) { > + s->zoned_header.max_open_zones = qcow2_opts->max_active_zones; > + } else { > + s->zoned_header.max_open_zones = qcow2_opts->max_active_zones; > + } > + } > + s->zoned_header.max_append_bytes = qcow2_opts->max_append_bytes; > + } else { > + s->zoned_header.zoned = BLK_Z_NONE; > + } > + > /* Create a full header (including things like feature table) */ > ret = qcow2_update_header(blk_bs(blk)); > bdrv_graph_co_rdunlock(); > @@ -3921,6 +4082,13 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, > { BLOCK_OPT_COMPAT_LEVEL, "version" }, > { BLOCK_OPT_DATA_FILE_RAW, "data-file-raw" }, > { BLOCK_OPT_COMPRESSION_TYPE, "compression-type" }, > + { BLOCK_OPT_ZONE_MODEL, "zone-model"}, > + { BLOCK_OPT_CONVENTIONAL_ZONES, "conventional-zones"}, > + { BLOCK_OPT_MAX_OPEN_ZONES, "max-open-zones"}, > + { BLOCK_OPT_MAX_ACTIVE_ZONES, "max-active-zones"}, > + { BLOCK_OPT_MAX_APPEND_BYTES, "max-append-bytes"}, > + { BLOCK_OPT_ZONE_SIZE, "zone-size"}, > + { BLOCK_OPT_ZONE_CAPACITY, "zone-capacity"}, > { NULL, NULL }, > }; > > @@ -6087,6 +6255,41 @@ static QemuOptsList qcow2_create_opts = { > .help = "Compression method used for image cluster " \ > "compression", \ > .def_value_str = "zlib" \ > + }, \ > + { \ > + .name = BLOCK_OPT_ZONE_MODEL, \ > + .type = QEMU_OPT_STRING, \ > + .help = "zone model", \ > + }, \ > + { \ > + .name = BLOCK_OPT_ZONE_SIZE, \ > + .type = QEMU_OPT_SIZE, \ > + .help = "zone size", \ > + }, \ > + { \ > + .name = BLOCK_OPT_ZONE_CAPACITY, \ > + .type = QEMU_OPT_SIZE, \ > + .help = "zone capacity", \ > + }, \ > + { \ > + .name = BLOCK_OPT_CONVENTIONAL_ZONES, \ > + .type = QEMU_OPT_NUMBER, \ > + .help = "numbers of conventional zones", \ > + }, \ > + { \ > + .name = BLOCK_OPT_MAX_APPEND_BYTES, \ > + .type = QEMU_OPT_NUMBER, \ > + .help = "max append bytes", \ > + }, \ > + { \ > + .name = BLOCK_OPT_MAX_ACTIVE_ZONES, \ > + .type = QEMU_OPT_NUMBER, \ > + .help = "max active zones", \ > + }, \ > + { \ > + .name = BLOCK_OPT_MAX_OPEN_ZONES, \ > + .type = QEMU_OPT_NUMBER, \ > + .help = "max open zones", \ > }, > QCOW_COMMON_OPTIONS, > { /* end of list */ } > diff --git a/block/qcow2.h b/block/qcow2.h > index 29958c512b..6e5d90afd2 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -236,6 +236,27 @@ typedef struct Qcow2CryptoHeaderExtension { > uint64_t length; > } QEMU_PACKED Qcow2CryptoHeaderExtension; > > +typedef struct Qcow2ZonedHeaderExtension { > + /* Zoned device attributes */ > + uint8_t zoned; > + uint8_t reserved[3]; > + uint32_t zone_size; > + uint32_t zone_capacity; > + uint32_t conventional_zones; > + uint32_t nr_zones; > + uint32_t max_active_zones; > + uint32_t max_open_zones; > + uint32_t max_append_bytes; > + uint64_t zonedmeta_size; > + uint64_t zonedmeta_offset; > +} QEMU_PACKED Qcow2ZonedHeaderExtension; > + > +typedef struct Qcow2ZoneListEntry { > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; > +} Qcow2ZoneListEntry; > + > typedef struct Qcow2UnknownHeaderExtension { > uint32_t magic; > uint32_t len; > @@ -256,17 +277,20 @@ enum { > QCOW2_INCOMPAT_DATA_FILE_BITNR = 2, > QCOW2_INCOMPAT_COMPRESSION_BITNR = 3, > QCOW2_INCOMPAT_EXTL2_BITNR = 4, > + QCOW2_INCOMPAT_ZONED_FORMAT_BITNR = 5, > QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR, > QCOW2_INCOMPAT_CORRUPT = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR, > QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, > QCOW2_INCOMPAT_COMPRESSION = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR, > QCOW2_INCOMPAT_EXTL2 = 1 << QCOW2_INCOMPAT_EXTL2_BITNR, > + QCOW2_INCOMPAT_ZONED_FORMAT = 1 << QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, > > QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY > | QCOW2_INCOMPAT_CORRUPT > | QCOW2_INCOMPAT_DATA_FILE > | QCOW2_INCOMPAT_COMPRESSION > - | QCOW2_INCOMPAT_EXTL2, > + | QCOW2_INCOMPAT_EXTL2 > + | QCOW2_INCOMPAT_ZONED_FORMAT, > }; > > /* Compatible feature bits */ > @@ -285,7 +309,6 @@ enum { > QCOW2_AUTOCLEAR_DATA_FILE_RAW = 1 << QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR, > > QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS > - | QCOW2_AUTOCLEAR_DATA_FILE_RAW, What is the purpose of this change? > }; > > enum qcow2_discard_type { > @@ -422,6 +445,16 @@ typedef struct BDRVQcow2State { > * is to convert the image with the desired compression type set. > */ > Qcow2CompressionType compression_type; > + > + /* States of zoned device */ > + Qcow2ZonedHeaderExtension zoned_header; > + QLIST_HEAD(, Qcow2ZoneListEntry) exp_open_zones; > + QLIST_HEAD(, Qcow2ZoneListEntry) imp_open_zones; > + QLIST_HEAD(, Qcow2ZoneListEntry) closed_zones; > + Qcow2ZoneListEntry *zone_list_entries; > + uint32_t nr_zones_exp_open; > + uint32_t nr_zones_imp_open; > + uint32_t nr_zones_closed; > } BDRVQcow2State; > > typedef struct Qcow2COWRegion { > diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt > index 2c4618375a..b210bc4580 100644 > --- a/docs/interop/qcow2.txt > +++ b/docs/interop/qcow2.txt > @@ -125,7 +125,18 @@ the next fields through header_length. > allows subcluster-based allocation. See the > Extended L2 Entries section for more details. > > - Bits 5-63: Reserved (set to 0) > + Bit 5: Zoned extension bit. If this bit is set then > + the file is a zoned device file with > + host-managed model. > + > + It is unsafe when any qcow2 writer which does > + not understand zone information edits a file > + with the zoned extension. The write pointer > + tracking is corrupted between different version > + of qcow2 writes. In such cases, the file can > + no longer be seen as a zoned device. Here is a suggestion for rephrasing this: Bit 5: Zoned emulation bit. If this bit is set then the file is an emulated zoned device. The Zoned extension must be present. Implementations that do not support zoned emulation cannot open this file because it generally only makes sense to interpret the data along with the zone information and write pointers. Don't mention the host-managed model in case other zone models are supported in the future. > + > + Bits 6-63: Reserved (set to 0) > > 80 - 87: compatible_features > Bitmask of compatible features. An implementation can > @@ -249,6 +260,7 @@ be stored. Each extension has a structure like the following: > 0x23852875 - Bitmaps extension > 0x0537be77 - Full disk encryption header pointer > 0x44415441 - External data file name string > + 0x007a6264 - Zoned extension > other - Unknown header extension, can be safely > ignored > > @@ -331,6 +343,59 @@ The fields of the bitmaps extension are: > Offset into the image file at which the bitmap directory > starts. Must be aligned to a cluster boundary. > > +== Zoned extension == > + > +The zoned extension is an optional header extension. It contains fields for > +emulating the zoned stroage model (https://zonedstorage.io/). s/stroage/storage/ > + > +When the device model is not recognized as host-managed, it is regared as s/regared/regarded/ > +incompatible and report an error to users. > + > +The fields of the zoned extension are: > + Byte 0: zoned > + This bit represents zone model of devices. 1 is for > + host-managed, which only allows sequential writes. > + And 0 is for non-zoned devices without such constraints. > + > + 1 - 3: Reserved, must be zero. > + > + 4 - 7: zone_size > + Total size of each zones, in bytes. It is less than 4GB > + in the contained image for simplicity. > + > + 8 - 11: zone_capacity > + The number of writable bytes within the zones. > + A zone capacity is always smaller or equal to the > + zone size. > + > + 12 - 15: conventional_zones > + The number of conventional zones. > + > + 16 - 19: nr_zones > + The number of zones. It is the sum of conventional zones > + and sequential zones. > + > + 20 - 23: max_active_zones > + The number of the zones that are in the implicit open, > + explicit open or closed state. > + > + 24 - 27: max_open_zones > + The maximal number of open (implicitly open or explicitly > + open) zones. > + > + 28 - 31: max_append_bytes > + The number of bytes of a zone append request that can be > + issued to the device. It must be 512-byte aligned. > + > + 32 - 39: zonedmeta_size > + The size of zoned metadata in bytes. It contains no more > + than 4GB. The zoned metadata structure is the write > + pointers for each zone. > + > + 40 - 47: zonedmeta_offset > + The offset of zoned metadata structure in the contained > + image, in bytes. > + > == Full disk encryption header pointer == > > The full disk encryption header must be present if, and only if, the > diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h > index b8d9d24f39..4b94e55eb4 100644 > --- a/include/block/block_int-common.h > +++ b/include/block/block_int-common.h > @@ -57,6 +57,13 @@ > #define BLOCK_OPT_DATA_FILE_RAW "data_file_raw" > #define BLOCK_OPT_COMPRESSION_TYPE "compression_type" > #define BLOCK_OPT_EXTL2 "extended_l2" > +#define BLOCK_OPT_ZONE_MODEL "zone_model" > +#define BLOCK_OPT_ZONE_SIZE "zone_size" > +#define BLOCK_OPT_ZONE_CAPACITY "zone_capacity" > +#define BLOCK_OPT_CONVENTIONAL_ZONES "conventional_zones" > +#define BLOCK_OPT_MAX_APPEND_BYTES "max_append_bytes" > +#define BLOCK_OPT_MAX_ACTIVE_ZONES "max_active_zones" > +#define BLOCK_OPT_MAX_OPEN_ZONES "max_open_zones" > > #define BLOCK_PROBE_BUF_SIZE 512 > > @@ -883,6 +890,12 @@ typedef struct BlockLimits { > /* zone size expressed in bytes */ > uint32_t zone_size; > > + /* > + * the number of usable logical blocks within the zone, expressed > + * in bytes. A zone capacity is smaller or equal to the zone size. > + */ > + uint32_t zone_capacity; > + > /* total number of zones */ > uint32_t nr_zones; > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 89751d81f2..884e058046 100644 > --- a/qapi/block-core.json > +++ 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) > +# > +# @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' } } > > ## > # @BlockdevCreateOptionsQed: > -- > 2.40.1 >
On Mon, Oct 30, 2023 at 08:18:45PM +0800, Sam Li wrote: > +typedef struct Qcow2ZoneListEntry { > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; Where is closed_zone_entry used?
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.
Stefan Hajnoczi <stefanha@redhat.com> 于2023年11月3日周五 11:24写道： > > On Mon, Oct 30, 2023 at 08:18:45PM +0800, Sam Li wrote: > > +typedef struct Qcow2ZoneListEntry { > > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; > > Where is closed_zone_entry used? When the number of implicitly open zones are reaching the max implicitly open zone and one implicitly open zone is closed, it will add one closed zone to closed_zone_entry. (Will be in the next version)
Markus Armbruster <armbru@redhat.com> 于2023年11月3日周五 17:08写道： > > 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. Yes, it should use a QMP union. It's better to separate those fields for zoned and non-zoned. > > >> +# > >> +# @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. > Yes, it's better. Thanks! Sam
Hi Eric, Eric Blake <eblake@redhat.com> 于2023年10月30日周一 22:53写道： > > 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. > > > block/qcow2.c | 205 ++++++++++++++++++++++++++++++- > > block/qcow2.h | 37 +++++- > > docs/interop/qcow2.txt | 67 +++++++++- > > include/block/block_int-common.h | 13 ++ > > qapi/block-core.json | 45 ++++++- > > 5 files changed, 362 insertions(+), 5 deletions(-) > > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index aa01d9e7b5..cd53268ca7 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, > > @@ -210,6 +211,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; > > @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, > > break; > > } > > > > + case QCOW2_EXT_MAGIC_ZONED_FORMAT: > > + { > > + if (ext.len != sizeof(zoned_ext)) { > > + error_setg(errp, "zoned_ext: Invalid extension length"); > > + return -EINVAL; > > + } > > Do we ever anticipate the struct growing in size in the future to add > further features? Forcing the size to be constant, rather than a > minimum, may get in the way of clean upgrades to a future version of > the extension header. > > > + 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; > > + } > > + > > + if (s->incompatible_features & QCOW2_INCOMPAT_ZONED_FORMAT) { > > + warn_report("A program lacking zoned format support " > > + "may modify this file and zoned metadata are " > > + "now considered inconsistent"); > > + error_printf("The zoned metadata is corrupted.\n"); > > Why is this mixing warn_report and error_printf at the same time. > Also, grammar is inconsistent from the similar > QCOW2_AUTOCLEAR_BITMAPS, which used: > > if (s->qcow_version < 3) { > /* Let's be a bit more specific */ > warn_report("This qcow2 v2 image contains bitmaps, but " > "they may have been modified by a program " > "without persistent bitmap support; so now " > "they must all be considered inconsistent"); > } else { > warn_report("a program lacking bitmap support " > "modified this file, so all bitmaps are now " > "considered inconsistent"); > > This also raises the question whether we want to ever allow zoned > support with a v2 image, or whether it should just be a hard error if > it is not a v3 image (my preference would be the latter). > > > + } > > + > > + 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_to_cpu(zoned_ext.max_append_bytes); > > + s->zoned_header = zoned_ext; > > + > > + /* refuse to open broken images */ > > + if (zoned_ext.zone_size == 0) { > > + error_setg(errp, "Zoned extension header zone_size field " > > + "can not be 0"); > > + return -EINVAL; > > + } > > + if (zoned_ext.zone_capacity > zoned_ext.zone_size) { > > + error_setg(errp, "Zoned extension header zone_capacity field " > > + "can not be larger that zone_size field"); > > + return -EINVAL; > > + } > > + if (zoned_ext.nr_zones != DIV_ROUND_UP( > > + bs->total_sectors * BDRV_SECTOR_SIZE, zoned_ext.zone_size)) { > > + error_setg(errp, "Zoned extension header nr_zones field " > > + "is wrong"); > > + return -EINVAL; > > + } > > Are there any other sanity checks you should do, such as ensuring > max_open_zones <= the number of total possible zones once you divide > image size by zone size? > > Such sanity checks would also be useful when creating new image with > zones (and not just when opening a pre-existing image); should you > have a helper function that performs all the sanity checks for zone > values being self-consistent, and reuse it in each context, rather > than repeated open-coding the same checks? > > > + > > +#ifdef DEBUG_EXT > > + printf("Qcow2: Got zoned format extension: " > > + "offset=%" PRIu32 "\n", offset); > > +#endif > > + break; > > + } > > + > > default: > > /* unknown magic - save it in case we need to rewrite the header */ > > /* If you add a new feature, make sure to also update the fast > > +++ b/block/qcow2.h > > @@ -236,6 +236,27 @@ typedef struct Qcow2CryptoHeaderExtension { > > uint64_t length; > > } QEMU_PACKED Qcow2CryptoHeaderExtension; > > > > +typedef struct Qcow2ZonedHeaderExtension { > > + /* Zoned device attributes */ > > + uint8_t zoned; > > + uint8_t reserved[3]; > > + uint32_t zone_size; > > + uint32_t zone_capacity; > > + uint32_t conventional_zones; > > + uint32_t nr_zones; > > + uint32_t max_active_zones; > > + uint32_t max_open_zones; > > + uint32_t max_append_bytes; > > + uint64_t zonedmeta_size; > > + uint64_t zonedmeta_offset; > > +} QEMU_PACKED Qcow2ZonedHeaderExtension; > > Nice that everything is well-aligned so that the struct packs into 6 > consecutive 8-byte values. > > > + > > +typedef struct Qcow2ZoneListEntry { > > + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; > > + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; > > +} Qcow2ZoneListEntry; > > + > > typedef struct Qcow2UnknownHeaderExtension { > > uint32_t magic; > > uint32_t len; > > @@ -256,17 +277,20 @@ enum { > > QCOW2_INCOMPAT_DATA_FILE_BITNR = 2, > > QCOW2_INCOMPAT_COMPRESSION_BITNR = 3, > > QCOW2_INCOMPAT_EXTL2_BITNR = 4, > > + QCOW2_INCOMPAT_ZONED_FORMAT_BITNR = 5, > > QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR, > > QCOW2_INCOMPAT_CORRUPT = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR, > > QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, > > QCOW2_INCOMPAT_COMPRESSION = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR, > > QCOW2_INCOMPAT_EXTL2 = 1 << QCOW2_INCOMPAT_EXTL2_BITNR, > > + QCOW2_INCOMPAT_ZONED_FORMAT = 1 << QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, > > > > QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY > > | QCOW2_INCOMPAT_CORRUPT > > | QCOW2_INCOMPAT_DATA_FILE > > | QCOW2_INCOMPAT_COMPRESSION > > - | QCOW2_INCOMPAT_EXTL2, > > + | QCOW2_INCOMPAT_EXTL2 > > + | QCOW2_INCOMPAT_ZONED_FORMAT, > > }; > > In the previous version, I had suggested an autoclear bit; but it > looks like you went with a full-bore incompatible bit. What about the > format of the disk makes it impossible to read an image if you don't > know about zoned formats? Other incompat bits have obvious reasons: > for example, you can't extract data from an extl2 if you don't know > how to access the external data; and you can't uncompress an image > with zstd if you don't have the compression header calling out that > compression type. But so far, I haven't seen anything about how zone > information makes the image unreadable by an earlier version of qemu. > > Do you have proper sanity checks that the incompat bit and the zone > extension header are either both present or both absent? > > > > > /* Compatible feature bits */ > > @@ -285,7 +309,6 @@ enum { > > QCOW2_AUTOCLEAR_DATA_FILE_RAW = 1 << QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR, > > > > QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS > > - | QCOW2_AUTOCLEAR_DATA_FILE_RAW, > > }; > > Why are you removing a bit from the autoclear mask? Did you > experiment with making zoned devices an autoclear bit, and then change > your mind to making it an incompatible bit instead? At any rate, this > part of the hunk looks wrong. > > > > > enum qcow2_discard_type { > > @@ -422,6 +445,16 @@ typedef struct BDRVQcow2State { > > * is to convert the image with the desired compression type set. > > */ > > Qcow2CompressionType compression_type; > > + > > + /* States of zoned device */ > > + Qcow2ZonedHeaderExtension zoned_header; > > + QLIST_HEAD(, Qcow2ZoneListEntry) exp_open_zones; > > + QLIST_HEAD(, Qcow2ZoneListEntry) imp_open_zones; > > + QLIST_HEAD(, Qcow2ZoneListEntry) closed_zones; > > + Qcow2ZoneListEntry *zone_list_entries; > > + uint32_t nr_zones_exp_open; > > + uint32_t nr_zones_imp_open; > > + uint32_t nr_zones_closed; > > } BDRVQcow2State; > > > > typedef struct Qcow2COWRegion { > > diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt > > index 2c4618375a..b210bc4580 100644 > > --- a/docs/interop/qcow2.txt > > +++ b/docs/interop/qcow2.txt > > @@ -125,7 +125,18 @@ the next fields through header_length. > > allows subcluster-based allocation. See the > > Extended L2 Entries section for more details. > > > > - Bits 5-63: Reserved (set to 0) > > + Bit 5: Zoned extension bit. If this bit is set then > > + the file is a zoned device file with > > + host-managed model. > > + > > + It is unsafe when any qcow2 writer which does > > + not understand zone information edits a file > > + with the zoned extension. The write pointer > > + tracking is corrupted between different version > > + of qcow2 writes. In such cases, the file can > > + no longer be seen as a zoned device. > > This wording sounds more like you want an autoclear bit than an > incompat bit. An incompat bit implies that an image unaware of the > zone extension cannot even open the device for reads (making it > impossible to write and corrupt the zone information). > > > + > > + Bits 6-63: Reserved (set to 0) > > > > 80 - 87: compatible_features > > Bitmask of compatible features. An implementation can > > @@ -249,6 +260,7 @@ be stored. Each extension has a structure like the following: > > 0x23852875 - Bitmaps extension > > 0x0537be77 - Full disk encryption header pointer > > 0x44415441 - External data file name string > > + 0x007a6264 - Zoned extension > > other - Unknown header extension, can be safely > > ignored > > > > @@ -331,6 +343,59 @@ The fields of the bitmaps extension are: > > Offset into the image file at which the bitmap directory > > starts. Must be aligned to a cluster boundary. > > > > +== Zoned extension == > > + > > +The zoned extension is an optional header extension. It contains fields for > > +emulating the zoned stroage model (https://zonedstorage.io/). > > If you stick with an incompat bit, then this header should be > mandatory when the incompat bit is set, and omitted when the incompat > bit is clear. > > > + > > +When the device model is not recognized as host-managed, it is regared as > > regarded > > > +incompatible and report an error to users. > > I'm not quite sure what you meant by a 'device model is not recognized > as host-managed'. The phrase 'and report an error to users' does not > seem to match anywhere else in the spec; I think what you are trying > to state is that a given implementation must refuse to open a qcow2 > file with the zone extension header containing a 'zoned' byte (the > enum at offset 0) with a value it cannot support. > > > + > > +The fields of the zoned extension are: > > + Byte 0: zoned > > + This bit represents zone model of devices. 1 is for > > + host-managed, which only allows sequential writes. > > + And 0 is for non-zoned devices without such constraints. > > Grammar suggestions, and it's nice to list values in order. How about: > > This bit represents the zone model of the device. 0 is for a > non-zoned device (all other information in this header is ignored). 1 > is for a host-managed device, which only allows for sequential writes > within each zone. Other values may be added later; the implementation > must refuse to open a device containing an unknown zone model. > > > + > > + 1 - 3: Reserved, must be zero. > > + > > + 4 - 7: zone_size > > + Total size of each zones, in bytes. It is less than 4GB > > each zone > > > + in the contained image for simplicity. > > Must this be a power of 2, and/or a multiple of the cluster size? > Will we ever want to support making zones larger than 4G, in which > case we need to plan on sizing this field bigger to begin with? > > > + > > + 8 - 11: zone_capacity > > + The number of writable bytes within the zones. > > + A zone capacity is always smaller or equal to the > > + zone size. > > I'm still not understanding why we need this separate from zone_size. > > > + > > + 12 - 15: conventional_zones > > + The number of conventional zones. > > Given that this is a spec, it is probably worth defining what a > conventional zone is. I'm assuming it is a zone that does not have > append-only semantics? Ok. The conventional zones refer to zones that allow sequential writes and random writes. While the sequential zones only allows sequential writes. > > > + > > + 16 - 19: nr_zones > > + The number of zones. It is the sum of conventional zones > > + and sequential zones. > > Does the qcow2 implementation of zones guarantee that all conventional > zones appear before any sequential zones? Yes. The conventional zones are first. > > > + > > + 20 - 23: max_active_zones > > + The number of the zones that are in the implicit open, > > + explicit open or closed state. > > s/are/can be/ > > > + > > + 24 - 27: max_open_zones > > + The maximal number of open (implicitly open or explicitly > > + open) zones. > > What other states are there besides open and closed? If a closed zone > is active, then when can a zone ever not be active? Is it required > that max_open_zones <= max_active_zones <= nr_zones? The other states are empty and full (read only and offline states are device-internal events and not considered in qcow2 emulation). When opening a qcow2 emulated file, the closed zones are kept active. It is mandatorily required but implicitly, yes. > > > + > > + 28 - 31: max_append_bytes > > + The number of bytes of a zone append request that can be > > + issued to the device. It must be 512-byte aligned. > > Here, you call out bytes; in the docs above you called out sectors. > Make sure your patch is self-consistent. It's bytes here. Sorry about that. I will check the consistency whenever sending patches again. > > > + > > + 32 - 39: zonedmeta_size > > + The size of zoned metadata in bytes. It contains no more > > + than 4GB. The zoned metadata structure is the write > > + pointers for each zone. > > It contains no more than 4G, but yet has an 8-byte value. Why? The zone size and the number of zones are 4-byte values. The zoned meta is nr_zones * zone_size. Therefore, it is an 8-byte value. Will clarify this more clearly in v6. > > > + > > + 40 - 47: zonedmeta_offset > > + The offset of zoned metadata structure in the contained > > + image, in bytes. > > + > > == Full disk encryption header pointer == > > > > The full disk encryption header must be present if, and only if, the > > diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h > > It feels like you are missing documentation on the zonedmeta > cluster(s) - you've described it as being write pointers for each > zone, but is it just 8 bytes per zone, taking up as many bytes as > needed to cover all zones, with all remaining bytes in the cluster > being zero padding? Ok, I will add it. Yes, it's just 8 bytes per zone for all zones. The remaining bytes in the cluster are not zero padding. > > > +++ 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? > > > +# > > +# @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"} } > > 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. I see. Thanks! > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. > Virtualization: qemu.org | libguestfs.org >
diff --git a/block/qcow2.c b/block/qcow2.c index aa01d9e7b5..cd53268ca7 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, @@ -210,6 +211,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; @@ -431,6 +433,63 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, break; } + case QCOW2_EXT_MAGIC_ZONED_FORMAT: + { + if (ext.len != sizeof(zoned_ext)) { + error_setg(errp, "zoned_ext: Invalid extension length"); + 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; + } + + if (s->incompatible_features & QCOW2_INCOMPAT_ZONED_FORMAT) { + warn_report("A program lacking zoned format support " + "may modify this file and zoned metadata are " + "now considered inconsistent"); + error_printf("The zoned metadata is corrupted.\n"); + } + + 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_to_cpu(zoned_ext.max_append_bytes); + s->zoned_header = zoned_ext; + + /* refuse to open broken images */ + if (zoned_ext.zone_size == 0) { + error_setg(errp, "Zoned extension header zone_size field " + "can not be 0"); + return -EINVAL; + } + if (zoned_ext.zone_capacity > zoned_ext.zone_size) { + error_setg(errp, "Zoned extension header zone_capacity field " + "can not be larger that zone_size field"); + return -EINVAL; + } + if (zoned_ext.nr_zones != DIV_ROUND_UP( + bs->total_sectors * BDRV_SECTOR_SIZE, zoned_ext.zone_size)) { + error_setg(errp, "Zoned extension header nr_zones field " + "is wrong"); + return -EINVAL; + } + +#ifdef DEBUG_EXT + printf("Qcow2: Got zoned format extension: " + "offset=%" PRIu32 "\n", offset); +#endif + break; + } + default: /* unknown magic - save it in case we need to rewrite the header */ /* If you add a new feature, make sure to also update the fast @@ -1967,6 +2026,15 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp) } bs->bl.pwrite_zeroes_alignment = s->subcluster_size; bs->bl.pdiscard_alignment = s->cluster_size; + bs->bl.zoned = s->zoned_header.zoned; + bs->bl.nr_zones = s->zoned_header.nr_zones; + bs->bl.max_append_sectors = s->zoned_header.max_append_bytes + >> BDRV_SECTOR_BITS; + bs->bl.max_active_zones = s->zoned_header.max_active_zones; + bs->bl.max_open_zones = s->zoned_header.max_open_zones; + bs->bl.zone_size = s->zoned_header.zone_size; + bs->bl.zone_capacity = s->zoned_header.zone_capacity; + bs->bl.write_granularity = BDRV_SECTOR_SIZE; } static int GRAPH_UNLOCKED @@ -3062,6 +3130,11 @@ int qcow2_update_header(BlockDriverState *bs) .bit = QCOW2_INCOMPAT_EXTL2_BITNR, .name = "extended L2 entries", }, + { + .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, + .bit = QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, + .name = "zoned format", + }, { .type = QCOW2_FEAT_TYPE_COMPATIBLE, .bit = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR, @@ -3107,6 +3180,31 @@ int qcow2_update_header(BlockDriverState *bs) buflen -= ret; } + /* Zoned devices header extension */ + if (s->zoned_header.zoned == BLK_Z_HM) { + Qcow2ZonedHeaderExtension zoned_header = { + .zoned = s->zoned_header.zoned, + .zone_size = cpu_to_be32(s->zoned_header.zone_size), + .zone_capacity = cpu_to_be32(s->zoned_header.zone_capacity), + .conventional_zones = + cpu_to_be32(s->zoned_header.conventional_zones), + .nr_zones = cpu_to_be32(s->zoned_header.nr_zones), + .max_open_zones = cpu_to_be32(s->zoned_header.max_open_zones), + .max_active_zones = + cpu_to_be32(s->zoned_header.max_active_zones), + .max_append_bytes = + cpu_to_be32(s->zoned_header.max_append_bytes) + }; + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_ZONED_FORMAT, + &zoned_header, sizeof(zoned_header), + buflen); + if (ret < 0) { + goto fail; + } + buf += ret; + buflen -= ret; + } + /* Keep unknown header extensions */ QLIST_FOREACH(uext, &s->unknown_header_ext, next) { ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen); @@ -3718,6 +3816,10 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) header->incompatible_features |= cpu_to_be64(QCOW2_INCOMPAT_DATA_FILE); } + if (qcow2_opts->zone_model != QCOW2_ZONE_MODEL_HOST_MANAGED) { + header->incompatible_features |= + cpu_to_be64(QCOW2_INCOMPAT_ZONED_FORMAT); + } if (qcow2_opts->data_file_raw) { header->autoclear_features |= cpu_to_be64(QCOW2_AUTOCLEAR_DATA_FILE_RAW); @@ -3786,11 +3888,70 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) } /* Set the external data file if necessary */ + BDRVQcow2State *s = blk_bs(blk)->opaque; if (data_bs) { - BDRVQcow2State *s = blk_bs(blk)->opaque; s->image_data_file = g_strdup(data_bs->filename); } + if (qcow2_opts->zone_model == QCOW2_ZONE_MODEL_HOST_MANAGED) { + if (!qcow2_opts->has_zone_size) { + error_setg(errp, "Missing zone_size parameter"); + ret = -EINVAL; + goto out; + } + + if (qcow2_opts->zone_size == 0) { + s->zoned_header.zoned = BLK_Z_NONE; + error_setg(errp, "Zoned devices can not allow a larger-than-zero " + "zone_size"); + ret = -EINVAL; + goto out; + } + + s->zoned_header.zoned = BLK_Z_HM; + s->zoned_header.zone_size = qcow2_opts->zone_size; + s->zoned_header.nr_zones = DIV_ROUND_UP(qcow2_opts->size, + qcow2_opts->zone_size); + + if (qcow2_opts->has_zone_capacity) { + if (qcow2_opts->zone_capacity > qcow2_opts->zone_size) { + s->zoned_header.zoned = BLK_Z_NONE; + error_setg(errp, "zone capacity %" PRIu64 "B exceeds zone size " + "%" PRIu64"B", qcow2_opts->zone_capacity, + qcow2_opts->zone_size); + ret = -EINVAL; + goto out; + } + s->zoned_header.zone_capacity = qcow2_opts->zone_capacity; + } else { + s->zoned_header.zone_capacity = qcow2_opts->zone_size; + } + + if (qcow2_opts->has_conventional_zones) { + s->zoned_header.conventional_zones = qcow2_opts->conventional_zones; + } + + if (qcow2_opts->has_max_active_zones) { + if (qcow2_opts->max_open_zones > qcow2_opts->max_active_zones) { + s->zoned_header.zoned = BLK_Z_NONE; + error_setg(errp, "max_open_zones %" PRIu32 " exceeds " + "max_active_zones %" PRIu32"", + qcow2_opts->max_open_zones, + qcow2_opts->max_active_zones); + ret = -EINVAL; + goto out; + } + if (qcow2_opts->has_max_open_zones) { + s->zoned_header.max_open_zones = qcow2_opts->max_active_zones; + } else { + s->zoned_header.max_open_zones = qcow2_opts->max_active_zones; + } + } + s->zoned_header.max_append_bytes = qcow2_opts->max_append_bytes; + } else { + s->zoned_header.zoned = BLK_Z_NONE; + } + /* Create a full header (including things like feature table) */ ret = qcow2_update_header(blk_bs(blk)); bdrv_graph_co_rdunlock(); @@ -3921,6 +4082,13 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts, { BLOCK_OPT_COMPAT_LEVEL, "version" }, { BLOCK_OPT_DATA_FILE_RAW, "data-file-raw" }, { BLOCK_OPT_COMPRESSION_TYPE, "compression-type" }, + { BLOCK_OPT_ZONE_MODEL, "zone-model"}, + { BLOCK_OPT_CONVENTIONAL_ZONES, "conventional-zones"}, + { BLOCK_OPT_MAX_OPEN_ZONES, "max-open-zones"}, + { BLOCK_OPT_MAX_ACTIVE_ZONES, "max-active-zones"}, + { BLOCK_OPT_MAX_APPEND_BYTES, "max-append-bytes"}, + { BLOCK_OPT_ZONE_SIZE, "zone-size"}, + { BLOCK_OPT_ZONE_CAPACITY, "zone-capacity"}, { NULL, NULL }, }; @@ -6087,6 +6255,41 @@ static QemuOptsList qcow2_create_opts = { .help = "Compression method used for image cluster " \ "compression", \ .def_value_str = "zlib" \ + }, \ + { \ + .name = BLOCK_OPT_ZONE_MODEL, \ + .type = QEMU_OPT_STRING, \ + .help = "zone model", \ + }, \ + { \ + .name = BLOCK_OPT_ZONE_SIZE, \ + .type = QEMU_OPT_SIZE, \ + .help = "zone size", \ + }, \ + { \ + .name = BLOCK_OPT_ZONE_CAPACITY, \ + .type = QEMU_OPT_SIZE, \ + .help = "zone capacity", \ + }, \ + { \ + .name = BLOCK_OPT_CONVENTIONAL_ZONES, \ + .type = QEMU_OPT_NUMBER, \ + .help = "numbers of conventional zones", \ + }, \ + { \ + .name = BLOCK_OPT_MAX_APPEND_BYTES, \ + .type = QEMU_OPT_NUMBER, \ + .help = "max append bytes", \ + }, \ + { \ + .name = BLOCK_OPT_MAX_ACTIVE_ZONES, \ + .type = QEMU_OPT_NUMBER, \ + .help = "max active zones", \ + }, \ + { \ + .name = BLOCK_OPT_MAX_OPEN_ZONES, \ + .type = QEMU_OPT_NUMBER, \ + .help = "max open zones", \ }, QCOW_COMMON_OPTIONS, { /* end of list */ } diff --git a/block/qcow2.h b/block/qcow2.h index 29958c512b..6e5d90afd2 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -236,6 +236,27 @@ typedef struct Qcow2CryptoHeaderExtension { uint64_t length; } QEMU_PACKED Qcow2CryptoHeaderExtension; +typedef struct Qcow2ZonedHeaderExtension { + /* Zoned device attributes */ + uint8_t zoned; + uint8_t reserved[3]; + uint32_t zone_size; + uint32_t zone_capacity; + uint32_t conventional_zones; + uint32_t nr_zones; + uint32_t max_active_zones; + uint32_t max_open_zones; + uint32_t max_append_bytes; + uint64_t zonedmeta_size; + uint64_t zonedmeta_offset; +} QEMU_PACKED Qcow2ZonedHeaderExtension; + +typedef struct Qcow2ZoneListEntry { + QLIST_ENTRY(Qcow2ZoneListEntry) exp_open_zone_entry; + QLIST_ENTRY(Qcow2ZoneListEntry) imp_open_zone_entry; + QLIST_ENTRY(Qcow2ZoneListEntry) closed_zone_entry; +} Qcow2ZoneListEntry; + typedef struct Qcow2UnknownHeaderExtension { uint32_t magic; uint32_t len; @@ -256,17 +277,20 @@ enum { QCOW2_INCOMPAT_DATA_FILE_BITNR = 2, QCOW2_INCOMPAT_COMPRESSION_BITNR = 3, QCOW2_INCOMPAT_EXTL2_BITNR = 4, + QCOW2_INCOMPAT_ZONED_FORMAT_BITNR = 5, QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR, QCOW2_INCOMPAT_CORRUPT = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR, QCOW2_INCOMPAT_DATA_FILE = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR, QCOW2_INCOMPAT_COMPRESSION = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR, QCOW2_INCOMPAT_EXTL2 = 1 << QCOW2_INCOMPAT_EXTL2_BITNR, + QCOW2_INCOMPAT_ZONED_FORMAT = 1 << QCOW2_INCOMPAT_ZONED_FORMAT_BITNR, QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY | QCOW2_INCOMPAT_CORRUPT | QCOW2_INCOMPAT_DATA_FILE | QCOW2_INCOMPAT_COMPRESSION - | QCOW2_INCOMPAT_EXTL2, + | QCOW2_INCOMPAT_EXTL2 + | QCOW2_INCOMPAT_ZONED_FORMAT, }; /* Compatible feature bits */ @@ -285,7 +309,6 @@ enum { QCOW2_AUTOCLEAR_DATA_FILE_RAW = 1 << QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR, QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS - | QCOW2_AUTOCLEAR_DATA_FILE_RAW, }; enum qcow2_discard_type { @@ -422,6 +445,16 @@ typedef struct BDRVQcow2State { * is to convert the image with the desired compression type set. */ Qcow2CompressionType compression_type; + + /* States of zoned device */ + Qcow2ZonedHeaderExtension zoned_header; + QLIST_HEAD(, Qcow2ZoneListEntry) exp_open_zones; + QLIST_HEAD(, Qcow2ZoneListEntry) imp_open_zones; + QLIST_HEAD(, Qcow2ZoneListEntry) closed_zones; + Qcow2ZoneListEntry *zone_list_entries; + uint32_t nr_zones_exp_open; + uint32_t nr_zones_imp_open; + uint32_t nr_zones_closed; } BDRVQcow2State; typedef struct Qcow2COWRegion { diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt index 2c4618375a..b210bc4580 100644 --- a/docs/interop/qcow2.txt +++ b/docs/interop/qcow2.txt @@ -125,7 +125,18 @@ the next fields through header_length. allows subcluster-based allocation. See the Extended L2 Entries section for more details. - Bits 5-63: Reserved (set to 0) + Bit 5: Zoned extension bit. If this bit is set then + the file is a zoned device file with + host-managed model. + + It is unsafe when any qcow2 writer which does + not understand zone information edits a file + with the zoned extension. The write pointer + tracking is corrupted between different version + of qcow2 writes. In such cases, the file can + no longer be seen as a zoned device. + + Bits 6-63: Reserved (set to 0) 80 - 87: compatible_features Bitmask of compatible features. An implementation can @@ -249,6 +260,7 @@ be stored. Each extension has a structure like the following: 0x23852875 - Bitmaps extension 0x0537be77 - Full disk encryption header pointer 0x44415441 - External data file name string + 0x007a6264 - Zoned extension other - Unknown header extension, can be safely ignored @@ -331,6 +343,59 @@ The fields of the bitmaps extension are: Offset into the image file at which the bitmap directory starts. Must be aligned to a cluster boundary. +== Zoned extension == + +The zoned extension is an optional header extension. It contains fields for +emulating the zoned stroage model (https://zonedstorage.io/). + +When the device model is not recognized as host-managed, it is regared as +incompatible and report an error to users. + +The fields of the zoned extension are: + Byte 0: zoned + This bit represents zone model of devices. 1 is for + host-managed, which only allows sequential writes. + And 0 is for non-zoned devices without such constraints. + + 1 - 3: Reserved, must be zero. + + 4 - 7: zone_size + Total size of each zones, in bytes. It is less than 4GB + in the contained image for simplicity. + + 8 - 11: zone_capacity + The number of writable bytes within the zones. + A zone capacity is always smaller or equal to the + zone size. + + 12 - 15: conventional_zones + The number of conventional zones. + + 16 - 19: nr_zones + The number of zones. It is the sum of conventional zones + and sequential zones. + + 20 - 23: max_active_zones + The number of the zones that are in the implicit open, + explicit open or closed state. + + 24 - 27: max_open_zones + The maximal number of open (implicitly open or explicitly + open) zones. + + 28 - 31: max_append_bytes + The number of bytes of a zone append request that can be + issued to the device. It must be 512-byte aligned. + + 32 - 39: zonedmeta_size + The size of zoned metadata in bytes. It contains no more + than 4GB. The zoned metadata structure is the write + pointers for each zone. + + 40 - 47: zonedmeta_offset + The offset of zoned metadata structure in the contained + image, in bytes. + == Full disk encryption header pointer == The full disk encryption header must be present if, and only if, the diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index b8d9d24f39..4b94e55eb4 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -57,6 +57,13 @@ #define BLOCK_OPT_DATA_FILE_RAW "data_file_raw" #define BLOCK_OPT_COMPRESSION_TYPE "compression_type" #define BLOCK_OPT_EXTL2 "extended_l2" +#define BLOCK_OPT_ZONE_MODEL "zone_model" +#define BLOCK_OPT_ZONE_SIZE "zone_size" +#define BLOCK_OPT_ZONE_CAPACITY "zone_capacity" +#define BLOCK_OPT_CONVENTIONAL_ZONES "conventional_zones" +#define BLOCK_OPT_MAX_APPEND_BYTES "max_append_bytes" +#define BLOCK_OPT_MAX_ACTIVE_ZONES "max_active_zones" +#define BLOCK_OPT_MAX_OPEN_ZONES "max_open_zones" #define BLOCK_PROBE_BUF_SIZE 512 @@ -883,6 +890,12 @@ typedef struct BlockLimits { /* zone size expressed in bytes */ uint32_t zone_size; + /* + * the number of usable logical blocks within the zone, expressed + * in bytes. A zone capacity is smaller or equal to the zone size. + */ + uint32_t zone_capacity; + /* total number of zones */ uint32_t nr_zones; diff --git a/qapi/block-core.json b/qapi/block-core.json index 89751d81f2..884e058046 100644 --- a/qapi/block-core.json +++ 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) +# +# @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' } } ## # @BlockdevCreateOptionsQed:
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? --- block/qcow2.c | 205 ++++++++++++++++++++++++++++++- block/qcow2.h | 37 +++++- docs/interop/qcow2.txt | 67 +++++++++- include/block/block_int-common.h | 13 ++ qapi/block-core.json | 45 ++++++- 5 files changed, 362 insertions(+), 5 deletions(-)