Re: [PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice

On Wed, Jul 08, 2020 at 10:19:11 -0400, Stefan Berger wrote: > On 7/8/20 10:10 AM, Peter Krempa wrote: >> On Wed, Jul 08, 2020 at 09:57:56 -0400, Stefan Berger wrote: >>> From: Stefan Berger <stefanb(a)linux.ibm.com&gt; >>> >>> The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so >>> prevent the choice of TPM 1.2 when the SPAPR device model is chosen >>> and use a default of '2.0' (TPM 2) for the backend. >>> >>> Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; >>> --- >>> src/qemu/qemu_validate.c | 14 ++++++++++++-- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c >>> index bd7590a00a..f4d71aebf5 100644 >>> --- a/src/qemu/qemu_validate.c >>> +++ b/src/qemu/qemu_validate.c >>> @@ -3645,8 +3645,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, >>> virQEMUCapsFlags flag; >>> /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */ >>> - if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) >>> - tpm->version = VIR_DOMAIN_TPM_VERSION_1_2; >>> + if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) { >>> + if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) >>> + tpm->version = VIR_DOMAIN_TPM_VERSION_2_0; >>> + else >>> + tpm->version = VIR_DOMAIN_TPM_VERSION_1_2; >>> + } >> This is the validation callback, which must not modify the config. >> >> qemuDomainDefTPMsPostParse is the correct place. Beaware that the >> function has a short-circuit condition at the beginning. > > So should I remove the existing code that already set the version to > VIR_DOMAIN_TPM_VERSION_1_2 and move this into that other function? Ideally yes as a refactor prior to your change. The XML parser shouldn't really be setting defaults or validating any form of interdependance of values.