On Mon, Jul 18, 2022 at 11:30:46 +0200, Michal Privoznik wrote:
The _virDomainTPMDef structure has 'version' member, which is a bit misplaced. It's only emulator type of TPM that can have a version, even our documentation says so:
``version`` The ``version`` attribute indicates the version of the TPM. This attribute only works with the ``emulator`` backend. The following versions are supported:
Therefore, move the member into that part of union that's covering emulated TPM devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 34 +++++++++++----------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain.c | 7 +++-- src/qemu/qemu_tpm.c | 10 ++++--- src/qemu/qemu_validate.c | 53 ++++++++++++++++++----------------- src/security/virt-aa-helper.c | 2 +- 6 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d8989e4ff..28f0e75e60 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10396,15 +10396,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, goto error; }
- version = virXMLPropString(backends[0], "version"); - if (version && - (def->version = virDomainTPMVersionTypeFromString(version)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported TPM version '%s'"), - version); - goto error; - } - switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: if (!(def->data.passthrough.source = virDomainChrSourceDefNew(xmlopt))) @@ -10416,6 +10407,15 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, def->data.passthrough.source->data.file.path = g_steal_pointer(&path); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + version = virXMLPropString(backends[0], "version"); + if (version && + (def->data.emulator.version = virDomainTPMVersionTypeFromString(version)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported TPM version '%s'"), + version); + goto error; + } + if (!(def->data.emulator.source = virDomainChrSourceDefNew(xmlopt))) goto error; secretuuid = virXPathString("string(./backend/encryption/@secret)", ctxt);
Same conditions for using my R-b as per previous patch apply. [...]
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 764d5b029e..ff164118b7 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4760,33 +4760,34 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, { virDomainCapsDeviceTPM tpmCaps = { 0 };
- switch (tpm->version) { - case VIR_DOMAIN_TPM_VERSION_1_2: - /* TPM 1.2 + CRB do not work */ - if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && - tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported interface %s for TPM 1.2"), - virDomainTPMModelTypeToString(tpm->model)); - return -1; + if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) { + switch (tpm->data.emulator.version) { + case VIR_DOMAIN_TPM_VERSION_1_2: + /* TPM 1.2 + CRB do not work */ + if (tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported interface %s for TPM 1.2"), + virDomainTPMModelTypeToString(tpm->model));
I know this is supposed to be a pure code movement but please add quotes around '%s' in the error message.
+ return -1; + } + /* TPM 1.2 + SPAPR do not work with any 'type' (backend) */ + if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("TPM 1.2 is not supported with the SPAPR device model")); + return -1;
Under same conditions as previous patch: Reviewed-by: Peter Krempa <pkrempa@redhat.com>