[v3] qemu: tpm: do not update profile name for transient domains

[libvirt PATCH v3] qemu: tpm: do not update profile name for transient domains

Posted by Ján Tomko 1 week, 3 days ago

If we do not have a persistent definition, there's no point in
looking for it since we cannot store it.

Also skip the update if the tpm device(s) in the persistent
definition are different.

This fixes the crash when starting a transient domain.

https://issues.redhat.com/browse/RHEL-69774

Fixes: d79542eec669eb9c449bb8228179e7a87e768017
Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_extdevice.c | 13 ++++++++++++-
 src/qemu/qemu_tpm.c       |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index a6f31f9773..954cb323a4 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -190,7 +190,18 @@ qemuExtDevicesStart(virQEMUDriver *driver,
 
     for (i = 0; i < def->ntpms; i++) {
         virDomainTPMDef *tpm = def->tpms[i];
-        virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i];
+        virDomainTPMDef *persistentTPMDef = NULL;
+
+        if (persistentDef) {
+            /* do not try to update the profile in the persistent definition
+             * if the device does not match */
+            if (persistentDef->ntpms == def->ntpms)
+                persistentTPMDef = persistentDef->tpms[i];
+            if (persistentTPMDef &&
+                (persistentTPMDef->type != tpm->type ||
+                 persistentTPMDef->model != tpm->model))
+                persistentTPMDef = NULL;
+        }
 
         if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
             qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index f223dcb9ae..f5e0184e54 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -773,7 +773,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
                                 incomingMigration) < 0)
         goto error;
 
-    if (run_setup && !incomingMigration &&
+    if (run_setup && !incomingMigration && persistentTPMDef &&
         qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef,
                                          cfg, saveDef) < 0)
         goto error;
-- 
2.47.0

Re: [libvirt PATCH v3] qemu: tpm: do not update profile name for transient domains

Posted by Stefan Berger 1 week, 2 days ago


On 12/11/24 3:46 AM, Ján Tomko wrote:
> If we do not have a persistent definition, there's no point in
> looking for it since we cannot store it.
> 
> Also skip the update if the tpm device(s) in the persistent
> definition are different.
> 
> This fixes the crash when starting a transient domain.
> 
> https://issues.redhat.com/browse/RHEL-69774
> 
> Fixes: d79542eec669eb9c449bb8228179e7a87e768017
> Signed-off-by: Ján Tomko <jtomko@redhat.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   src/qemu/qemu_extdevice.c | 13 ++++++++++++-
>   src/qemu/qemu_tpm.c       |  2 +-
>   2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index a6f31f9773..954cb323a4 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -190,7 +190,18 @@ qemuExtDevicesStart(virQEMUDriver *driver,
>   
>       for (i = 0; i < def->ntpms; i++) {
>           virDomainTPMDef *tpm = def->tpms[i];
> -        virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i];
> +        virDomainTPMDef *persistentTPMDef = NULL;
> +
> +        if (persistentDef) {
> +            /* do not try to update the profile in the persistent definition
> +             * if the device does not match */
> +            if (persistentDef->ntpms == def->ntpms)
> +                persistentTPMDef = persistentDef->tpms[i];
> +            if (persistentTPMDef &&
> +                (persistentTPMDef->type != tpm->type ||
> +                 persistentTPMDef->model != tpm->model))
> +                persistentTPMDef = NULL;
> +        }
>   
>           if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
>               qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index f223dcb9ae..f5e0184e54 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -773,7 +773,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>                                   incomingMigration) < 0)
>           goto error;
>   
> -    if (run_setup && !incomingMigration &&
> +    if (run_setup && !incomingMigration && persistentTPMDef &&
>           qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef,
>                                            cfg, saveDef) < 0)
>           goto error;

Re: [libvirt PATCH v3] qemu: tpm: do not update profile name for transient domains

Posted by Peter Krempa 1 week, 2 days ago

On Wed, Dec 11, 2024 at 09:46:49 +0100, Ján Tomko wrote:
> If we do not have a persistent definition, there's no point in
> looking for it since we cannot store it.
> 
> Also skip the update if the tpm device(s) in the persistent
> definition are different.
> 
> This fixes the crash when starting a transient domain.
> 
> https://issues.redhat.com/browse/RHEL-69774
> 
> Fixes: d79542eec669eb9c449bb8228179e7a87e768017

As even people upstream now seem to be hitting this when installing via
virt-manager (Which seems to use a transient config during install)

https://gitlab.com/libvirt/libvirt/-/issues/715

I suggest you also add a NEWS entry soon so that we can refer to it.

Re: [libvirt PATCH v3] qemu: tpm: do not update profile name for transient domains

Posted by Jiri Denemark 1 week, 3 days ago

On Wed, Dec 11, 2024 at 09:46:49 +0100, Ján Tomko wrote:
> If we do not have a persistent definition, there's no point in
> looking for it since we cannot store it.
> 
> Also skip the update if the tpm device(s) in the persistent
> definition are different.
> 
> This fixes the crash when starting a transient domain.
> 
> https://issues.redhat.com/browse/RHEL-69774
> 
> Fixes: d79542eec669eb9c449bb8228179e7a87e768017
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_extdevice.c | 13 ++++++++++++-
>  src/qemu/qemu_tpm.c       |  2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>