[PATCH v2 0/2] qemu: Support backup and fsync optional attributes in SWTPM
In the upcoming release of swtpm, 2 new options, backup and fsync will be added to the directory backend [1] and [2]. This allows the user to backup and fsync the state file and its parent directory. It lowers the probability of TPM state file corruption in case of a power loss. Although this has not been added to the stable swtpm release yet, Libvirt will require some changes to incorporate these options in the XML config. Following is the sample xml configuration snip to enable backup and fsync: <tpm model='tpm-crb'> <backend type='emulator' version='2.0' backup='yes' fsync='yes'/> </tpm> Tested with swtpm commit #8ecf2c1 and libtpms commit #18e42c8d: $ ps aux | grep swtpm arun 33881 0.0 0.0 24080 10188 ? S 00:09 0:00 /usr/local/bin/swtpm socket --ctrl type=unixio,path=/run/user/1000/libvirt/qemu/run/swtpm/5-guest-vm-swtpm.sock,mode=0600 --tpmstate dir=/home/arun/.config/libvirt/qemu/swtpm/0a64239e-6311-4e53-8903-d1d49608a751/tpm2,mode=0600,lock,backup,fsync --log file=/home/arun/.cache/libvirt/qemu/log/guest-vm-swtpm.log --terminate --tpm2 $ ls -lrt $HOME/.config/libvirt/qemu/swtpm/0a64239e-6311-4e53-8903-d1d49608a751/tpm2 total 16 -rw-------. 1 arun arun 6807 Sep 7 00:14 tpm2-00.permall.bak -rw-------. 1 arun arun 6807 Sep 7 01:02 tpm2-00.permall [1] https://github.com/stefanberger/swtpm/commit/186f72a3a995fc68957ffc0b1438bb6... [2] https://github.com/stefanberger/swtpm/commit/795c1973c913da59925fa6c2d688c5e... Changes since v1: - Use TriStateBool type for both backup and sync in virDomainTPMDefParseXML() - Refactor qemuTPMVirCommandSwtpmAddTPMState() to use virBuffer API and return errors on failure. Arun Menon (2): conf: Add backup and fsync optional swtpm attributes qemu: tpm: Add backup and fsync options to the swtpm command docs/formatdomain.rst | 15 +++++ src/conf/domain_conf.c | 22 ++++++++ src/conf/domain_conf.h | 2 + src/conf/schemas/domaincommon.rng | 10 ++++ src/qemu/qemu_tpm.c | 55 ++++++++++++++----- src/util/virtpm.c | 2 + src/util/virtpm.h | 2 + .../qemuxmlconfdata/tpm-emulator-tpm2-enc.xml | 2 +- 8 files changed, 96 insertions(+), 14 deletions(-) -- 2.54.0
From: Arun Menon <armenon@redhat.com> SWTPM will support backup and fsync of the state file. [1] and [2] Libvirt needs to support these options while launching a VM. This commit adds these optional attributes to the RNG schema, and updates the logic for both creation and parsing them from the XML. [1] https://github.com/stefanberger/swtpm/commit/186f72a3a995fc68957ffc0b1438bb6... [2] https://github.com/stefanberger/swtpm/commit/795c1973c913da59925fa6c2d688c5e... Signed-off-by: Arun Menon <armenon@redhat.com> --- docs/formatdomain.rst | 15 +++++++++++++ src/conf/domain_conf.c | 22 +++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 +++++++++ .../qemuxmlconfdata/tpm-emulator-tpm2-enc.xml | 2 +- 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index e4eb2d9ba3..402c3ab221 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8932,6 +8932,21 @@ Example: usage of external TPM emulator :since:`Since 9.0.0` This attribute only works with the ``emulator`` backend. The accepted values are ``yes`` and ``no``. :since:`Since 7.0.0` +``backup`` + The optional ``backup`` attribute controls whether swtpm creates a + backup of its TPM state file before updating it. Enabling backup + protects against TPM state file corruption if swtpm process crashes + unexpectedly. Disabling backups avoids file-copying overhead. The + accepted values are ``yes`` and ``no``. :since:`Since 12.8.0` + +``fsync`` + The optional ``fsync`` attribute controls whether swtpm explicitly + flushes its state changes to the physical storage using fsync(). + Enabling fsync ensures data remains consistent across host power loss, + but might introduces I/O latency. Disabling fsync removes this latency + at the expense of host-level crash resilience. Acceptable + values are ``yes`` and ``no``. :since:`Since 12.8.0` + ``active_pcr_banks`` The ``active_pcr_banks`` node is used to define which of the PCR banks of a TPM 2.0 to activate. Valid names are for example sha1, sha256, sha384, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1327793b0a..283658849a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11449,6 +11449,11 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, * </backend> * </tpm> * + * Emulator state backup and fsync is supported with the following: + * <tpm model='tpm-crb'> + * <backend type='emulator' version='2.0' backup='yes' fsync='yes'> + * </tpm> + * */ static virDomainTPMDef * virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, @@ -11562,6 +11567,16 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, } } + if (virXMLPropTristateBool(backends[0], "backup", + VIR_XML_PROP_NONE, + &def->data.emulator.backup) < 0) + goto error; + + if (virXMLPropTristateBool(backends[0], "fsync", + VIR_XML_PROP_NONE, + &def->data.emulator.fsync) < 0) + goto error; + if ((nnodes = virXPathNodeSet("./backend/active_pcr_banks/*", ctxt, &nodes)) < 0) break; if (nnodes > 0) @@ -26743,6 +26758,13 @@ virDomainTPMDefFormat(virBuffer *buf, } if (def->data.emulator.persistent_state) virBufferAddLit(&backendAttrBuf, " persistent_state='yes'"); + + if (def->data.emulator.backup) + virBufferAddLit(&backendAttrBuf, " backup='yes'"); + + if (def->data.emulator.fsync) + virBufferAddLit(&backendAttrBuf, " fsync='yes'"); + if (def->data.emulator.debug != 0) virBufferAsprintf(&backendAttrBuf, " debug='%u'", def->data.emulator.debug); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f7cbe7bf1d..fd022ea66f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1546,6 +1546,8 @@ struct _virDomainTPMEmulatorDef { char *name; /* name read from active profile */ virDomainTPMProfileRemoveDisabled removeDisabled; } profile; + virTristateBool backup; + virTristateBool fsync; }; struct _virDomainTPMDef { diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 0c0a3597a9..13de7ce528 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6208,6 +6208,16 @@ <ref name="uint8"/> </attribute> </optional> + <optional> + <attribute name="backup"> + <ref name="virYesNo"/> + </attribute> + </optional> + <optional> + <attribute name="fsync"> + <ref name="virYesNo"/> + </attribute> + </optional> </group> <group> <attribute name="type"> diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml index e6746d5739..515831ab8c 100644 --- a/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml +++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2-enc.xml @@ -28,7 +28,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <tpm model='tpm-tis'> - <backend type='emulator' version='2.0'> + <backend type='emulator' version='2.0' backup='yes' fsync='yes'> <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> <source type='dir' path='/some/dir'/> </backend> -- 2.54.0
From: Arun Menon <armenon@redhat.com> This commit adds the backup and fsync options while constructing the swtpm command. Both of these attributes only work with the directory backend. Refactor qemuTPMVirCommandSwtpmAddTPMState() function, use virBuffer API to construct the qemu command and use virReportError to report configuration errors. Signed-off-by: Arun Menon <armenon@redhat.com> --- src/qemu/qemu_tpm.c | 55 ++++++++++++++++++++++++++++++++++----------- src/util/virtpm.c | 2 ++ src/util/virtpm.h | 2 ++ 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 34e11cc02f..2421c0ddba 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -669,34 +669,61 @@ qemuTPMVirCommandSwtpmAddEncryption(virCommand *cmd, return 0; } -static void +static int qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, const virDomainTPMEmulatorDef *emulator, const virDomainTPMDef *tpmDef, const virQEMUDriverConfig *cfg) { - const char *lock = ",lock"; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autofree char *swtpm = NULL; + bool hasLock = virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK); - if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK)) { - if (qemuTPMHasSharedStorage(cfg, tpmDef)) - VIR_WARN("This swtpm version doesn't support explicit locking"); + swtpm = virTPMGetSwtpm(); - lock = ""; - } + if (!hasLock && qemuTPMHasSharedStorage(cfg, tpmDef)) + VIR_WARN("This swtpm version doesn't support explicit locking"); virCommandAddArg(cmd, "--tpmstate"); + switch (emulator->source_type) { case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: - virCommandAddArgFormat(cmd, "backend-uri=file://%s%s", - emulator->source_path, lock); + virBufferAsprintf(&buf, "backend-uri=file://%s", emulator->source_path); + if (hasLock) + virBufferAddLit(&buf, ",lock"); break; + case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: - virCommandAddArgFormat(cmd, "dir=%s,mode=0600%s", - emulator->source_path, lock); + virBufferAsprintf(&buf, "dir=%s,mode=0600", emulator->source_path); + if (hasLock) + virBufferAddLit(&buf, ",lock"); + + if (emulator->backup == VIR_TRISTATE_BOOL_YES) { + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_BACKUP)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%1$s does not support backup feature"), + swtpm); + return -1; + } + virBufferAddLit(&buf, ",backup"); + } + + if (emulator->fsync == VIR_TRISTATE_BOOL_YES) { + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_FSYNC)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%1$s does not support fsync feature"), + swtpm); + return -1; + } + virBufferAddLit(&buf, ",fsync"); + } break; } + + virCommandAddArgBuffer(cmd, &buf); + return 0; } /* qemuTPMEmulatorUpdateProfileName: @@ -736,7 +763,8 @@ qemuTPMEmulatorUpdateProfileName(virDomainTPMEmulatorDef *emulator, virCommandAddArgList(cmd, "socket", "--print-info", "0x20", "--tpm2", NULL); - qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator, persistentTPMDef, cfg); + if (qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator, persistentTPMDef, cfg) < 0) + return -1; if (qemuTPMVirCommandSwtpmAddEncryption(cmd, emulator, swtpm) < 0) return -1; @@ -860,7 +888,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600", tpm->data.emulator.source->data.nix.path); - qemuTPMVirCommandSwtpmAddTPMState(cmd, &tpm->data.emulator, tpm, cfg); + if (qemuTPMVirCommandSwtpmAddTPMState(cmd, &tpm->data.emulator, tpm, cfg) < 0) + goto error; virCommandAddArg(cmd, "--log"); if (tpm->data.emulator.debug != 0) diff --git a/src/util/virtpm.c b/src/util/virtpm.c index cf0f20e009..2e890f94ef 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -44,6 +44,8 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature, "nvram-backend-file", "cmdarg-print-info", "tpmstate-opt-lock", + "tpmstate-dir-backend-opt-backup", + "tpmstate-dir-backend-opt-fsync", ); VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 2892dd307e..1d2c94d36b 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -35,6 +35,8 @@ typedef enum { VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_FILE, VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO, VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK, + VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_BACKUP, + VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_FSYNC, VIR_TPM_SWTPM_FEATURE_LAST } virTPMSwtpmFeature; -- 2.54.0
participants (1)
-
Arun Menon