On Mon, Apr 20, 2026 at 3:16 PM Peter Krempa <[email protected]> wrote:

> In last review I've asked to fix the summary to mention 'vmx' instead of
> the redundant 'libvirt:'.
>

Yes changed - apologies to have missed last time


>
> On Mon, Apr 20, 2026 at 12:10:38 +0530, Srihari Parimi via Devel wrote:
> > Parses vtpm.present from VMX files and converts to libvirt TPM
> > device with CRB model and emulator backend. VMware vTPM uses
> > TPM 2.0 with the CRB
>
> In last review I've asked for a link to the document stating where the
> assumption to use TPM 2.0 comes from Cole provided it. Please include it
> as requested.
>
>
Included the document link which Cole provided. The CRB vs TIS - my google
search only shows recommendations to use CRB


> >
> > Signed-off-by: Srihari Parimi <[email protected]>
> > ---
> >  src/vmx/vmx.c              | 34 ++++++++++++++++++++++++++++++++++
> >  tests/vmx2xmldata/vtpm.vmx | 22 ++++++++++++++++++++++
> >  tests/vmx2xmldata/vtpm.xml | 32 ++++++++++++++++++++++++++++++++
> >  tests/vmx2xmltest.c        |  2 ++
> >  4 files changed, 90 insertions(+)
> >  create mode 100644 tests/vmx2xmldata/vtpm.vmx
> >  create mode 100644 tests/vmx2xmldata/vtpm.xml
> >
> > diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> > index 57dfd57cfc..9873794568 100644
> > --- a/src/vmx/vmx.c
> > +++ b/src/vmx/vmx.c
> > @@ -599,6 +599,7 @@ static int virVMXParseSerial(virVMXContext *ctx,
> virConf *conf, int port,
> >  static int virVMXParseParallel(virVMXContext *ctx, virConf *conf, int
> port,
> >                                 virDomainChrDef **def);
> >  static int virVMXParseSVGA(virConf *conf, virDomainVideoDef **def);
> > +static int virVMXParseTPM(virConf *conf, virDomainTPMDef **def);
> >
> >  static int virVMXFormatVNC(virDomainGraphicsDef *def, virBuffer
> *buffer);
> >  static int virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDef *def,
> > @@ -1938,6 +1939,18 @@ virVMXParseConfig(virVMXContext *ctx,
> >
> >      def->nvideos = 1;
> >
> > +    /* def:tpms */
> > +    {
> > +        virDomainTPMDef *tpm = NULL;
> > +        if (virVMXParseTPM(conf, &tpm) < 0)
> > +            goto cleanup;
> > +
> > +        VIR_DEBUG("Is vtpm present: %s",
> > +                (tpm != NULL) ? "yes" : "no");
>
> This is mis-aligned. And differently than in v1 and doesn't even exceed
> maximul line size.
>
> Also none of the other parsers in this file have a VIR_DEBUG statement.
> Either drop it completely or just format it as:
>
>            VIR_DEBUG("vTPM present: '%d'", !!tpm);
>

I have removed this


>
>
> > +        if (tpm)
> > +            VIR_APPEND_ELEMENT(def->tpms, def->ntpms, tpm);
> > +    }
>
>

Reply via email to