Re: [PATCH v3 3/4] qcow2: add zoned emulation capability

2023-09-13 Thread Stefan Hajnoczi
On Mon, Aug 28, 2023 at 11:09:54PM +0800, Sam Li wrote:
> By adding zone operations and zoned metadata, the zoned emulation
> capability enables full emulation support of zoned device using
> a qcow2 file. The zoned device metadata includes zone type,
> zoned device state and write pointer of each zone, which is stored
> to an array of unsigned integers.
> 
> Each zone of a zoned device makes state transitions following
> the zone state machine. The zone state machine mainly describes
> five states, IMPLICIT OPEN, EXPLICIT OPEN, FULL, EMPTY and CLOSED.
> READ ONLY and OFFLINE states will generally be affected by device
> internal events. The operations on zones cause corresponding state
> changing.
> 
> Zoned devices have a limit on zone resources, which puts constraints on
> write operations into zones.
> 
> Signed-off-by: Sam Li 
> ---
>  block/qcow2.c  | 657 -
>  block/qcow2.h  |   2 +
>  block/trace-events |   1 +
>  docs/interop/qcow2.txt |   6 +
>  4 files changed, 664 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7074bfc620..bc98d98c8e 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -194,6 +194,153 @@ qcow2_extract_crypto_opts(QemuOpts *opts, const char 
> *fmt, Error **errp)
>  return cryptoopts_qdict;
>  }
>  
> +#define QCOW2_ZT_IS_CONV(wp)(wp & 1ULL << 59)
> +
> +static inline int qcow2_get_wp(uint64_t wp)
> +{
> +/* clear state and type information */
> +return ((wp << 5) >> 5);
> +}
> +
> +static inline int qcow2_get_zs(uint64_t wp)
> +{
> +return (wp >> 60);
> +}
> +
> +static inline void qcow2_set_zs(uint64_t *wp, BlockZoneState zs)
> +{
> +uint64_t addr = qcow2_get_wp(*wp);
> +addr |= ((uint64_t)zs << 60);
> +*wp = addr;
> +}
> +
> +/*
> + * Perform a state assignment and a flush operation that writes the new wp
> + * value to the dedicated location of the disk file.
> + */
> +static int qcow2_write_wp_at(BlockDriverState *bs, uint64_t *wp,
> + uint32_t index, BlockZoneState zs) {
> +BDRVQcow2State *s = bs->opaque;
> +uint64_t wpv = *wp;
> +int ret;
> +
> +qcow2_set_zs(wp, zs);
> +ret = bdrv_pwrite(bs->file, s->zoned_header.zonedmeta_offset
> ++ sizeof(uint64_t) * index, sizeof(uint64_t), wp, 0);
> +
> +if (ret < 0) {
> +goto exit;
> +}
> +trace_qcow2_wp_tracking(index, qcow2_get_wp(*wp) >> BDRV_SECTOR_BITS);
> +return ret;
> +
> +exit:
> +*wp = wpv;
> +error_report("Failed to write metadata with file");
> +return ret;
> +}
> +
> +static bool qcow2_check_active_zones(BlockDriverState *bs)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +
> +if (!s->zoned_header.max_active_zones) {
> +return true;
> +}
> +
> +if (s->nr_zones_exp_open + s->nr_zones_imp_open + s->nr_zones_closed
> +< s->zoned_header.max_active_zones) {
> +return true;
> +}
> +
> +return false;
> +}
> +
> +static int qcow2_check_open_zones(BlockDriverState *bs)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +int ret;
> +
> +if (!s->zoned_header.max_open_zones) {
> +return 0;
> +}
> +
> +if (s->nr_zones_exp_open + s->nr_zones_imp_open
> +< s->zoned_header.max_open_zones) {
> +return 0;
> +}
> +
> +if(s->nr_zones_imp_open && qcow2_check_active_zones(bs)) {
> +/* TODO: it takes O(n) time complexity (n = nr_zones).
> + * Optimizations required. */
> +/* close one implicitly open zones to make it available */
> +for (int i = s->zoned_header.nr_conv_zones;

Please use uint32_t to keep the types consistent.

> +i < bs->bl.nr_zones; ++i) {
> +uint64_t *wp = >wps->wp[i];
> +if (qcow2_get_zs(*wp) == BLK_ZS_IOPEN) {
> +ret = qcow2_write_wp_at(bs, wp, i, BLK_ZS_CLOSED);
> +if (ret < 0) {
> +return ret;
> +}
> +bs->wps->wp[i] = *wp;

This assignment is unnecessary since wp points to bs->wps->wp[i]. The
value has already been updated.

> +s->nr_zones_imp_open--;
> +s->nr_zones_closed++;
> +break;
> +}
> +}
> +return 0;

If this for loop completes with i == bs->bl.nr_zones then we've failed
to close an IOPEN zone. The return value should be an error like -EBUSY.

> +}
> +
> +return -EINVAL;

Which case does this -EINVAL cover? Won't we get here when no zones are
open yet?


signature.asc
Description: PGP signature


[PATCH v3 3/4] qcow2: add zoned emulation capability

2023-08-28 Thread Sam Li
By adding zone operations and zoned metadata, the zoned emulation
capability enables full emulation support of zoned device using
a qcow2 file. The zoned device metadata includes zone type,
zoned device state and write pointer of each zone, which is stored
to an array of unsigned integers.

