On Mon, Jun 29, 2026 at 8:24 PM Stefan Hajnoczi <[email protected]> wrote:
>
> 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.
Another idea is making the wps for qcow2 follow the same format as the
block layer's. If there are more bits of a write pointer used, changes
can be performed on the qemu's representation.
>
> > @@ -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()?
