Re: [PATCH v12 4/5] qcow2: add zoned emulation capability

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3396 bytes --]

On Tue, Jun 23, 2026 at 08:48:29PM +0200, Sam Li wrote:
> +static int coroutine_fn GRAPH_RDLOCK
> +qcow2_rw_wp_at(BlockDriverState *bs, uint64_t *wp,
> +                  int32_t index, bool is_write)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    uint64_t wp_byte_off = sizeof(uint64_t) * index;
> +    uint64_t cluster_file_off =
> +        s->zoned_header.zonedmeta_offset +
> +        (wp_byte_off & ~((uint64_t)s->cluster_size - 1));
> +    size_t off_in_cluster = wp_byte_off & (s->cluster_size - 1);
> +    void *cluster_buf;
> +    uint64_t *slot;
> +    int ret;
> +
> +    assert(s->wp_cache != NULL);
> +
> +    qemu_co_mutex_lock(&s->lock);
> +    ret = qcow2_cache_get(bs, s->wp_cache, cluster_file_off, &cluster_buf);
> +    if (ret < 0) {
> +        qemu_co_mutex_unlock(&s->lock);
> +        error_report("Failed to %s WP slot (zone %d): %s",
> +                     is_write ? "write" : "read", index, strerror(-ret));
> +        return ret;
> +    }
> +
> +    slot = (uint64_t *)((char *)cluster_buf + off_in_cluster);
> +    if (is_write) {
> +        *slot = *wp;

Missing cpu_to_be64()?

> +        qcow2_cache_entry_mark_dirty(s->wp_cache, cluster_buf);
> +    } else {
> +        *wp = *slot;

Missing be64_to_cpu()?

> +static inline int coroutine_fn GRAPH_RDLOCK
> +qcow2_refresh_zonedmeta(BlockDriverState *bs)
> +{
> +    int ret;
> +    BDRVQcow2State *s = bs->opaque;
> +    uint64_t wps_size = s->zoned_header.nr_zones * sizeof(uint64_t);
> +    g_autofree uint64_t *temp = NULL;
> +
> +    temp = g_new(uint64_t, s->zoned_header.nr_zones);
> +    ret = bdrv_pread(bs->file, s->zoned_header.zonedmeta_offset,
> +                     wps_size, temp, 0);
> +    if (ret < 0) {
> +        error_report("Cannot read metadata");
> +        return ret;
> +    }
> +
> +    memcpy(bs->wps->wp, temp, wps_size);

Missing be64_to_cpu()?

bs->wps->wp[] is in QEMU's format while temp is in qcow2's format. The
representations are not guaranteed to remain the same. An explicit
conversion from qcow2's representation to QEMU's representation would be
best. You could also use build-time assertions (QEMU_BUILD_BUG_ON()) to
check that the qcow2 constants match the QEMU constants.

> @@ -559,8 +874,34 @@ qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>                             "is wrong");
>                  return -EINVAL;
>              }
> -            if (!qcow2_check_zone_options(&zoned_ext, errp)) {
> -                return -EINVAL;
> +
> +            bs->wps = g_malloc(sizeof(BlockZoneWps)
> +                + s->zoned_header.nr_zones * sizeof(uint64_t));
> +            ret = qcow2_refresh_zonedmeta(bs);

Is bs->wps freed when qcow2_refresh_zonedmeta() fails?
> @@ -4161,6 +4879,45 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>              ret = -EINVAL;
>              goto unlock;
>          }
> +
> +        uint32_t nrz = s->zoned_header.nr_zones;
> +        zoned_meta_size =  sizeof(uint64_t) * nrz;
> +        g_autofree uint64_t *meta = NULL;
> +        meta = g_new0(uint64_t, nrz);
> +
> +        for (i = 0; i < s->zoned_header.conventional_zones; ++i) {
> +            meta[i] = i * s->zoned_header.zone_size;
> +            meta[i] |= 1ULL << 59;
> +        }
> +
> +        for (; i < nrz; ++i) {
> +            meta[i] = i * s->zoned_header.zone_size;
> +        }

Missing cpu_to_be64()?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]