[PATCH v3 5/9] domain_conf.c: XML parsing for VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY

[Ján Tomko]

On a Wednesday in 2020, Daniel Henrique Barboza wrote:
>Aside from trivial XML parsing/format changes, this patch adds
>additional rules for TPM device support to better accomodate
>all the available scenarios with the new TPM Proxy.
>
>The changes make no impact to existing domains. This means that
>the scenario of a domain with a single TPM device is still
>supported in the same way.  The restriction of multiple TPM devices
>got alleviated to allow a TPM Proxy device to be added together
>with a TPM device in the same domain. All other combinations
>are still forbidden.
>
>To summarize, after this patch, the following combinations in the same
>domain are valid:
>
>- a single TPM device
>- a single TPM Proxy device
>- a single TPM + single TPM Proxy devices
>
>These combinations in the same domain are NOT allowed:
>
>- 2 or more TPM devices
>- 2 or more TPM Proxy devices
>
>Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
>---
> src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 41 insertions(+), 4 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 01a32f62d1..8164cd58c9 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -13730,6 +13730,14 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>         goto error;
>     }
>
>+    /* TPM Proxy devices have 'passthrough' backend */
>+    if (def->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY &&
>+        def->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) {
>+        virReportError(VIR_ERR_XML_ERROR, "%s",
>+                       _("'Passthrough' backend is required for TPM Proxy devices"));
>+        goto error;
>+    }
>+

This check should be in a Validate function, not the parser.

>     if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
>         goto error;
>
>@@ -21972,15 +21980,39 @@ virDomainDefParseXML(xmlDocPtr xml,
>     if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0)
>         goto error;
>
>-    if (n > 1) {
>+    if (n > 2) {
>         virReportError(VIR_ERR_XML_ERROR, "%s",
>-                       _("only a single TPM device is supported"));
>+                       _("a maximum of two TPM devices is supported, one of "
>+                         "them being a TPM Proxy device"));
>         goto error;
>     }
>
>     if (n > 0) {
>-        if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags)))
>-            goto error;
>+        for (i = 0; i < n; i++) {
>+            g_autoptr(virDomainTPMDef) dev = NULL;
>+
>+            if (!(dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags)))
>+                goto error;
>+
>+            /* TPM Proxy devices must be held in def->tpmproxy. Error
>+             * out if there's a TPM Proxy declared already */
>+            if (dev->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
>+                if (def->tpmproxy) {
>+                    virReportError(VIR_ERR_XML_ERROR, "%s",
>+                                   _("only a single TPM Proxy device is supported"));
>+                    goto error;
>+                }
>+                def->tpmproxy = g_steal_pointer(&dev);
>+            } else {
>+                /* all other TPM devices goes to def->tpm */

Any reason why you store them separately?

It seems they are treated the same in every place except when building
QEMU command line. Switching to a def->tpms array would better reflect
the XML. The Validate function would then check wheteher there's just
one copy of each device type.

Jano

>+                if (def->tpm) {
>+                    virReportError(VIR_ERR_XML_ERROR, "%s",
>+                                   _("only a single TPM non-proxy device is supported"));
>+                    goto error;
>+                }
>+                def->tpm = g_steal_pointer(&dev);
>+            }
>+        }
>     }
>     VIR_FREE(nodes);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200514/76629b26/attachment-0001.sig>