Re: [libvirt] [PATCH v8 10/18] conf: Add support for choosing emulation of a TPM 2

On Thu, May 24, 2018 at 04:26:06PM -0400, Stefan Berger wrote: > This patch extends the TPM's device XML with TPM 2 support. This only > works > for the emulator type backend and looks as follows: > >    <tpm model='tpm-tis'> >      <backend type='emulator' version='2'/> >    </tpm> > > The swtpm process now has --tpm2 as an additional parameter: > > system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8  0.0 28364  > 3868 ?        Rs   11:13  13:50 /usr/bin/swtpm socket --daemon --ctrl > type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 > --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log > file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid > file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid > > The version of the TPM can be changed and the state of the TPM is > preserved. > > Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com&gt; > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > docs/formatdomain.html.in                          | 15 ++++++- > docs/schemas/domaincommon.rng                      | 12 ++++++ > src/conf/domain_conf.c                             | 38 > ++++++++++++++++- > src/conf/domain_conf.h                             |  9 ++++ > src/qemu/qemu_tpm.c                                | 49 > +++++++++++++++++++--- > .../tpm-emulator-tpm2.x86_64-latest.args           | 33 +++++++++++++++ > tests/qemuxml2argvdata/tpm-emulator-tpm2.xml       | 30 +++++++++++++ > tests/qemuxml2argvtest.c                           |  1 + > tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml     | 34 +++++++++++++++ > tests/qemuxml2xmloutdata/tpm-emulator.xml          |  2 +- > 10 files changed, 214 insertions(+), 9 deletions(-) > create mode 100644 > tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args > create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml > create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml > > @@ -12717,6 +12722,27 @@ > virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, >         goto error; >     } > > +    version = virXMLPropString(backends[0], "version"); > +    if (!version) > +        def->version = VIR_DOMAIN_TPM_VERSION_1_2; ParseXML should just turn the XML form into virDomainDef. Defaults should be set in PostParse (that will require an addition of VIR_DOMAIN_TPM_VERSION_DEFAULT) > +    else > +        def->version = virDomainTPMVersionTypeFromString(version); This function might return -1. Even though it might be caught by the switch below, I'd rather use: int val; if ((val = vir..TPM.FromString(version) < 0) {    ...    goto error; } def->version = val; > +    switch (def->version) { > +    case VIR_DOMAIN_TPM_VERSION_1_2: > +        /* only TIS available for emulator */ > +        if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) > +            def->model = VIR_DOMAIN_TPM_MODEL_TIS; > +        break; Another default that belongs in PostParse. > +    case VIR_DOMAIN_TPM_VERSION_2: > +        break; > +    case VIR_DOMAIN_TPM_VERSION_LAST: > +    default: > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > +                       _("Unsupported TPM version '%s'"), > +                       version); > +        goto error; > +    } > + >     switch (def->type) { >     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: >         path = virXPathString("string(./backend/device/@path)", ctxt); With the defaults moved:

Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano