Re: [RFC PATCH] conf: Add support for keeping TPM emulator state

On Dec 26, 2020, at 14:15, Stefan Berger <stefanb(a)linux.ibm.com&gt; wrote: On 12/22/20 7:12 PM, Eiichi Tsukata wrote: > Currently, swtpm TPM state file is removed when the transient domain is > powered off or the domain is undefined. When we store TPM state on a > shared storage such as NFS and use transient domain, TPM states should > be kept as it is. > > Add per-TPM emulator option `persistent_sate` for keeping TPM state. > This option only works for the emulator type backend and looks as follows: > > <tpm model='tpm-tis'> > <backend type='emulator' persistent_state='yes'/> > </tpm> > > Signed-off-by: Eiichi Tsukata <eiichi.tsukata(a)nutanix.com&gt; > --- > docs/formatdomain.rst | 7 ++++ > docs/schemas/domaincommon.rng | 12 ++++++ > src/conf/domain_conf.c | 21 ++++++++++ > src/conf/domain_conf.h | 1 + > src/qemu/qemu_tpm.c | 3 +- > ...pm-emulator-tpm2-pstate.x86_64-latest.args | 38 +++++++++++++++++++ > .../tpm-emulator-tpm2-pstate.xml | 30 +++++++++++++++ > tests/qemuxml2argvtest.c | 1 + > ...tpm-emulator-tpm2-pstate.x86_64-latest.xml | 37 ++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 + > 10 files changed, 150 insertions(+), 1 deletion(-) > create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args > create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml > create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 512939679b..9e5f6340fb 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -6986,6 +6986,13 @@ Example: usage of the TPM Emulator > - '1.2' : creates a TPM 1.2 > - '2.0' : creates a TPM 2.0 > > +``persistent_state`` > + The ``persistent_state`` attribute indicates whether 'swtpm' TPM state is > + kept or not when the transient domain is powered off or undefined. This Nit: the transient domain -> a transient domain

> + option can be used for storing TPM state on shared storage. By default the Nit: storing -> preserving But this is not only related to shared storage, is it? Can we remove 'on shared storage'?