Each zone of a zoned device makes state transitions following
the zone state machine. The zone state machine mainly describes
five states, IMPLICIT OPEN, EXPLICIT OPEN, FULL, EMPTY and CLOSED.
READ ONLY and OFFLINE states will generally be affected by device
internal events. The operations on zones cause corresponding state
changing.

Zoned devices have a limit on zone resources, which puts constraints on
write operations into zones.

Signed-off-by: Sam Li 
---
 block/qcow2.c  | 657 -
 block/qcow2.h  |   2 +
 block/trace-events |   1 +
 docs/interop/qcow2.txt |   6 +
 4 files changed, 664 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7074bfc620..bc98d98c8e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -194,6 +194,153 @@ qcow2_extract_crypto_opts(QemuOpts *opts, const char 
*fmt, Error **errp)
 return cryptoopts_qdict;
 }
 
+#define QCOW2_ZT_IS_CONV(wp)(wp & 1ULL << 59)
+
+static inline int qcow2_get_wp(uint64_t wp)
+{
+/* clear state and type information */
+return ((wp << 5) >> 5);
+}
+
+static inline int qcow2_get_zs(uint64_t wp)
+{
+return (wp >> 60);
+}
+
+static inline void qcow2_set_zs(uint64_t *wp, BlockZoneState zs)
+{
+uint64_t addr = qcow2_get_wp(*wp);
+addr |= ((uint64_t)zs << 60);
+*wp = addr;
+}
+
+/*
+ * Perform a state assignment and a flush operation that writes the new wp
+ * value to the dedicated location of the disk file.
+ */
+static int qcow2_write_wp_at(BlockDriverState *bs, uint64_t *wp,
+ uint32_t index, BlockZoneState zs) {
+BDRVQcow2State *s = bs->opaque;
+uint64_t wpv = *wp;
+int ret;
+
+qcow2_set_zs(wp, zs);
+ret = bdrv_pwrite(bs->file, s->zoned_header.zonedmeta_offset
++ sizeof(uint64_t) * index, sizeof(uint64_t), wp, 0);
+
+if (ret < 0) {
+goto exit;
+}
+trace_qcow2_wp_tracking(index, qcow2_get_wp(*wp) >> BDRV_SECTOR_BITS);
+return ret;
+
+exit:
+*wp = wpv;
+error_report("Failed to write metadata with file");
+return ret;
+}
+
+static bool qcow2_check_active_zones(BlockDriverState *bs)
+{
+BDRVQcow2State *s = bs->opaque;
+
+if (!s->zoned_header.max_active_zones) {
+return true;
+}
+
+if (s->nr_zones_exp_open + s->nr_zones_imp_open + s->nr_zones_closed
+< s->zoned_header.max_active_zones) {
+return true;
+}
+
+return false;
+}
+
+static int qcow2_check_open_zones(BlockDriverState *bs)
+{
+BDRVQcow2State *s = bs->opaque;
+int ret;
+
+if (!s->zoned_header.max_open_zones) {
+return 0;
+}
+
+if (s->nr_zones_exp_open + s->nr_zones_imp_open
+< s->zoned_header.max_open_zones) {
+return 0;
+}
+
+if(s->nr_zones_imp_open && qcow2_check_active_zones(bs)) {
+/* TODO: it takes O(n) time complexity (n = nr_zones).
+ * Optimizations required. */
+/* close one implicitly open zones to make it available */
+for (int i = s->zoned_header.nr_conv_zones;
+i < bs->bl.nr_zones; ++i) {
+uint64_t *wp = >wps->wp[i];
+if (qcow2_get_zs(*wp) == BLK_ZS_IOPEN) {
+ret = qcow2_write_wp_at(bs, wp, i, BLK_ZS_CLOSED);
+if (ret < 0) {
+return ret;
+}
+bs->wps->wp[i] = *wp;
+s->nr_zones_imp_open--;
+s->nr_zones_closed++;
+break;
+}
+}
+return 0;
+}
+
+return -EINVAL;
+}
+
+/*
+ * The zoned device has limited zone resources of open, closed, active
+ * zones.
+ */
+static int qcow2_check_zone_resources(BlockDriverState *bs,
+  BlockZoneState zs)
+{
+int ret = 0;
+
+switch (zs) {
+case BLK_ZS_EMPTY:
+if (!qcow2_check_active_zones(bs)) {
+error_report("No enough active zones");
+return -EINVAL;
+}
+return ret;
+case BLK_ZS_CLOSED:
+ret = qcow2_check_open_zones(bs);
+if (ret < 0) {
+error_report("No enough open zones");
+return ret;
+}
+return ret;
+default:
+return -EINVAL;
+}
+
+}
+
+static inline int qcow2_refresh_zonedmeta(BlockDriverState *bs)
+{
+int ret;
+BDRVQcow2State *s = bs->opaque;
+uint64_t wps_size = s->zoned_header.zonedmeta_size;
+g_autofree uint64_t *temp = NULL;
+temp = g_new(uint64_t, wps_size);
+ret = bdrv_pread(bs->file, s->zoned_header.zonedmeta_offset,
+ wps_size, temp, 0);
+if (ret < 0) {
+error_report("Can not read metadata");
+