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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