> + value is ``no``. This attribute only works with the ``emulator`` backend. > + The accepted values are ``yes`` and ``no``. > + > ``encryption`` > The ``encryption`` element allows the state of a TPM emulator to be > encrypted. The ``secret`` must reference a secret object that holds the > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 795b654feb..d7cedc014c 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -4780,6 +4780,18 @@ > </optional> > </group> > </choice> > + <choice> > + <group> > + <optional> > + <attribute name="persistent_state"> > + <choice> > + <value>yes</value> > + <value>no</value> > + </choice> > + </attribute> > + </optional> > + </group> > + </choice> > </element> > </define> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 23415b323c..82c3a68347 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -13178,6 +13178,12 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, > * <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> > * </backend> > * </tpm> > + * > + * Emulator persistent_state is supported with the following: > + * > + * <tpm model='tpm-tis'> > + * <backend type='emulator' version='2.0' persistent_state='yes'> > + * </tpm> > */ > static virDomainTPMDefPtr > virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, > @@ -13193,6 +13199,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, > g_autofree char *backend = NULL; > g_autofree char *version = NULL; > g_autofree char *secretuuid = NULL; > + g_autofree char *persistent_state = NULL; > g_autofree xmlNodePtr *backends = NULL; > > def = g_new0(virDomainTPMDef, 1); > @@ -13265,6 +13272,18 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, > } > def->data.emulator.hassecretuuid = true; > } > + > + persistent_state = virXMLPropString(backends[0], "persistent_state"); > + if (persistent_state) { > + if (virStringParseYesNo(persistent_state, > + &def->data.emulator.persistent_state) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Invalid persistent_state value, either 'yes' or 'no'")); > + goto error; > + } > + } else { > + def->data.emulator.persistent_state = false; > + } > break; > case VIR_DOMAIN_TPM_TYPE_LAST: > goto error; > @@ -26952,6 +26971,8 @@ virDomainTPMDefFormat(virBufferPtr buf, > case VIR_DOMAIN_TPM_TYPE_EMULATOR: > virBufferAsprintf(buf, " version='%s'", > virDomainTPMVersionTypeToString(def->version)); > + if (def->data.emulator.persistent_state) > + virBufferAddLit(buf, " persistent_state='yes'"); > if (def->data.emulator.hassecretuuid) { > char uuidstr[VIR_UUID_STRING_BUFLEN]; > virBufferAddLit(buf, ">\n"); > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 72771c46b9..109625828a 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1362,6 +1362,7 @@ struct _virDomainTPMDef { > char *logfile; > unsigned char secretuuid[VIR_UUID_BUFLEN]; > bool hassecretuuid; > + bool persistent_state; > } emulator; > } data; > }; > diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c > index 872be16570..532e0912bd 100644 > --- a/src/qemu/qemu_tpm.c > +++ b/src/qemu/qemu_tpm.c > @@ -729,7 +729,8 @@ qemuExtTPMCleanupHost(virDomainDefPtr def) > if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR) > continue; > > - qemuTPMDeleteEmulatorStorage(def->tpms[i]); > + if (!def->tpms[i]->data.emulator.persistent_state) > + qemuTPMDeleteEmulatorStorage(def->tpms[i]); > } > } > > diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args > new file mode 100644 > index 0000000000..90505c7a76 > --- /dev/null > +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args > @@ -0,0 +1,38 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-TPM-VM \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-TPM-VM/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-TPM-VM/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-TPM-VM/.config \ > +QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu-system-x86_64 \ > +-name guest=TPM-VM,debug-threads=on \ > +-S \ > +-object secret,id=masterKey0,format=raw,\ > +file=/tmp/lib/domain--1-TPM-VM/master-key.aes \ > +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off,\ > +memory-backend=pc.ram \ > +-cpu qemu64 \ > +-m 2048 \ > +-object memory-backend-ram,id=pc.ram,size=2147483648 \ > +-overcommit mem-lock=off \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \ > +-display none \ > +-no-user-config \ > +-nodefaults \ > +-chardev socket,id=charmonitor,fd=1729,server,nowait \ > +-mon chardev=charmonitor,id=monitor,mode=control \ > +-rtc base=utc \ > +-no-shutdown \ > +-boot menu=on,strict=on \ > +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ > +-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \ > +-chardev socket,id=chrtpm,path=/dev/test \ > +-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \ > +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ > +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ > +resourcecontrol=deny \ > +-msg timestamp=on > diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml > new file mode 100644 > index 0000000000..45fc4c0e1a > --- /dev/null > +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml > @@ -0,0 +1,30 @@ > +<domain type='qemu'> > + <name>TPM-VM</name> > + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> > + <memory unit='KiB'>2097152</memory> > + <currentMemory unit='KiB'>512288</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> > + <boot dev='hd'/> > + <bootmenu enable='yes'/> > + </os> > + <features> > + <acpi/> > + </features> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-x86_64</emulator> > + <controller type='usb' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <tpm model='tpm-tis'> > + <backend type='emulator' version='2.0' persistent_state='yes'/> > + </tpm> > + <memballoon model='virtio'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 9b853c6d59..e96a51d18b 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -2460,6 +2460,7 @@ mymain(void) > DO_TEST_CAPS_LATEST("tpm-emulator"); > DO_TEST_CAPS_LATEST("tpm-emulator-tpm2"); > DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc"); > + DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); > DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr"); > > DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE); > diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml > new file mode 100644 > index 0000000000..08bc8d690c > --- /dev/null > +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml > @@ -0,0 +1,37 @@ > +<domain type='qemu'> > + <name>TPM-VM</name> > + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> > + <memory unit='KiB'>2097152</memory> > + <currentMemory unit='KiB'>512288</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> > + <boot dev='hd'/> > + <bootmenu enable='yes'/> > + </os> > + <features> > + <acpi/> > + </features> > + <cpu mode='custom' match='exact' check='none'> > + <model fallback='forbid'>qemu64</model> > + </cpu> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu-system-x86_64</emulator> > + <controller type='usb' index='0' model='piix3-uhci'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> > + </controller> > + <controller type='pci' index='0' model='pci-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <tpm model='tpm-tis'> > + <backend type='emulator' version='2.0' persistent_state='yes'/> > + </tpm> > + <memballoon model='virtio'> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> > + </memballoon> > + </devices> > +</domain> > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 1968be6782..f8bca9f559 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -761,6 +761,7 @@ mymain(void) > DO_TEST_CAPS_LATEST("tpm-emulator"); > DO_TEST_CAPS_LATEST("tpm-emulator-tpm2"); > DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc"); > + DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); > > DO_TEST("metadata", NONE); > DO_TEST("metadata-duplicate", NONE); Otherwise looks good to me. Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com&gt;