1. Patchew
  2. Libvirt
  3. [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
  4. View patch

[PATCH v4 2/7] qemu: tpm: Allow offline migration with TPM_EMULATOR only with shared storage

Stefan Berger posted 7 patches 3 years, 3 months ago
[PATCH v4 2/7] qemu: tpm: Allow offline migration with TPM_EMULATOR only with shared storage
Posted by Stefan Berger 3 years, 3 months ago 
Allow migration with TPM_EMULATOR (swtpm) only if shared storage has been
set up for the TPM state directory.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/qemu/qemu_migration.c |  6 ++++++
 src/qemu/qemu_tpm.c       | 28 ++++++++++++++++++++++++++++
 src/qemu/qemu_tpm.h       |  5 +++++
 3 files changed, 39 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 33105cf07b..16bf7ac178 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -38,6 +38,7 @@
 #include "qemu_security.h"
 #include "qemu_slirp.h"
 #include "qemu_block.h"
+#include "qemu_tpm.h"
 
 #include "domain_audit.h"
 #include "virlog.h"
@@ -2579,6 +2580,11 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
                            _("tunnelled offline migration does not make sense"));
             return NULL;
         }
+        if (qemuTPMHasSharedStorage(driver, vm->def) != 1) {
+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("offline migration requires TPM state directory to be on shared storage"));
+            return NULL;
+        }
     }
 
     if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) {
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index dc09c94a4d..5f89a6bb18 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -954,6 +954,34 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
 }
 
 
+int
+qemuTPMHasSharedStorage(virQEMUDriver *driver,
+                        virDomainDef *def)
+{
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    size_t i;
+
+    for (i = 0; i < def->ntpms; i++) {
+        virDomainTPMDef *tpm = def->tpms[i];
+
+        switch (tpm->type) {
+        case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+            if (qemuTPMEmulatorInitPaths(tpm,
+                                         cfg->swtpmStorageDir,
+                                         cfg->swtpmLogDir,
+                                         def->name,
+                                         def->uuid) < 0)
+                return -1;
+            return virFileIsSharedFS(tpm->data.emulator.storagepath);
+        case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+        case VIR_DOMAIN_TPM_TYPE_LAST:
+        }
+    }
+
+    return 0;
+}
+
+
 /* ---------------------
  *  Module entry points
  * ---------------------
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index f068f3ca5a..531d93846b 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -56,3 +56,8 @@ int qemuExtTPMSetupCgroup(virQEMUDriver *driver,
                           virCgroup *cgroup)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
     G_GNUC_WARN_UNUSED_RESULT;
+
+int qemuTPMHasSharedStorage(virQEMUDriver *driver,
+                            virDomainDef *def)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+    G_GNUC_WARN_UNUSED_RESULT;
-- 
2.37.3
Re: [PATCH v4 2/7] qemu: tpm: Allow offline migration with TPM_EMULATOR only with shared storage
Posted by Michal Prívozník 3 years, 3 months ago 
On 10/24/22 12:28, Stefan Berger wrote:
> Allow migration with TPM_EMULATOR (swtpm) only if shared storage has been
> set up for the TPM state directory.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  src/qemu/qemu_migration.c |  6 ++++++
>  src/qemu/qemu_tpm.c       | 28 ++++++++++++++++++++++++++++
>  src/qemu/qemu_tpm.h       |  5 +++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 33105cf07b..16bf7ac178 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -38,6 +38,7 @@
>  #include "qemu_security.h"
>  #include "qemu_slirp.h"
>  #include "qemu_block.h"
> +#include "qemu_tpm.h"
>  
>  #include "domain_audit.h"
>  #include "virlog.h"
> @@ -2579,6 +2580,11 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
>                             _("tunnelled offline migration does not make sense"));
>              return NULL;
>          }
> +        if (qemuTPMHasSharedStorage(driver, vm->def) != 1) {
> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                           _("offline migration requires TPM state directory to be on shared storage"));

Does it though? We don't have such requirement for say domain disks. I
believe the point of --offline is to merely just get domain XML an
define it on the destination. That's why --offline requires --persistent
and why a lot of checks are skipped when --offline.

I mean, for disks we just assume users will copy them onto destination
(or use a shared FS). We can assume the same for TPM, can't we? This
would also allow us to simplify qemuTPMHasSharedStorage() because it no
longer needs to call qemuTPMEmulatorInitPaths() because for running
guests the paths were already initialized.

And if we chose to keep this check in, then I think it should live in
qemuMigrationSrcIsAllowed() which is the places where similar checks are
performed.

Michal
Re: [PATCH v4 2/7] qemu: tpm: Allow offline migration with TPM_EMULATOR only with shared storage
Posted by Stefan Berger 3 years, 3 months ago 

On 11/7/22 03:31, Michal Prívozník wrote:
> On 10/24/22 12:28, Stefan Berger wrote:
>> Allow migration with TPM_EMULATOR (swtpm) only if shared storage has been
>> set up for the TPM state directory.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   src/qemu/qemu_migration.c |  6 ++++++
>>   src/qemu/qemu_tpm.c       | 28 ++++++++++++++++++++++++++++
>>   src/qemu/qemu_tpm.h       |  5 +++++
>>   3 files changed, 39 insertions(+)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index 33105cf07b..16bf7ac178 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -38,6 +38,7 @@
>>   #include "qemu_security.h"
>>   #include "qemu_slirp.h"
>>   #include "qemu_block.h"
>> +#include "qemu_tpm.h"
>>   
>>   #include "domain_audit.h"
>>   #include "virlog.h"
>> @@ -2579,6 +2580,11 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
>>                              _("tunnelled offline migration does not make sense"));
>>               return NULL;
>>           }
>> +        if (qemuTPMHasSharedStorage(driver, vm->def) != 1) {
>> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> +                           _("offline migration requires TPM state directory to be on shared storage"));
> 
> Does it though? We don't have such requirement for say domain disks. I
> believe the point of --offline is to merely just get domain XML an
> define it on the destination. That's why --offline requires --persistent
> and why a lot of checks are skipped when --offline.
> 
> I mean, for disks we just assume users will copy them onto destination
> (or use a shared FS). We can assume the same for TPM, can't we? This
> would also allow us to simplify qemuTPMHasSharedStorage() because it no
> longer needs to call qemuTPMEmulatorInitPaths() because for running
> guests the paths were already initialized.
> 
> And if we chose to keep this check in, then I think it should live in
> qemuMigrationSrcIsAllowed() which is the places where similar checks are
> performed.

I don't mind dropping the patch then.

   Stefan

> 
> Michal
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.