On 10/21/22 17:27, Stefan Berger wrote: > > > On 10/21/22 06:55, Michal Prívozník wrote: >> On 10/18/22 19:04, 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 >>> >>> 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 (6): >>> util: Add parsing support for swtpm's cmdarg-migration capability >>> 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 | 20 ++++--- >>> src/qemu/qemu_process.c | 9 ++-- >>> src/qemu/qemu_snapshot.c | 4 +- >>> src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- >>> src/qemu/qemu_tpm.h | 12 ++++- >>> src/util/virtpm.c | 1 + >>> src/util/virtpm.h | 1 + >>> 14 files changed, 318 insertions(+), 50 deletions(-) >>> >> >> Patches look good. I did an experiment and tried to rework the code that >> patch 3/6 is not needed. You can find it here: >> >> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_rework >> >> If you don't like it, I can merge your patches as they are. Or, let me >> know if you're okay with my changes. No need to resend either way. > > > I think we need a v4. I have mine here: > https://github.com/stefanberger/libvirt-tpm/tree/swtpm_shared_storage.v4 Yep. Also I realized that we need that extra argument to qemuDomainRemoveInactive(), because in some cases we want to pass VIR_DOMAIN_UNDEFINE_TPM flag, and yet not remove any TPM state if it's on a shared storage. Alright, I've uploaded my fixup commits here: https://gitlab.com/MichalPrivoznik/libvirt/-/tree/tpm_migration_rework Can you work in those fixup commits and resend? Michal
