12:54 p.m.
On 5/13/20 11:35 AM, Stefan Berger wrote:
On 5/13/20 10:10 AM, 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(a)gmail.com>
> ---
> src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 01a32f62d1..33b7d69318 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;
> + }
> +
> if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) <
0)
> goto error;
> @@ -21972,15 +21980,41 @@ 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++) {
> + virDomainTPMDefPtr dev = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt,
flags);
> +
> + if (!dev)
> + 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"));
> + VIR_FREE(dev);
> + goto error;
> + }
> + def->tpmproxy = g_steal_pointer(&dev);
Is g_steal_pointer necessary ?
Not in this case, since the VIR_FREE() call is being done before the jump
and I'm not using g_autopt() with this pointer. Thing is that we should
use g_autoptr() in these scenarios to avoid the manual cleanup. Also, I
don't think I can use VIR_FREE() with this pointer - I should've used
virDomainTPMDefFree().
I'll change the code to using g_autoptr() and g_steal_pointer().
Thanks,
DHB
> + } else {
> + /* all other TPM devices goes to def->tpm */
> + if (def->tpm) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("only a single TPM non-proxy device is
supported"));
> + VIR_FREE(dev);
> + goto error;
> + }
> + def->tpm = g_steal_pointer(&dev);
> + }
> + }
> }
> VIR_FREE(nodes);
> @@ -29807,6 +29841,11 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
> goto error;
> }
> + if (def->tpmproxy) {
> + if (virDomainTPMDefFormat(buf, def->tpmproxy, flags) < 0)
> + goto error;
> + }
> +
> for (n = 0; n < def->ngraphics; n++) {
> if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0)
> goto error;