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

On 5/14/20 11:09 AM, Ján Tomko wrote: > 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

> > 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. It is an attempt to minimize the amount of changes needed. For example, making a def->tpms array would impact all the code related to the 'emulator' TPM type, in particular the files qemu_tpm.c and qemu_extdevice.c, that would need to handle an array instead of the def->tpm pointer. It is also an attempt of making it easier for any future "TPM Proxy" device style to be added in the future. Instead of revisiting the logic inside each def->tpms loop one would simply deal with the def->tpmproxy pointer directly. And, in the case this is wrong and no one else cares about it, at the very least we didn't change the design of all TPM devices because of one single PPC64 specific model. This is all up to debate, of course. If we we decide that changing to a def->tpms array is worth the extra lines of code I'll make it happen in the v4.

Thanks, DHB > > 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);