Re: [PATCH v3 0/6] qemu: tpm: Add support for migration across shared storage

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.

Michal