After previous cleanup, the qemuDomainDefTPMsPostParse() function
does nothing more than validates TPM devices. Therefore, it
should live in qemu_validate.c instead of qemu_domain.c. Move it
there and rename to reflect the fact that the function is doing
validation instead of PostParsing.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_domain.c   | 34 ----------------------------------
 src/qemu/qemu_validate.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bcee4d2602..e3d1bb548f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4583,37 +4583,6 @@ qemuDomainDefNumaCPUsPostParse(virDomainDef *def,
 }
 
 
-static int
-qemuDomainDefTPMsPostParse(virDomainDef *def)
-{
-    virDomainTPMDef *proxyTPM = NULL;
-    virDomainTPMDef *regularTPM = NULL;
-    size_t i;
-
-    for (i = 0; i < def->ntpms; i++) {
-        virDomainTPMDef *tpm = def->tpms[i];
-
-        if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
-            if (proxyTPM) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                               _("only a single TPM Proxy device is supported"));
-                return -1;
-            } else {
-                proxyTPM = tpm;
-            }
-        } else if (regularTPM) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("only a single TPM non-proxy device is supported"));
-            return -1;
-        } else {
-            regularTPM = tpm;
-        }
-    }
-
-    return 0;
-}
-
-
 static int
 qemuDomainDefPostParseBasic(virDomainDef *def,
                             void *opaque G_GNUC_UNUSED)
@@ -4709,9 +4678,6 @@ qemuDomainDefPostParse(virDomainDef *def,
     if (qemuDomainDefNumaCPUsPostParse(def, qemuCaps) < 0)
         return -1;
 
-    if (qemuDomainDefTPMsPostParse(def) < 0)
-        return -1;
-
     return 0;
 }
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index ff164118b7..ce8f92f301 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1113,6 +1113,37 @@ qemuValidateDomainDefPanic(const virDomainDef *def,
 }
 
 
+static int
+qemuValidateDomainDefTPMs(const virDomainDef *def)
+{
+    const virDomainTPMDef *proxyTPM = NULL;
+    const virDomainTPMDef *regularTPM = NULL;
+    size_t i;
+
+    for (i = 0; i < def->ntpms; i++) {
+        virDomainTPMDef *tpm = def->tpms[i];
+
+        if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
+            if (proxyTPM) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("only a single TPM Proxy device is supported"));
+                return -1;
+            } else {
+                proxyTPM = tpm;
+            }
+        } else if (regularTPM) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("only a single TPM non-proxy device is supported"));
+            return -1;
+        } else {
+            regularTPM = tpm;
+        }
+    }
+
+    return 0;
+}
+
+
 int
 qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff,
                             virDomainLifecycleAction onReboot,
@@ -1310,6 +1341,9 @@ qemuValidateDomainDef(const virDomainDef *def,
     if (qemuValidateDomainDefPanic(def, qemuCaps) < 0)
         return -1;
 
+    if (qemuValidateDomainDefTPMs(def) < 0)
+        return -1;
+
     if (def->sec) {
         switch ((virDomainLaunchSecurity) def->sec->sectype) {
         case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-- 
2.35.1

On Mon, Jul 18, 2022 at 11:30:49 +0200, Michal Privoznik wrote:
> After previous cleanup, the qemuDomainDefTPMsPostParse() function
> does nothing more than validates TPM devices. Therefore, it
> should live in qemu_validate.c instead of qemu_domain.c. Move it
> there and rename to reflect the fact that the function is doing
> validation instead of PostParsing.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_domain.c   | 34 ----------------------------------
>  src/qemu/qemu_validate.c | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bcee4d2602..e3d1bb548f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c

[...]

> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index ff164118b7..ce8f92f301 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1113,6 +1113,37 @@ qemuValidateDomainDefPanic(const virDomainDef *def,
>  }
>  
>  
> +static int
> +qemuValidateDomainDefTPMs(const virDomainDef *def)
> +{
> +    const virDomainTPMDef *proxyTPM = NULL;
> +    const virDomainTPMDef *regularTPM = NULL;
> +    size_t i;

Please refactor the function to un-do unnecessarily complex logic:

> +
> +    for (i = 0; i < def->ntpms; i++) {
> +        virDomainTPMDef *tpm = def->tpms[i];
> +
> +        if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
> +            if (proxyTPM) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("only a single TPM Proxy device is supported"));
> +                return -1;
> +            } else {
> +                proxyTPM = tpm;
> +            }

This else section is not needed, and proxyTPM can be assigned
unconditionally.

> +        } else if (regularTPM) {

Make this into a plain 'else' and move this condition inside ...

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("only a single TPM non-proxy device is supported"));
> +            return -1;
> +        } else {
> +            regularTPM = tpm;

... and remove this else branch.

> +        }
> +    }
> +
> +    return 0;
> +}

Reviewed-by: Peter Krempa <pkrempa@redhat.com>