[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 3/4] qcow2: add zoned emulation capability
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v3 3/4] qcow2: add zoned emulation capability |
Date: |
Wed, 13 Sep 2023 17:11:37 -0400 |
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 <faithilikerun@gmail.com>
> ---
> 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 = &bs->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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v3 3/4] qcow2: add zoned emulation capability,
Stefan Hajnoczi <=