Hi Peter, Thanks for the review. On Mon, Sep 07, 2026 at 01:04:51PM +0200, Peter Krempa wrote: > On Mon, Sep 07, 2026 at 01:24:10 +0530, Arun Menon via Devel wrote: > > From: Arun Menon <[email protected]> > > > > This commit adds the backup and fsync options while constructing the > > swtpm command. These attributes only work with the directory backend. > > > > Signed-off-by: Arun Menon <[email protected]> > > --- > > docs/formatdomain.rst | 16 ++++++++++++++++ > > src/qemu/qemu_tpm.c | 17 +++++++++++++++-- > > src/util/virtpm.c | 2 ++ > > src/util/virtpm.h | 2 ++ > > 4 files changed, 35 insertions(+), 2 deletions(-) > > > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > > index 5a278f3717..e0b090b8cc 100644 > > --- a/docs/formatdomain.rst > > +++ b/docs/formatdomain.rst > > @@ -8923,6 +8923,22 @@ 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 ``backup`` attribute indicates whether a backup of the 'swtpm' TPM > > + state is taken or not. This option can be used also for restoring > > + the state from the backup. By default the value is ``no``. > > + This attribute only works with the directory backend. The accepted > > values > > + are ``yes`` and ``no``. :since:`Since 12.8.0` > > + > > +``fsync`` > > + The ``fsync`` attribute indicates whether an fsync will be called on the > > + file and the directory of the file when the 'swtpm' TPM state is written > > + to disk. This option ensures that all data have been successfully > > written > > + to physical storage before the TPM processes the next command. It lowers > > + the probability of TPM state file corruption in case of a power loss. By > > + default the value is ``no``. This attribute only works with the > > directory > > + backend. The accepted 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/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c > > index 34e11cc02f..379a590c0c 100644 > > --- a/src/qemu/qemu_tpm.c > > +++ b/src/qemu/qemu_tpm.c > > @@ -676,6 +676,8 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, > > const virQEMUDriverConfig *cfg) > > { > > const char *lock = ",lock"; > > + const char *backup = ",backup"; > > + const char *fsync = ",fsync"; > > > > if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK)) { > > if (qemuTPMHasSharedStorage(cfg, tpmDef)) > > @@ -684,6 +686,17 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, > > lock = ""; > > } > > > > + if (emulator->source_type == VIR_DOMAIN_TPM_SOURCE_TYPE_DIR) { > > + if > > (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_BACKUP)) { > > + VIR_WARN("This swtpm version doesn't support state file backup > > feature"); > > + backup = ""; > > + } > > + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_FSYNC)) > > { > > + VIR_WARN("This swtpm version doesn't support state file fsync > > feature"); > > This should be a hard error. Not honouring user's config shoudn't > happen. > > Additionally VIR_WARN is usually wrong. It only logs an entry which will > get ignored by users. Don't use it, use VIR_DEGBUG for debug logs or > report proper errors. The only acceptable place for VIR_WARN is on > cleanup paths that don't have a reasonable way to report errors to the > user. >
Agreed. I will use VIR_ERR_ARGUMENT_UNSUPPORTED. Thank you. Regards, Arun Menon
