>-----Original Message-----
>From: Peter Krempa <[email protected]>
>Subject: Re: [PATCH v2 02/12] conf: Add scalable_mode attribute to iommu
>
>On Wed, Sep 09, 2026 at 16:29:49 +0800, Zhenzhong Duan wrote:
>> Add scalable_mode attribute to iommu to enable/disable scalable mode
>> for intel-iommu.
>>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> ---
>> docs/formatdomain.rst | 4 ++
>> src/conf/domain_conf.c | 16 ++++++++
>> src/conf/domain_conf.h | 1 +
>> src/conf/domain_validate.c | 3 ++
>> src/conf/schemas/domaincommon.rng | 5 +++
>> src/qemu/qemu_validate.c | 8 +++-
>> ...tel-iommu-scalable-mode.x86_64-latest.args | 34 +++++++++++++++++
>> ...ntel-iommu-scalable-mode.x86_64-latest.xml | 37 +++++++++++++++++++
>> .../intel-iommu-scalable-mode.xml | 37 +++++++++++++++++++
>> tests/qemuxmlconftest.c | 1 +
>> 10 files changed, 145 insertions(+), 1 deletion(-)
>> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-
>mode.x86_64-latest.args
>> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-
>mode.x86_64-latest.xml
>> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-mode.xml
>
>[...]
>
>
>> @@ -22823,6 +22828,13 @@
>virDomainIOMMUDefCheckABIStability(virDomainIOMMUDef *src,
>> virTristateSwitchTypeToString(src->dma_translation));
>> return false;
>> }
>> + if (src->scalable_mode != dst->scalable_mode) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("Target domain IOMMU device scalable mode '%1$s'
>> does not
>match source '%2$s'"),
>
>The error message should mention the attribute name verbatim:
>
> _("Target domain IOMMU device 'scalable_mode' '%1$s' does not match source
>'%2$s'"),
The existing code in virDomainIOMMUDefCheckABIStability() formats the attribute
without quotes (i.e. ssid_size value rather than 'ssid_size' value).
Should I follow existing code style or you prefer 'scalable_mode'?
>
>
>Please apply this change to any further patch in the series.
Sure.
>
>
>> + virTristateSwitchTypeToString(dst->scalable_mode),
>> + virTristateSwitchTypeToString(src->scalable_mode));
>> + return false;
>> + }
>> if (src->pt != dst->pt) {
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("Target domain IOMMU device passthrough '%1$s'
>> does not
>match source '%2$s'"),
>
>[...]
>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index e946f55351..55513b2691 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -5815,7 +5815,7 @@ qemuValidateDomainDeviceDefIOMMU(const
>virDomainIOMMUDef *iommu,
>> return -1;
>> }
>> if (iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT &&
>> - !virQEMUCapsGet(qemuCaps,
>QEMU_CAPS_INTEL_IOMMU_CACHING_MODE)) {
>> + !virQEMUCapsGet(qemuCaps,
>QEMU_CAPS_INTEL_IOMMU_CACHING_MODE)) {
>
>This hunk is unrelated.
Will delete it.
>
>
>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> _("iommu: caching mode is not supported with this
>> QEMU
>binary"));
>> return -1;
>> @@ -5843,6 +5843,12 @@ qemuValidateDomainDeviceDefIOMMU(const
>virDomainIOMMUDef *iommu,
>> _("iommu: updating dma translation is not supported
>> with this
>QEMU binary"));
>> return -1;
>> }
>> + if (iommu->scalable_mode != VIR_TRISTATE_SWITCH_ABSENT &&
>
>Missing newline before if.
This is following existing code style in qemuValidateDomainDeviceDefIOMMU().
I'm not sure, should I add new line starting from scalable_mode?
>
>> + !virQEMUCapsGet(qemuCaps,
>QEMU_CAPS_INTEL_IOMMU_SCALABLE_MODE)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("iommu: scalable mode is not supported with this
>> QEMU
>binary"));
>
>Here also use the verbatim name of the feature:
>
> _("iommu: 'scalable_mode' is not supported with this QEMU binary"));
Got it.
>
>
>and same for all other patches.
Sure.
Thanks
Zhenzhong
