On 11/8/22 15:10, Stefan Berger wrote: > > > On 11/7/22 03:31, Michal Prívozník wrote: >> On 10/24/22 12:28, Stefan Berger wrote: >>> This series of patches adds support for migrating vTPMs across hosts >>> whose >>> storage has been set up to share the directory structure holding the >>> state >>> of the TPM (swtpm). The existence of share storage influences the >>> management of the directory structure holding the TPM state, which for >>> example is only removed when a domain is undefined and not when a VM is >>> removed on the migration source host. Further, when shared storage is >>> used >>> then security labeling on the destination side is skipped assuming >>> that the >>> labeling was already done on the source side. >>> >>> I have tested this with an NFS setup where I had to turn SELinux off on >>> the hosts since the SELinux MLS range labeling is not supported by NFS. >>> >>> For shared storage support to work properly both sides of the migration >>> need to be clients of the shared storage setup, meaning that they >>> both have >>> to have /var/lib/libvirt/swtpm mounted as shared storage, because other- >>> wise the virFileIsSharedFS() may not detect shared storage and in the >>> worst case may cause the TPM emulator (swtpm) to malfunction if for >>> example the source side removed the TPM state directory structure. >>> >>> Shared storage migration requires (upcoming) swtpm v0.8. >>> >>> Stefan >>> >>> v4: >>> - Fixed long-standing bug regarding offline migration that now >>> blocks if >>> no shared storage is set up >>> - Addressed Michal's concerns >>> >>> v3: >>> - Relying entirely on virFileIsSharedFS() on migration source and >>> destination sides to detect whether shared storage is set up >>> between >>> hosts; no more hint about shared storage from user via flag >>> - Added support for virDomainTPMPrivate structure to store and >>> persist >>> TPM-related private data >>> >>> Stefan Berger (7): >>> util: Add parsing support for swtpm's cmdarg-migration capability >>> qemu: tpm: Allow offline migration with TPM_EMULATOR only with shared >>> storage >>> qemu: tpm: Conditionally create storage on incoming migration >>> qemu: tpm: Add support for storing private TPM-related data >>> qemu: tpm: Pass --migration option to swtpm if supported and needed >>> qemu: tpm: Avoid security labels on incoming migration with shared >>> storage >>> qemu: tpm: Never remove state on outgoing migration and shared >>> storage >>> >>> src/conf/domain_conf.c | 63 ++++++++++++++++++-- >>> src/conf/domain_conf.h | 9 +++ >>> src/qemu/qemu_domain.c | 85 ++++++++++++++++++++++++-- >>> src/qemu/qemu_domain.h | 17 +++++- >>> src/qemu/qemu_driver.c | 20 +++---- >>> src/qemu/qemu_extdevice.c | 10 ++-- >>> src/qemu/qemu_extdevice.h | 6 +- >>> src/qemu/qemu_migration.c | 28 +++++++-- >>> src/qemu/qemu_process.c | 9 ++- >>> src/qemu/qemu_snapshot.c | 4 +- >>> src/qemu/qemu_tpm.c | 122 ++++++++++++++++++++++++++++++++++---- >>> src/qemu/qemu_tpm.h | 14 ++++- >>> src/util/virtpm.c | 1 + >>> src/util/virtpm.h | 1 + >>> 14 files changed, 339 insertions(+), 50 deletions(-) >>> >> >> Hey, sorry for late review. I just have to small nits. I surely want to >> avoid making you send another version. My suggestions are trivial > > How is it going to work without another version. I am fine with you > making the changes but I can also send another version... I'll do the changes just before pushing. Fortunately, we are not doing merge requests where this would be more complicated. Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> and pushed. Thank you for your patience. BTW: do you have any plans of making a release of swtpm in near future? I think it may be worth it so that distros can pick up this new feature. Michal