by default since elaborate how the bugs show themselves? justification would be great. 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. >> Signed-off-by: yuxiating <yuxiating(a)huawei.com>> --- >> src/conf/domain_conf.c | 15 +++++++++++++++ >> src/conf/domain_conf.h | 9 +++++++++ >> src/conf/domain_validate.c | 6 ++++++ >> src/libvirt_private.syms | 3 ++- >> src/qemu/qemu_command.c | 11 +++++++++++ >> 5 files changed, 43 insertions(+), 1 deletion(-) >Please split the conf/* changes from the qemu changes. >Also you didn't add any documentation to docs/formatdomain.rst which describes the XML which is unacceptable. >Additionally you are also missing test cases for qemuxml2argvtest. I will add a description to docs/ formatDomain.rst. qemuxml2argvtest will be added in version V2. >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index >> 6127513117..bfe4721e60 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -1278,6 +1278,13 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, >> "ignore", >> ); >>> +VIR_ENUM_IMPL(virDomainDiskDiscardEnable, >> + VIR_DOMAIN_DISK_DISCARD_ENABLE_LAST, >> + "default", >> + "off", >> + "on", >NACK to this hunk, 'default' value feels weird, and for the rest we have the virTristateBool type. I find virTristateSwitch more appropriate. virDomainDiskDefDriverParseXML(virDomainDiskDef *def, &def->queues) < 0) >> return -1; >>> + if (virXMLPropEnum(cur, "discard_enable", virDomainDiskDiscardEnableTypeFromString, >> + VIR_XML_PROP_NONZERO, &def->discard_enable) < 0) >> + return -1; >> + >> return 0; >> } >>> @@ -23416,6 +23427,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf, >> if (disk->queues) >> virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues); >>> + if (disk->discard_enable) >> + virBufferAsprintf(&attrBuf, " discard_enable='%s'", >> + >> + virDomainDiskDiscardEnableTypeToString(disk->discard_enable)); >It even behaves exactly like virTristateBool. I'll use virTristateSwitch instead >> + >> virDomainVirtioOptionsFormat(&attrBuf, disk->virtio); >>> if (disk->src->metadataCacheMaxSize > 0) { diff --git >> a/src/conf/domain_conf.h b/src/conf/domain_conf.h index >> c7e6df7981..c39694a19e 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >[...] >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index >> ab8a6c00c3..52a74dd2d5 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -1,5 +1,5 @@ >> -# >> # General private symbols. Add symbols here, and see src/meson.build >> for >> +# mainDiskDeviceTypeToString >> # more details. >> # >> # Keep this file sorted by header name, then by symbols with each header. >NACK to this hunk too, you shouldn't need to modify the header here. It was my mistake. >> @@ -377,6 +377,7 @@ virDomainDiskDefParseSource; >> virDomainDiskDetectZeroesTypeFromString; >> virDomainDiskDetectZeroesTypeToString; >> virDomainDiskDeviceTypeToString; >> +virDomainDiskDiscardEnableTypeToString; >> virDomainDiskDiscardTypeToString; >> virDomainDiskEmptySource; >> virDomainDiskErrorPolicyTypeFromString; >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index >> b230314f7f..894c8b17b9 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -1739,6 +1739,17 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, >> virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); >> } >>> + if (disk->discard_enable) { >> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DISCARD)) >> + { >This capability was also checked in the validator so you don't need to do this here. >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("virtio-blk discard property isn't supported by this " >> + "QEMU binary")); I'm going to delete this check. virDomainDiskDiscardEnableTypeToString(disk->discard_enable)); >> + } >> + >> qemuBuildVirtioOptionsStr(&opt, disk->virtio); >>> if (qemuBuildDeviceAddressStr(&opt, def, &disk->info) < 0) >> -- >> 2.27.0 >>