Re: [PATCH v14 3/6] qcow2: add configurations for zoned format extension
Niklas Cassel <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <anUfL40CLw_amIJE@ryzen> |
Hello Sam,
On Thu, Jul 09, 2026 at 12:16:12AM +0200, Sam Li wrote:
> @@ -2068,6 +2236,25 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
> }
> bs->bl.pwrite_zeroes_alignment = s->subcluster_size;
> bs->bl.pdiscard_alignment = s->cluster_size;
> +
> + switch (s->zoned_header.zoned) {
> + case QCOW2_Z_HM:
> + bs->bl.zoned = BLK_Z_HM;
> + break;
> + case QCOW2_Z_NONE:
> + default:
> + bs->bl.zoned = BLK_Z_NONE;
> + break;
> + }
> +
> + bs->bl.nr_zones = s->zoned_header.nr_zones;
> + bs->bl.max_append_sectors = s->zoned_header.max_append_bytes
> + >> BDRV_SECTOR_BITS;
> + bs->bl.max_active_zones = s->zoned_header.max_active_zones;
> + bs->bl.max_open_zones = s->zoned_header.max_open_zones;
> + bs->bl.zone_size = s->zoned_header.zone_size;
> + bs->bl.zone_capacity = s->zoned_header.zone_capacity;
> + bs->bl.write_granularity = BDRV_SECTOR_SIZE;
Since we are already defining max_append_sectors in the qcow2 extension,
is there a reason not to allow the user to define the write_granularity?
From the virtio spec:
"The size of the data that is appended MUST be a multiple of
write_granularity bytes and MUST NOT exceed the max_append_sectors
value provided by the device in virtio_blk_zoned_characteristics
configuration space structure."
At least for virtio, it seems like they go together.
And if you consider that virtio is a specification that can be implemented
by hardware, somehow I do feel like we should (somehow) be able to make the
backing device (qcow2 in this case) represent the hardware.
And I don't really see the logic to define all the struct members in from
virtio
struct virtio_blk_zoned_characteristics {
le32 zone_sectors;
le32 max_open_zones;
le32 max_active_zones;
le32 max_append_sectors;
le32 write_granularity;
u8 model;
u8 unused2[3];
} zoned;
except for write_granularity. (I guess we could still default
write_granularity to BDRV_SECTOR_SIZE.)
If we define max_append_sectors in qcow2, I think we should define
write_granularity in qcow2 as well.
Looking at the write_granularity in virtio-blk + file-posix:
for blkcfg.zoned.write_granularity, which is what virtio-blk exposes to
the guest, it is initialized using:
https://github.com/qemu/qemu/blob/v11.1.0-rc3/hw/block/virtio-blk.c#L1257
conf->logical_block_size:
https://github.com/qemu/qemu/blob/v11.1.0-rc3/hw/block/virtio-blk.c#L1183
and the value we set to the qemu block layer - bs->bl.write_granularity,
which file-posix initilizes using:
https://github.com/qemu/qemu/blob/v11.1.0-rc3/block/file-posix.c#L1498-L1501
the sysfs value for physical_block_size.
Sam, you seem to be the author of the code for both assignments.
It seems inconsistent that one uses the logical block size and one uses
the physical block size. I could send a patch for this if we know which
one to use. I assume that using the logical block size in both places
makes most sense for writes (even if it would be a read-modify-write),
but the physical block size makes most sense for zone appends...
Damien, thoughts?
There are some ZAC drives that support 512e, but they obviously don't
support zone append.
Trying to understand how qemu deals with other hardware limits:
logical_block_size and physical_block_size are parameters for the
virtio-blk-pci device, rather than the qcow2 blockdev.
E.g.
-drive file=vm-disk.qcow2,format=qcow2,if=none,id=hd0 \
-device virtio-blk-pci,drive=hd0,logical_block_size=4096,physical_block_size=4096
for a 4K native device.
Looking at the qcow2 driver, we can see that it does not set
bs->bl.request_alignment in the normal case (only if using crypto).
Looking at the file-posix driver, it does probe the logical block
size of the backing device (using ioctls()) and stores it in
bl.request_alignment.
The virtio-blk driver does have a check:
virtio_blk_sect_range_ok() where it will fail any request where the
size is not aligned to the logical block size, so I guess it is fine
that the qcow2 driver does not initialize bs->bl.request_alignment.
Perhaps we should modify check_zoned_request() to also check if
the len is a multiple of the logical block size?
Without such a check, for a 4Kn device (physical block size == 4K,
logical block size == 4K), with write_granularity unconditionally set to
BDRV_SECTOR_SIZE (512) (like the qcow2 driver seems to do), I think we
would be able to append less than a logical block, which seems wrong.
(This is not a problem right now since file-posix.c does initialize
bs->bl.write_granularity using the sysfs value for physical_block_size
and not a hardcoded 512 value.)
Kind regards,
Niklas