Re: [PATCH 3/3] qemu_tpm: Only warn about missing locking feature on shared filesystems

[Martin Kletzander via Devel]

On Fri, Jul 18, 2025 at 11:03:56AM +0200, Pavel Hrdina wrote:

On Thu, Jul 17, 2025 at 12:34:43PM +0200, Martin Kletzander via Devel wrote:

From: Martin Kletzander <mklet...@redhat.com>

The warning pollutes the logs and might give a bad impression on someone
reading them even though the locking is not always needed.  This way we
at least limit the logging in unnecessary cases.

Resolves: https://issues.redhat.com/browse/RHEL-80155
Signed-off-by: Martin Kletzander <mklet...@redhat.com>
---
 src/qemu/qemu_tpm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 855d732e60d0..cdbd6e3993b2 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -660,12 +660,16 @@ qemuTPMVirCommandSwtpmAddEncryption(virCommand *cmd,

 static void
 qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd,
-                                  const virDomainTPMEmulatorDef *emulator)
+                                  const virDomainTPMEmulatorDef *emulator,
+                                  const virDomainTPMDef *tpmDef,
+                                  const virQEMUDriverConfig *cfg)
 {
     const char *lock = ",lock";

     if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_TPMSTATE_OPT_LOCK)) {
-        VIR_WARN("This swtpm version doesn't support explicit locking");
+        if (qemuTPMHasSharedStorage(cfg, tpmDef))
+            VIR_WARN("This swtpm version doesn't support explicit locking");
+
         lock = "";
     }

@@ -721,7 +725,7 @@ qemuTPMEmulatorUpdateProfileName(virDomainTPMEmulatorDef 
*emulator,

     virCommandAddArgList(cmd, "socket", "--print-info", "0x20", "--tpm2", 
NULL);

-    qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator);
+    qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator, persistentTPMDef, cfg);

     if (qemuTPMVirCommandSwtpmAddEncryption(cmd, emulator, swtpm) < 0)
         return -1;
@@ -848,7 +852,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
     virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
                            tpm->data.emulator.source->data.nix.path);

-    qemuTPMVirCommandSwtpmAddTPMState(cmd, &tpm->data.emulator);
+    qemuTPMVirCommandSwtpmAddTPMState(cmd, &tpm->data.emulator, 
persistentTPMDef, cfg);

Coverity complains here that the `persistentTPMDef` can be NULL and that
it is dereferenced in `qemuTPMHasSharedStorage`.

This is called from `qemuExtDevicesStart` where the for loop calling
`qemuExtTPMStart` can actually pass NULL.

Not sure if it can happen with any real VM.

It can, yes.  I have a fix ready.  Thanks for catching that.

Pavel

     virCommandAddArg(cmd, "--log");
     if (tpm->data.emulator.debug != 0)
--
2.50.1

signature.asc
Description: PGP signature