The maximum zone append size that the device reports to its driver was taken from BlockLimits.max_append_sectors, a limit of the host disk that the backend happens to sit on. A guest reads it out of the configuration space, so it becomes part of what the guest has been told about its device, and migrating that guest to a host whose disks report something smaller leaves it appending more than the device accepts.
Make it a property, as max-discard-sectors and max-write-zeroes-sectors already are, so that the value a guest is given is part of the device model and does not change under it. Nothing from BlockLimits reaches the configuration space any more. Zero is a valid value here, unlike for those two. There is no feature bit for zone append, and a maximum of zero is how the specification has a device say that it does not support the operation (virtio 1.4, 5.2.5.2). It is also what a ZBC or ZAC disk is: such a disk has no zone append command at all, and Linux emulates one in its block layer. What is reported is capped by the size of a zone. The specification asks for the largest append that can be carried out, and one larger than a zone never can be, so the default would otherwise advertise a size that the block layer refuses. The cap tells a guest nothing about the host that zone_sectors has not already told it. Signed-off-by: Niklas Cassel <[email protected]> --- hw/block/virtio-blk.c | 67 ++++++++++++++++++++++++++++++---- include/hw/virtio/virtio-blk.h | 1 + 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 74bd08b0ec..536d71a91a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -521,6 +521,24 @@ typedef struct ZoneCmdData { }; } ZoneCmdData; +/* + * The maximum zone append size that the device reports to its driver, in 512 + * byte sectors. + * + * An append cannot cross a zone boundary, so the configured maximum is capped + * by the size of a zone: the specification asks for the largest append that + * can be carried out, and a larger one never can be. The zone size is already + * reported to the driver as zone_sectors, so this tells it nothing new about + * the host. + */ +static uint32_t virtio_blk_max_append_sectors(VirtIOBlock *s) +{ + BlockDriverState *bs = blk_bs(s->blk); + + return MIN(s->conf.max_append_sectors, + bs->bl.zone_size >> BDRV_SECTOR_BITS); +} + /* * check zoned_request: error checking before issuing requests. If all checks * passed, return true. @@ -556,12 +574,13 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len, return false; } - if (len / 512 > bs->bl.max_append_sectors) { - if (bs->bl.max_append_sectors == 0) { - *status = VIRTIO_BLK_S_UNSUPP; - } else { - *status = VIRTIO_BLK_S_ZONE_INVALID_CMD; - } + if (!s->conf.max_append_sectors) { + *status = VIRTIO_BLK_S_UNSUPP; + return false; + } + + if ((len >> BDRV_SECTOR_BITS) > virtio_blk_max_append_sectors(s)) { + *status = VIRTIO_BLK_S_ZONE_INVALID_CMD; return false; } } @@ -1315,7 +1334,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) virtio_stl_p(vdev, &blkcfg.zoned.write_granularity, blkconf_zone_write_granularity(conf)); virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors, - bs->bl.max_append_sectors); + virtio_blk_max_append_sectors(s)); } else { blkcfg.zoned.model = VIRTIO_BLK_Z_NONE; } @@ -1852,6 +1871,34 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) } } + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_ZONED)) { + uint32_t wg = blkconf_zone_write_granularity(&conf->conf); + + /* + * Checked before the granularity below, so that a driver can shift the + * value that it reads into a byte count without overflowing. + */ + if (conf->max_append_sectors > BDRV_REQUEST_MAX_SECTORS) { + error_setg(errp, "invalid max-append-sectors property (%" PRIu32 + "), must not exceed %d", conf->max_append_sectors, + (int)BDRV_REQUEST_MAX_SECTORS); + return; + } + + /* + * Zero is allowed, and says that zone append is not supported, which + * is what a ZBC or ZAC disk is. Anything else has to be able to + * express a single write. + */ + if (conf->max_append_sectors && + (conf->max_append_sectors << BDRV_SECTOR_BITS) < wg) { + error_setg(errp, "invalid max-append-sectors property (%" PRIu32 + "), must be zero or at least the zone write granularity " + "(%" PRIu32 " bytes)", conf->max_append_sectors, wg); + return; + } + } + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD) && (!conf->max_discard_sectors || conf->max_discard_sectors > BDRV_REQUEST_MAX_SECTORS)) { @@ -1986,6 +2033,12 @@ static const Property virtio_blk_properties[] = { conf.max_discard_sectors, BDRV_REQUEST_MAX_SECTORS), DEFINE_PROP_UINT32("max-write-zeroes-sectors", VirtIOBlock, conf.max_write_zeroes_sectors, BDRV_REQUEST_MAX_SECTORS), + /* + * Zero is valid, and says that the device does not support zone append, + * as a ZBC or ZAC disk does not. + */ + DEFINE_PROP_UINT32("max-append-sectors", VirtIOBlock, + conf.max_append_sectors, BDRV_REQUEST_MAX_SECTORS), DEFINE_PROP_BOOL("x-enable-wce-if-config-wce", VirtIOBlock, conf.x_enable_wce_if_config_wce, true), }; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 3d8dee7ec1..0070ca6ba9 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -47,6 +47,7 @@ struct VirtIOBlkConf bool report_discard_granularity; uint32_t max_discard_sectors; uint32_t max_write_zeroes_sectors; + uint32_t max_append_sectors; bool x_enable_wce_if_config_wce; }; -- 2.55.0
