To configure the zoned format feature on the qcow2 driver, it
requires following arguments: the device size, zoned profile,
zoned model, zone size, zone capacity, number of conventional
zones, limits on zone resources (max append sectors, 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 zone_nr_conv=0 -o
max_append_sectors=512 -o max_open_zones=0 -o max_active_zones=0
-o zoned_profile=zbc
Signed-off-by: Sam Li <faithilikerun@gmail.com>
---
block/qcow2.c | 119 +++++++++++++++++++++++++++++++
block/qcow2.h | 21 ++++++
include/block/block-common.h | 5 ++
include/block/block_int-common.h | 8 +++
qapi/block-core.json | 46 ++++++++----
5 files changed, 185 insertions(+), 14 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 7f3948360d..b886dab42b 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 0x7a6264
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,37 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
break;
}
+ case QCOW2_EXT_MAGIC_ZONED_FORMAT:
+ {
+ if (ext.len != sizeof(zoned_ext)) {
+ error_setg_errno(errp, -ret, "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;
+ }
+
+ zoned_ext.zone_size = be32_to_cpu(zoned_ext.zone_size);
+ zoned_ext.nr_zones = be32_to_cpu(zoned_ext.nr_zones);
+ zoned_ext.zone_nr_conv = be32_to_cpu(zoned_ext.zone_nr_conv);
+ 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_sectors =
+ be32_to_cpu(zoned_ext.max_append_sectors);
+ s->zoned_header = zoned_ext;
+
+#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
@@ -3071,6 +3104,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_profile = s->zoned_header.zoned_profile,
+ .zoned = s->zoned_header.zoned,
+ .nr_zones = cpu_to_be32(s->zoned_header.nr_zones),
+ .zone_size = cpu_to_be32(s->zoned_header.zone_size),
+ .zone_capacity = cpu_to_be32(s->zoned_header.zone_capacity),
+ .zone_nr_conv = cpu_to_be32(s->zoned_header.zone_nr_conv),
+ .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_sectors =
+ cpu_to_be32(s->zoned_header.max_append_sectors)
+ };
+ 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);
@@ -3755,6 +3813,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
s->image_data_file = g_strdup(data_bs->filename);
}
+ if (!strcmp(qcow2_opts->zoned_profile, "zbc")) {
+ BDRVQcow2State *s = blk_bs(blk)->opaque;
+ s->zoned_header.zoned_profile = BLK_ZP_ZBC;
+ s->zoned_header.zoned = BLK_Z_HM;
+ s->zoned_header.zone_size = qcow2_opts->zone_size;
+ s->zoned_header.zone_capacity = qcow2_opts->zone_capacity;
+ s->zoned_header.zone_nr_conv = qcow2_opts->zone_nr_conv;
+ s->zoned_header.max_open_zones = qcow2_opts->max_open_zones;
+ s->zoned_header.max_active_zones = qcow2_opts->max_active_zones;
+ s->zoned_header.max_append_sectors = qcow2_opts->max_append_sectors;
+ }
+
/* Create a full header (including things like feature table) */
ret = qcow2_update_header(blk_bs(blk));
bdrv_graph_co_rdunlock();
@@ -3873,6 +3943,13 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts,
qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v3");
}
+ /* The available zoned-profile options are zbc, which stands for
+ * ZBC/ZAC standards, and zns following NVMe ZNS spec. */
+ val = qdict_get_try_str(qdict, BLOCK_OPT_Z_PROFILE);
+ if (val) {
+ qdict_put_str(qdict, BLOCK_OPT_Z_PROFILE, val);
+ }
+
/* Change legacy command line options into QMP ones */
static const QDictRenames opt_renames[] = {
{ BLOCK_OPT_BACKING_FILE, "backing-file" },
@@ -3885,6 +3962,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_Z_PROFILE, "zoned-profile"},
+ { BLOCK_OPT_Z_NR_COV, "zone-nr-conv"},
+ { BLOCK_OPT_Z_MOZ, "max-open-zones"},
+ { BLOCK_OPT_Z_MAZ, "max-active-zones"},
+ { BLOCK_OPT_Z_MAS, "max-append-sectors"},
+ { BLOCK_OPT_Z_SIZE, "zone-size"},
+ { BLOCK_OPT_Z_CAP, "zone-capacity"},
{ NULL, NULL },
};
@@ -6048,6 +6132,41 @@ static QemuOptsList qcow2_create_opts = {
.help = "Compression method used for image cluster " \
"compression", \
.def_value_str = "zlib" \
+ }, \
+ {
+ .name = BLOCK_OPT_Z_PROFILE, \
+ .type = QEMU_OPT_STRING, \
+ .help = "zoned format option for the disk img", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_SIZE, \
+ .type = QEMU_OPT_SIZE, \
+ .help = "zone size", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_CAP, \
+ .type = QEMU_OPT_SIZE, \
+ .help = "zone capacity", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_NR_COV, \
+ .type = QEMU_OPT_NUMBER, \
+ .help = "numbers of conventional zones", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_MAS, \
+ .type = QEMU_OPT_NUMBER, \
+ .help = "max append sectors", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_MAZ, \
+ .type = QEMU_OPT_NUMBER, \
+ .help = "max active zones", \
+ }, \
+ { \
+ .name = BLOCK_OPT_Z_MOZ, \
+ .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 4f67eb912a..fe18dc4d97 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
uint64_t length;
} QEMU_PACKED Qcow2CryptoHeaderExtension;
+typedef struct Qcow2ZonedHeaderExtension {
+ /* Zoned device attributes */
+ BlockZonedProfile zoned_profile;
+ BlockZoneModel zoned;
+ uint32_t zone_size;
+ uint32_t zone_capacity;
+ uint32_t nr_zones;
+ uint32_t zone_nr_conv;
+ uint32_t max_active_zones;
+ uint32_t max_open_zones;
+ uint32_t max_append_sectors;
+ uint8_t padding[3];
+} QEMU_PACKED Qcow2ZonedHeaderExtension;
+
typedef struct Qcow2UnknownHeaderExtension {
uint32_t magic;
uint32_t len;
@@ -419,6 +433,13 @@ typedef struct BDRVQcow2State {
* is to convert the image with the desired compression type set.
*/
Qcow2CompressionType compression_type;
+
+ /* States of zoned device */
+ Qcow2ZonedHeaderExtension zoned_header;
+ uint32_t nr_zones_exp_open;
+ uint32_t nr_zones_imp_open;
+ uint32_t nr_zones_closed;
+ BlockZoneWps *wps;
} BDRVQcow2State;
typedef struct Qcow2COWRegion {
diff --git a/include/block/block-common.h b/include/block/block-common.h
index e15395f2cb..9f04a772f6 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -108,6 +108,11 @@ typedef enum BlockZoneType {
BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
} BlockZoneType;
+typedef enum BlockZonedProfile {
+ BLK_ZP_ZBC = 0x1,
+ BLK_ZP_ZNS = 0x2,
+} BlockZonedProfile;
+
/*
* Zone descriptor data structure.
* Provides information on a zone with all position and size values in bytes.
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 74195c3004..4be35feaf8 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -57,6 +57,14 @@
#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_Z_PROFILE "zoned_profile"
+#define BLOCK_OPT_Z_MODEL "zoned"
+#define BLOCK_OPT_Z_SIZE "zone_size"
+#define BLOCK_OPT_Z_CAP "zone_capacity"
+#define BLOCK_OPT_Z_NR_COV "zone_nr_conv"
+#define BLOCK_OPT_Z_MAS "max_append_sectors"
+#define BLOCK_OPT_Z_MAZ "max_active_zones"
+#define BLOCK_OPT_Z_MOZ "max_open_zones"
#define BLOCK_PROBE_BUF_SIZE 512
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4bf89171c6..f9a584cbb3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5013,24 +5013,42 @@
#
# @compression-type: The image cluster compression method
# (default: zlib, since 5.1)
+# @zoned-profile: Two zoned device protocol options, zbc or zns
+# (default: off, since 8.0)
+# @zone-size: The size of a zone of the zoned device (since 8.0)
+# @zone-capacity: The capacity of a zone of the zoned device (since 8.0)
+# @zone-nr-conv: The number of conventional zones of the zoned device
+# (since 8.0)
+# @max-open-zones: The maximal allowed open zones (since 8.0)
+# @max-active-zones: The limit of the zones that have the implicit open,
+# explicit open or closed state (since 8.0)
+# @max-append-sectors: The maximal sectors that is allowed to append write
+# (since 8.0)
#
# Since: 2.12
##
{ 'struct': 'BlockdevCreateOptionsQcow2',
- 'data': { 'file': 'BlockdevRef',
- '*data-file': 'BlockdevRef',
- '*data-file-raw': 'bool',
- '*extended-l2': 'bool',
- 'size': 'size',
- '*version': 'BlockdevQcow2Version',
- '*backing-file': 'str',
- '*backing-fmt': 'BlockdevDriver',
- '*encrypt': 'QCryptoBlockCreateOptions',
- '*cluster-size': 'size',
- '*preallocation': 'PreallocMode',
- '*lazy-refcounts': 'bool',
- '*refcount-bits': 'int',
- '*compression-type':'Qcow2CompressionType' } }
+ 'data': { 'file': 'BlockdevRef',
+ '*data-file': 'BlockdevRef',
+ '*data-file-raw': 'bool',
+ '*extended-l2': 'bool',
+ 'size': 'size',
+ '*version': 'BlockdevQcow2Version',
+ '*backing-file': 'str',
+ '*backing-fmt': 'BlockdevDriver',
+ '*encrypt': 'QCryptoBlockCreateOptions',
+ '*cluster-size': 'size',
+ '*preallocation': 'PreallocMode',
+ '*lazy-refcounts': 'bool',
+ '*refcount-bits': 'int',
+ '*compression-type': 'Qcow2CompressionType',
+ '*zoned-profile': 'str',
+ '*zone-size': 'size',
+ '*zone-capacity': 'size',
+ '*zone-nr-conv': 'uint32',
+ '*max-open-zones': 'uint32',
+ '*max-active-zones': 'uint32',
+ '*max-append-sectors': 'uint32'}}
##
# @BlockdevCreateOptionsQed:
--
2.40.1On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> To configure the zoned format feature on the qcow2 driver, it
> requires following arguments: the device size, zoned profile,
> zoned model, zone size, zone capacity, number of conventional
> zones, limits on zone resources (max append sectors, 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 zone_nr_conv=0 -o
> max_append_sectors=512 -o max_open_zones=0 -o max_active_zones=0
> -o zoned_profile=zbc
>
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
> block/qcow2.c | 119 +++++++++++++++++++++++++++++++
> block/qcow2.h | 21 ++++++
> include/block/block-common.h | 5 ++
> include/block/block_int-common.h | 8 +++
> qapi/block-core.json | 46 ++++++++----
> 5 files changed, 185 insertions(+), 14 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7f3948360d..b886dab42b 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 0x7a6264
>
> 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,37 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> break;
> }
>
> + case QCOW2_EXT_MAGIC_ZONED_FORMAT:
> + {
> + if (ext.len != sizeof(zoned_ext)) {
> + error_setg_errno(errp, -ret, "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;
> + }
> +
> + zoned_ext.zone_size = be32_to_cpu(zoned_ext.zone_size);
> + zoned_ext.nr_zones = be32_to_cpu(zoned_ext.nr_zones);
> + zoned_ext.zone_nr_conv = be32_to_cpu(zoned_ext.zone_nr_conv);
> + 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_sectors =
> + be32_to_cpu(zoned_ext.max_append_sectors);
> + s->zoned_header = zoned_ext;
Please validate these values. The image file is not trusted and may be
broken/corrupt. For example, zone_size=0 and nr_zones=0 must be rejected
because the code can't do anything useful when these values are zero
(similar for values that are not multiples of the block size).
> +
> +#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
> @@ -3071,6 +3104,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_profile = s->zoned_header.zoned_profile,
> + .zoned = s->zoned_header.zoned,
> + .nr_zones = cpu_to_be32(s->zoned_header.nr_zones),
> + .zone_size = cpu_to_be32(s->zoned_header.zone_size),
> + .zone_capacity = cpu_to_be32(s->zoned_header.zone_capacity),
> + .zone_nr_conv = cpu_to_be32(s->zoned_header.zone_nr_conv),
> + .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_sectors =
> + cpu_to_be32(s->zoned_header.max_append_sectors)
> + };
> + 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);
> @@ -3755,6 +3813,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> s->image_data_file = g_strdup(data_bs->filename);
> }
>
> + if (!strcmp(qcow2_opts->zoned_profile, "zbc")) {
> + BDRVQcow2State *s = blk_bs(blk)->opaque;
> + s->zoned_header.zoned_profile = BLK_ZP_ZBC;
> + s->zoned_header.zoned = BLK_Z_HM;
> + s->zoned_header.zone_size = qcow2_opts->zone_size;
> + s->zoned_header.zone_capacity = qcow2_opts->zone_capacity;
> + s->zoned_header.zone_nr_conv = qcow2_opts->zone_nr_conv;
> + s->zoned_header.max_open_zones = qcow2_opts->max_open_zones;
> + s->zoned_header.max_active_zones = qcow2_opts->max_active_zones;
> + s->zoned_header.max_append_sectors = qcow2_opts->max_append_sectors;
Where is the presence of these optional qcow2_opts checked? For example,
if the user didn't specify zone_size, then they cannot create an image
with a zoned profile.
These options also need to be validated to ensure that they contain
reasonable values (e.g. not 0).
> + }
> +
> /* Create a full header (including things like feature table) */
> ret = qcow2_update_header(blk_bs(blk));
> bdrv_graph_co_rdunlock();
> @@ -3873,6 +3943,13 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts,
> qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v3");
> }
>
> + /* The available zoned-profile options are zbc, which stands for
> + * ZBC/ZAC standards, and zns following NVMe ZNS spec. */
> + val = qdict_get_try_str(qdict, BLOCK_OPT_Z_PROFILE);
> + if (val) {
> + qdict_put_str(qdict, BLOCK_OPT_Z_PROFILE, val);
> + }
What is the purpose of this code, it fetches and replaces the same qdict
element?
> +
> /* Change legacy command line options into QMP ones */
> static const QDictRenames opt_renames[] = {
> { BLOCK_OPT_BACKING_FILE, "backing-file" },
> @@ -3885,6 +3962,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_Z_PROFILE, "zoned-profile"},
> + { BLOCK_OPT_Z_NR_COV, "zone-nr-conv"},
> + { BLOCK_OPT_Z_MOZ, "max-open-zones"},
> + { BLOCK_OPT_Z_MAZ, "max-active-zones"},
> + { BLOCK_OPT_Z_MAS, "max-append-sectors"},
> + { BLOCK_OPT_Z_SIZE, "zone-size"},
> + { BLOCK_OPT_Z_CAP, "zone-capacity"},
> { NULL, NULL },
> };
>
> @@ -6048,6 +6132,41 @@ static QemuOptsList qcow2_create_opts = {
> .help = "Compression method used for image cluster " \
> "compression", \
> .def_value_str = "zlib" \
> + }, \
> + {
The forward slash ('\') that wraps the line is missing and indentation
is off.
> + .name = BLOCK_OPT_Z_PROFILE, \
> + .type = QEMU_OPT_STRING, \
> + .help = "zoned format option for the disk img", \
> + }, \
> + { \
Indentation is off.
> + .name = BLOCK_OPT_Z_SIZE, \
> + .type = QEMU_OPT_SIZE, \
> + .help = "zone size", \
> + }, \
> + { \
> + .name = BLOCK_OPT_Z_CAP, \
> + .type = QEMU_OPT_SIZE, \
> + .help = "zone capacity", \
> + }, \
> + { \
> + .name = BLOCK_OPT_Z_NR_COV, \
Indentation is off.
> + .type = QEMU_OPT_NUMBER, \
> + .help = "numbers of conventional zones", \
> + }, \
> + { \
> + .name = BLOCK_OPT_Z_MAS, \
> + .type = QEMU_OPT_NUMBER, \
> + .help = "max append sectors", \
> + }, \
> + { \
> + .name = BLOCK_OPT_Z_MAZ, \
> + .type = QEMU_OPT_NUMBER, \
> + .help = "max active zones", \
> + }, \
> + { \
> + .name = BLOCK_OPT_Z_MOZ, \
> + .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 4f67eb912a..fe18dc4d97 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> uint64_t length;
> } QEMU_PACKED Qcow2CryptoHeaderExtension;
>
> +typedef struct Qcow2ZonedHeaderExtension {
> + /* Zoned device attributes */
> + BlockZonedProfile zoned_profile;
> + BlockZoneModel zoned;
> + uint32_t zone_size;
> + uint32_t zone_capacity;
> + uint32_t nr_zones;
> + uint32_t zone_nr_conv;
> + uint32_t max_active_zones;
> + uint32_t max_open_zones;
> + uint32_t max_append_sectors;
> + uint8_t padding[3];
This looks strange. Why is there 3 bytes of padding at the end? Normally
padding would align to an even power-of-two number of bytes like 2, 4,
8, etc.
> +} QEMU_PACKED Qcow2ZonedHeaderExtension;
> +
> typedef struct Qcow2UnknownHeaderExtension {
> uint32_t magic;
> uint32_t len;
> @@ -419,6 +433,13 @@ typedef struct BDRVQcow2State {
> * is to convert the image with the desired compression type set.
> */
> Qcow2CompressionType compression_type;
> +
> + /* States of zoned device */
> + Qcow2ZonedHeaderExtension zoned_header;
> + uint32_t nr_zones_exp_open;
> + uint32_t nr_zones_imp_open;
> + uint32_t nr_zones_closed;
> + BlockZoneWps *wps;
Normally qcow2 code passes bs around, so it should be possible to access
the wps pointer without duplicating it here. This new field is not used
in this patch, so I can't tell yet how important it is. It's safer to
avoid duplicating pointers when the original pointer can be accessed
conveniently so that use-after-free, double-free, and similar memory
management bugs can be eliminated.
> } BDRVQcow2State;
>
> typedef struct Qcow2COWRegion {
> diff --git a/include/block/block-common.h b/include/block/block-common.h
> index e15395f2cb..9f04a772f6 100644
> --- a/include/block/block-common.h
> +++ b/include/block/block-common.h
> @@ -108,6 +108,11 @@ typedef enum BlockZoneType {
> BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> } BlockZoneType;
>
> +typedef enum BlockZonedProfile {
> + BLK_ZP_ZBC = 0x1,
> + BLK_ZP_ZNS = 0x2,
> +} BlockZonedProfile;
> +
> /*
> * Zone descriptor data structure.
> * Provides information on a zone with all position and size values in bytes.
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index 74195c3004..4be35feaf8 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -57,6 +57,14 @@
> #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_Z_PROFILE "zoned_profile"
> +#define BLOCK_OPT_Z_MODEL "zoned"
> +#define BLOCK_OPT_Z_SIZE "zone_size"
> +#define BLOCK_OPT_Z_CAP "zone_capacity"
> +#define BLOCK_OPT_Z_NR_COV "zone_nr_conv"
> +#define BLOCK_OPT_Z_MAS "max_append_sectors"
> +#define BLOCK_OPT_Z_MAZ "max_active_zones"
> +#define BLOCK_OPT_Z_MOZ "max_open_zones"
>
> #define BLOCK_PROBE_BUF_SIZE 512
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4bf89171c6..f9a584cbb3 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -5013,24 +5013,42 @@
> #
> # @compression-type: The image cluster compression method
> # (default: zlib, since 5.1)
> +# @zoned-profile: Two zoned device protocol options, zbc or zns
> +# (default: off, since 8.0)
> +# @zone-size: The size of a zone of the zoned device (since 8.0)
> +# @zone-capacity: The capacity of a zone of the zoned device (since 8.0)
> +# @zone-nr-conv: The number of conventional zones of the zoned device
> +# (since 8.0)
> +# @max-open-zones: The maximal allowed open zones (since 8.0)
> +# @max-active-zones: The limit of the zones that have the implicit open,
> +# explicit open or closed state (since 8.0)
> +# @max-append-sectors: The maximal sectors that is allowed to append write
> +# (since 8.0)
Since 8.1.
> #
> # Since: 2.12
> ##
> { 'struct': 'BlockdevCreateOptionsQcow2',
> - 'data': { 'file': 'BlockdevRef',
> - '*data-file': 'BlockdevRef',
> - '*data-file-raw': 'bool',
> - '*extended-l2': 'bool',
> - 'size': 'size',
> - '*version': 'BlockdevQcow2Version',
> - '*backing-file': 'str',
> - '*backing-fmt': 'BlockdevDriver',
> - '*encrypt': 'QCryptoBlockCreateOptions',
> - '*cluster-size': 'size',
> - '*preallocation': 'PreallocMode',
> - '*lazy-refcounts': 'bool',
> - '*refcount-bits': 'int',
> - '*compression-type':'Qcow2CompressionType' } }
> + 'data': { 'file': 'BlockdevRef',
> + '*data-file': 'BlockdevRef',
> + '*data-file-raw': 'bool',
> + '*extended-l2': 'bool',
> + 'size': 'size',
> + '*version': 'BlockdevQcow2Version',
> + '*backing-file': 'str',
> + '*backing-fmt': 'BlockdevDriver',
> + '*encrypt': 'QCryptoBlockCreateOptions',
> + '*cluster-size': 'size',
> + '*preallocation': 'PreallocMode',
> + '*lazy-refcounts': 'bool',
> + '*refcount-bits': 'int',
> + '*compression-type': 'Qcow2CompressionType',
> + '*zoned-profile': 'str',
> + '*zone-size': 'size',
> + '*zone-capacity': 'size',
> + '*zone-nr-conv': 'uint32',
> + '*max-open-zones': 'uint32',
> + '*max-active-zones': 'uint32',
> + '*max-append-sectors': 'uint32'}}
>
> ##
> # @BlockdevCreateOptionsQed:
> --
> 2.40.1
>
Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道：
>
> On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > To configure the zoned format feature on the qcow2 driver, it
> > requires following arguments: the device size, zoned profile,
> > zoned model, zone size, zone capacity, number of conventional
> > zones, limits on zone resources (max append sectors, 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 zone_nr_conv=0 -o
> > max_append_sectors=512 -o max_open_zones=0 -o max_active_zones=0
> > -o zoned_profile=zbc
> >
> > Signed-off-by: Sam Li <faithilikerun@gmail.com>
> > ---
> > block/qcow2.c | 119 +++++++++++++++++++++++++++++++
> > block/qcow2.h | 21 ++++++
> > include/block/block-common.h | 5 ++
> > include/block/block_int-common.h | 8 +++
> > qapi/block-core.json | 46 ++++++++----
> > 5 files changed, 185 insertions(+), 14 deletions(-)
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 7f3948360d..b886dab42b 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 0x7a6264
> >
> > 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,37 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> > break;
> > }
> >
> > + case QCOW2_EXT_MAGIC_ZONED_FORMAT:
> > + {
> > + if (ext.len != sizeof(zoned_ext)) {
> > + error_setg_errno(errp, -ret, "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;
> > + }
> > +
> > + zoned_ext.zone_size = be32_to_cpu(zoned_ext.zone_size);
> > + zoned_ext.nr_zones = be32_to_cpu(zoned_ext.nr_zones);
> > + zoned_ext.zone_nr_conv = be32_to_cpu(zoned_ext.zone_nr_conv);
> > + 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_sectors =
> > + be32_to_cpu(zoned_ext.max_append_sectors);
> > + s->zoned_header = zoned_ext;
>
> Please validate these values. The image file is not trusted and may be
> broken/corrupt. For example, zone_size=0 and nr_zones=0 must be rejected
> because the code can't do anything useful when these values are zero
> (similar for values that are not multiples of the block size).
>
> > +
> > +#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
> > @@ -3071,6 +3104,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_profile = s->zoned_header.zoned_profile,
> > + .zoned = s->zoned_header.zoned,
> > + .nr_zones = cpu_to_be32(s->zoned_header.nr_zones),
> > + .zone_size = cpu_to_be32(s->zoned_header.zone_size),
> > + .zone_capacity = cpu_to_be32(s->zoned_header.zone_capacity),
> > + .zone_nr_conv = cpu_to_be32(s->zoned_header.zone_nr_conv),
> > + .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_sectors =
> > + cpu_to_be32(s->zoned_header.max_append_sectors)
> > + };
> > + 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);
> > @@ -3755,6 +3813,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> > s->image_data_file = g_strdup(data_bs->filename);
> > }
> >
> > + if (!strcmp(qcow2_opts->zoned_profile, "zbc")) {
> > + BDRVQcow2State *s = blk_bs(blk)->opaque;
> > + s->zoned_header.zoned_profile = BLK_ZP_ZBC;
> > + s->zoned_header.zoned = BLK_Z_HM;
> > + s->zoned_header.zone_size = qcow2_opts->zone_size;
> > + s->zoned_header.zone_capacity = qcow2_opts->zone_capacity;
> > + s->zoned_header.zone_nr_conv = qcow2_opts->zone_nr_conv;
> > + s->zoned_header.max_open_zones = qcow2_opts->max_open_zones;
> > + s->zoned_header.max_active_zones = qcow2_opts->max_active_zones;
> > + s->zoned_header.max_append_sectors = qcow2_opts->max_append_sectors;
>
> Where is the presence of these optional qcow2_opts checked? For example,
> if the user didn't specify zone_size, then they cannot create an image
> with a zoned profile.
>
> These options also need to be validated to ensure that they contain
> reasonable values (e.g. not 0).
>
> > + }
> > +
> > /* Create a full header (including things like feature table) */
> > ret = qcow2_update_header(blk_bs(blk));
> > bdrv_graph_co_rdunlock();
> > @@ -3873,6 +3943,13 @@ qcow2_co_create_opts(BlockDriver *drv, const char *filename, QemuOpts *opts,
> > qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v3");
> > }
> >
> > + /* The available zoned-profile options are zbc, which stands for
> > + * ZBC/ZAC standards, and zns following NVMe ZNS spec. */
> > + val = qdict_get_try_str(qdict, BLOCK_OPT_Z_PROFILE);
> > + if (val) {
> > + qdict_put_str(qdict, BLOCK_OPT_Z_PROFILE, val);
> > + }
>
> What is the purpose of this code, it fetches and replaces the same qdict
> element?
It creates a string configuration for zoned_profile and matches the
user input to that config.
>
> > +
> > /* Change legacy command line options into QMP ones */
> > static const QDictRenames opt_renames[] = {
> > { BLOCK_OPT_BACKING_FILE, "backing-file" },
> > @@ -3885,6 +3962,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_Z_PROFILE, "zoned-profile"},
> > + { BLOCK_OPT_Z_NR_COV, "zone-nr-conv"},
> > + { BLOCK_OPT_Z_MOZ, "max-open-zones"},
> > + { BLOCK_OPT_Z_MAZ, "max-active-zones"},
> > + { BLOCK_OPT_Z_MAS, "max-append-sectors"},
> > + { BLOCK_OPT_Z_SIZE, "zone-size"},
> > + { BLOCK_OPT_Z_CAP, "zone-capacity"},
> > { NULL, NULL },
> > };
> >
> > @@ -6048,6 +6132,41 @@ static QemuOptsList qcow2_create_opts = {
> > .help = "Compression method used for image cluster " \
> > "compression", \
> > .def_value_str = "zlib" \
> > + }, \
> > + {
>
> The forward slash ('\') that wraps the line is missing and indentation
> is off.
>
> > + .name = BLOCK_OPT_Z_PROFILE, \
> > + .type = QEMU_OPT_STRING, \
> > + .help = "zoned format option for the disk img", \
> > + }, \
> > + { \
>
> Indentation is off.
>
> > + .name = BLOCK_OPT_Z_SIZE, \
> > + .type = QEMU_OPT_SIZE, \
> > + .help = "zone size", \
> > + }, \
> > + { \
> > + .name = BLOCK_OPT_Z_CAP, \
> > + .type = QEMU_OPT_SIZE, \
> > + .help = "zone capacity", \
> > + }, \
> > + { \
> > + .name = BLOCK_OPT_Z_NR_COV, \
>
> Indentation is off.
>
> > + .type = QEMU_OPT_NUMBER, \
> > + .help = "numbers of conventional zones", \
> > + }, \
> > + { \
> > + .name = BLOCK_OPT_Z_MAS, \
> > + .type = QEMU_OPT_NUMBER, \
> > + .help = "max append sectors", \
> > + }, \
> > + { \
> > + .name = BLOCK_OPT_Z_MAZ, \
> > + .type = QEMU_OPT_NUMBER, \
> > + .help = "max active zones", \
> > + }, \
> > + { \
> > + .name = BLOCK_OPT_Z_MOZ, \
> > + .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 4f67eb912a..fe18dc4d97 100644
> > --- a/block/qcow2.h
> > +++ b/block/qcow2.h
> > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > uint64_t length;
> > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> >
> > +typedef struct Qcow2ZonedHeaderExtension {
> > + /* Zoned device attributes */
> > + BlockZonedProfile zoned_profile;
> > + BlockZoneModel zoned;
> > + uint32_t zone_size;
> > + uint32_t zone_capacity;
> > + uint32_t nr_zones;
> > + uint32_t zone_nr_conv;
> > + uint32_t max_active_zones;
> > + uint32_t max_open_zones;
> > + uint32_t max_append_sectors;
> > + uint8_t padding[3];
>
> This looks strange. Why is there 3 bytes of padding at the end? Normally
> padding would align to an even power-of-two number of bytes like 2, 4,
> 8, etc.
It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
16, the padding is 2.
>
> > +} QEMU_PACKED Qcow2ZonedHeaderExtension;
> > +
> > typedef struct Qcow2UnknownHeaderExtension {
> > uint32_t magic;
> > uint32_t len;
> > @@ -419,6 +433,13 @@ typedef struct BDRVQcow2State {
> > * is to convert the image with the desired compression type set.
> > */
> > Qcow2CompressionType compression_type;
> > +
> > + /* States of zoned device */
> > + Qcow2ZonedHeaderExtension zoned_header;
> > + uint32_t nr_zones_exp_open;
> > + uint32_t nr_zones_imp_open;
> > + uint32_t nr_zones_closed;
> > + BlockZoneWps *wps;
>
> Normally qcow2 code passes bs around, so it should be possible to access
> the wps pointer without duplicating it here. This new field is not used
> in this patch, so I can't tell yet how important it is. It's safer to
> avoid duplicating pointers when the original pointer can be accessed
> conveniently so that use-after-free, double-free, and similar memory
> management bugs can be eliminated.
I see. Thanks!
>
> > } BDRVQcow2State;
> >
> > typedef struct Qcow2COWRegion {
> > diff --git a/include/block/block-common.h b/include/block/block-common.h
> > index e15395f2cb..9f04a772f6 100644
> > --- a/include/block/block-common.h
> > +++ b/include/block/block-common.h
> > @@ -108,6 +108,11 @@ typedef enum BlockZoneType {
> > BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> > } BlockZoneType;
> >
> > +typedef enum BlockZonedProfile {
> > + BLK_ZP_ZBC = 0x1,
> > + BLK_ZP_ZNS = 0x2,
> > +} BlockZonedProfile;
> > +
> > /*
> > * Zone descriptor data structure.
> > * Provides information on a zone with all position and size values in bytes.
> > diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> > index 74195c3004..4be35feaf8 100644
> > --- a/include/block/block_int-common.h
> > +++ b/include/block/block_int-common.h
> > @@ -57,6 +57,14 @@
> > #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_Z_PROFILE "zoned_profile"
> > +#define BLOCK_OPT_Z_MODEL "zoned"
> > +#define BLOCK_OPT_Z_SIZE "zone_size"
> > +#define BLOCK_OPT_Z_CAP "zone_capacity"
> > +#define BLOCK_OPT_Z_NR_COV "zone_nr_conv"
> > +#define BLOCK_OPT_Z_MAS "max_append_sectors"
> > +#define BLOCK_OPT_Z_MAZ "max_active_zones"
> > +#define BLOCK_OPT_Z_MOZ "max_open_zones"
> >
> > #define BLOCK_PROBE_BUF_SIZE 512
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 4bf89171c6..f9a584cbb3 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -5013,24 +5013,42 @@
> > #
> > # @compression-type: The image cluster compression method
> > # (default: zlib, since 5.1)
> > +# @zoned-profile: Two zoned device protocol options, zbc or zns
> > +# (default: off, since 8.0)
> > +# @zone-size: The size of a zone of the zoned device (since 8.0)
> > +# @zone-capacity: The capacity of a zone of the zoned device (since 8.0)
> > +# @zone-nr-conv: The number of conventional zones of the zoned device
> > +# (since 8.0)
> > +# @max-open-zones: The maximal allowed open zones (since 8.0)
> > +# @max-active-zones: The limit of the zones that have the implicit open,
> > +# explicit open or closed state (since 8.0)
> > +# @max-append-sectors: The maximal sectors that is allowed to append write
> > +# (since 8.0)
>
> Since 8.1.
>
> > #
> > # Since: 2.12
> > ##
> > { 'struct': 'BlockdevCreateOptionsQcow2',
> > - 'data': { 'file': 'BlockdevRef',
> > - '*data-file': 'BlockdevRef',
> > - '*data-file-raw': 'bool',
> > - '*extended-l2': 'bool',
> > - 'size': 'size',
> > - '*version': 'BlockdevQcow2Version',
> > - '*backing-file': 'str',
> > - '*backing-fmt': 'BlockdevDriver',
> > - '*encrypt': 'QCryptoBlockCreateOptions',
> > - '*cluster-size': 'size',
> > - '*preallocation': 'PreallocMode',
> > - '*lazy-refcounts': 'bool',
> > - '*refcount-bits': 'int',
> > - '*compression-type':'Qcow2CompressionType' } }
> > + 'data': { 'file': 'BlockdevRef',
> > + '*data-file': 'BlockdevRef',
> > + '*data-file-raw': 'bool',
> > + '*extended-l2': 'bool',
> > + 'size': 'size',
> > + '*version': 'BlockdevQcow2Version',
> > + '*backing-file': 'str',
> > + '*backing-fmt': 'BlockdevDriver',
> > + '*encrypt': 'QCryptoBlockCreateOptions',
> > + '*cluster-size': 'size',
> > + '*preallocation': 'PreallocMode',
> > + '*lazy-refcounts': 'bool',
> > + '*refcount-bits': 'int',
> > + '*compression-type': 'Qcow2CompressionType',
> > + '*zoned-profile': 'str',
> > + '*zone-size': 'size',
> > + '*zone-capacity': 'size',
> > + '*zone-nr-conv': 'uint32',
> > + '*max-open-zones': 'uint32',
> > + '*max-active-zones': 'uint32',
> > + '*max-append-sectors': 'uint32'}}
> >
> > ##
> > # @BlockdevCreateOptionsQed:
> > --
> > 2.40.1
> >
On Mon, Jun 19, 2023 at 06:32:52PM +0800, Sam Li wrote:
> Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道：
> > On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > > diff --git a/block/qcow2.h b/block/qcow2.h
> > > index 4f67eb912a..fe18dc4d97 100644
> > > --- a/block/qcow2.h
> > > +++ b/block/qcow2.h
> > > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > > uint64_t length;
> > > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> > >
> > > +typedef struct Qcow2ZonedHeaderExtension {
> > > + /* Zoned device attributes */
> > > + BlockZonedProfile zoned_profile;
> > > + BlockZoneModel zoned;
> > > + uint32_t zone_size;
> > > + uint32_t zone_capacity;
> > > + uint32_t nr_zones;
> > > + uint32_t zone_nr_conv;
> > > + uint32_t max_active_zones;
> > > + uint32_t max_open_zones;
> > > + uint32_t max_append_sectors;
> > > + uint8_t padding[3];
> >
> > This looks strange. Why is there 3 bytes of padding at the end? Normally
> > padding would align to an even power-of-two number of bytes like 2, 4,
> > 8, etc.
>
> It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
> 16, the padding is 2.
I don't understand. Can you explain why there is padding at the end of
this struct?
Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 22:42写道：
>
> On Mon, Jun 19, 2023 at 06:32:52PM +0800, Sam Li wrote:
> > Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道：
> > > On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > > > diff --git a/block/qcow2.h b/block/qcow2.h
> > > > index 4f67eb912a..fe18dc4d97 100644
> > > > --- a/block/qcow2.h
> > > > +++ b/block/qcow2.h
> > > > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > > > uint64_t length;
> > > > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> > > >
> > > > +typedef struct Qcow2ZonedHeaderExtension {
> > > > + /* Zoned device attributes */
> > > > + BlockZonedProfile zoned_profile;
> > > > + BlockZoneModel zoned;
> > > > + uint32_t zone_size;
> > > > + uint32_t zone_capacity;
> > > > + uint32_t nr_zones;
> > > > + uint32_t zone_nr_conv;
> > > > + uint32_t max_active_zones;
> > > > + uint32_t max_open_zones;
> > > > + uint32_t max_append_sectors;
> > > > + uint8_t padding[3];
> > >
> > > This looks strange. Why is there 3 bytes of padding at the end? Normally
> > > padding would align to an even power-of-two number of bytes like 2, 4,
> > > 8, etc.
> >
> > It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
> > 16, the padding is 2.
>
> I don't understand. Can you explain why there is padding at the end of
> this struct?
The overall size should be aligned with 64 bit, which leaves use one
uint32_t and two fields zoned, zoned_profile. I am not sure the size
of macros here and it used 4 for each. So it makes 3 (*8) + 32 + 8 =
64 in the end. If the macro size is wrong, then the padding will
change as well.
Sam
On Mon, Jun 19, 2023 at 10:50:31PM +0800, Sam Li wrote:
> Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 22:42写道：
> >
> > On Mon, Jun 19, 2023 at 06:32:52PM +0800, Sam Li wrote:
> > > Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道：
> > > > On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > > > > diff --git a/block/qcow2.h b/block/qcow2.h
> > > > > index 4f67eb912a..fe18dc4d97 100644
> > > > > --- a/block/qcow2.h
> > > > > +++ b/block/qcow2.h
> > > > > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > > > > uint64_t length;
> > > > > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> > > > >
> > > > > +typedef struct Qcow2ZonedHeaderExtension {
> > > > > + /* Zoned device attributes */
> > > > > + BlockZonedProfile zoned_profile;
> > > > > + BlockZoneModel zoned;
> > > > > + uint32_t zone_size;
> > > > > + uint32_t zone_capacity;
> > > > > + uint32_t nr_zones;
> > > > > + uint32_t zone_nr_conv;
> > > > > + uint32_t max_active_zones;
> > > > > + uint32_t max_open_zones;
> > > > > + uint32_t max_append_sectors;
> > > > > + uint8_t padding[3];
> > > >
> > > > This looks strange. Why is there 3 bytes of padding at the end? Normally
> > > > padding would align to an even power-of-two number of bytes like 2, 4,
> > > > 8, etc.
> > >
> > > It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
> > > 16, the padding is 2.
> >
> > I don't understand. Can you explain why there is padding at the end of
> > this struct?
>
> The overall size should be aligned with 64 bit, which leaves use one
> uint32_t and two fields zoned, zoned_profile. I am not sure the size
> of macros here and it used 4 for each. So it makes 3 (*8) + 32 + 8 =
> 64 in the end. If the macro size is wrong, then the padding will
> change as well.
The choice of the type (char or int) representing an enum is
implementation-defined according to the C17 standard (see "6.7.2.2
Enumeration specifiers").
Therefore it's not portable to use enums in structs exposed to the
outside world (on-disk formats or network protocols).
Please use uint8_t for the zoned_profile and zoned fields and move them
to the end of the struct so the uint32_t fields are naturally aligned.
I think only 2 bytes of padding will be required to align the struct to
a 64-bit boundary once you've done that.
Stefan
Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月20日周二 22:48写道：
>
> On Mon, Jun 19, 2023 at 10:50:31PM +0800, Sam Li wrote:
> > Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 22:42写道：
> > >
> > > On Mon, Jun 19, 2023 at 06:32:52PM +0800, Sam Li wrote:
> > > > Stefan Hajnoczi <stefanha@redhat.com> 于2023年6月19日周一 18:10写道：
> > > > > On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> > > > > > diff --git a/block/qcow2.h b/block/qcow2.h
> > > > > > index 4f67eb912a..fe18dc4d97 100644
> > > > > > --- a/block/qcow2.h
> > > > > > +++ b/block/qcow2.h
> > > > > > @@ -235,6 +235,20 @@ typedef struct Qcow2CryptoHeaderExtension {
> > > > > > uint64_t length;
> > > > > > } QEMU_PACKED Qcow2CryptoHeaderExtension;
> > > > > >
> > > > > > +typedef struct Qcow2ZonedHeaderExtension {
> > > > > > + /* Zoned device attributes */
> > > > > > + BlockZonedProfile zoned_profile;
> > > > > > + BlockZoneModel zoned;
> > > > > > + uint32_t zone_size;
> > > > > > + uint32_t zone_capacity;
> > > > > > + uint32_t nr_zones;
> > > > > > + uint32_t zone_nr_conv;
> > > > > > + uint32_t max_active_zones;
> > > > > > + uint32_t max_open_zones;
> > > > > > + uint32_t max_append_sectors;
> > > > > > + uint8_t padding[3];
> > > > >
> > > > > This looks strange. Why is there 3 bytes of padding at the end? Normally
> > > > > padding would align to an even power-of-two number of bytes like 2, 4,
> > > > > 8, etc.
> > > >
> > > > It is calculated as 3 if sizeof(zoned+zoned_profile) = 8. Else if it's
> > > > 16, the padding is 2.
> > >
> > > I don't understand. Can you explain why there is padding at the end of
> > > this struct?
> >
> > The overall size should be aligned with 64 bit, which leaves use one
> > uint32_t and two fields zoned, zoned_profile. I am not sure the size
> > of macros here and it used 4 for each. So it makes 3 (*8) + 32 + 8 =
> > 64 in the end. If the macro size is wrong, then the padding will
> > change as well.
>
> The choice of the type (char or int) representing an enum is
> implementation-defined according to the C17 standard (see "6.7.2.2
> Enumeration specifiers").
>
> Therefore it's not portable to use enums in structs exposed to the
> outside world (on-disk formats or network protocols).
>
> Please use uint8_t for the zoned_profile and zoned fields and move them
> to the end of the struct so the uint32_t fields are naturally aligned.
>
> I think only 2 bytes of padding will be required to align the struct to
> a 64-bit boundary once you've done that.
I see. Thanks!
Sam
On Mon, Jun 05, 2023 at 06:41:06PM +0800, Sam Li wrote:
> To configure the zoned format feature on the qcow2 driver, it
> requires following arguments: the device size, zoned profile,
> zoned model, zone size, zone capacity, number of conventional
> zones, limits on zone resources (max append sectors, 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 zone_nr_conv=0 -o
> max_append_sectors=512 -o max_open_zones=0 -o max_active_zones=0
> -o zoned_profile=zbc
>
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
> block/qcow2.c | 119 +++++++++++++++++++++++++++++++
> block/qcow2.h | 21 ++++++
> include/block/block-common.h | 5 ++
> include/block/block_int-common.h | 8 +++
> qapi/block-core.json | 46 ++++++++----
> 5 files changed, 185 insertions(+), 14 deletions(-)
>
...
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7f3948360d..b886dab42b 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 0x7a6264
>
> 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,37 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> break;
> }
>
> + case QCOW2_EXT_MAGIC_ZONED_FORMAT:
> + {
Missing a patch to docs/interop/qcow2.txt that describes the new
header so that other qcow2 implementations can be interoperable with
it.
[unrelated - maybe we should convert that file to .rst someday?]
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.