[PATCH v2 2/3] conf: support for virtio-blk-pci discard option

[Peter Krempa]

On Tue, Sep 07, 2021 at 15:12:09 +0800, yuxiating at huawei.com wrote:
> From: yuxiating <yuxiating at huawei.com>
> 
> DISCARD and WRITE_ZEROES features for machine type >= 4.0 is enabled by default
> since 5c81161f8041("virtio-blk: add "discard" and "write-zeroes" properties).
> Virtio_blk kernel driver has a bug that causes memory corruption in
> virtblk_setup_discard_write_zeroes();
> af822aa68fbd ("block: virtio_blk: fix handling single range discard request")
> has fix it.
> However, some operating systems are not fixed and need to disabled on the
> QEMU side.

I must say that I'm not persuaded that there's enough value in this
workaround. Similarly users could use virtio-scsi instead as a
workaround or fix their OS.

For me this reasoning will not be enough, but other on the list might
think otherwise.

> Signed-off-by: yuxiating <yuxiating at huawei.com>
> ---
>  docs/formatdomain.rst         | 3 +++
>  docs/schemas/domaincommon.rng | 8 ++++++++
>  src/conf/domain_conf.c        | 8 ++++++++
>  src/conf/domain_conf.h        | 1 +
>  src/conf/domain_validate.c    | 6 ++++++
>  5 files changed, 26 insertions(+)

[...]


> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6127513117..304015f42e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -23416,6 +23420,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf,
>      if (disk->queues)
>          virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues);
>  
> +    if (disk->discard_enable)

We tend to use explicit check:

if (disk->discard_enable != VIR_TRISTATE_BOOL_ABSENT)

> +        virBufferAsprintf(&attrBuf, " discard_enable='%s'",
> +                          virTristateSwitchTypeToString(disk->discard_enable));
> +
>      virDomainVirtioOptionsFormat(&attrBuf, disk->virtio);
>  
>      if (disk->src->metadataCacheMaxSize > 0) {

This definitely seems like a feature visible to the guest OS and thus
_must_ be covered by the ABI stability check to prevent regressions in
the ABI.

See 'virDomainDiskDefCheckABIStability'.