[PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage

Stefan Berger posted 7 patches 1 year, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20221024102848.619941-1-stefanb@linux.ibm.com
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(-)
[PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Stefan Berger 1 year, 5 months ago
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(-)

-- 
2.37.3
Re: [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Michal Prívozník 1 year, 4 months ago
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
(moving a few lines of code into a different function or dropping it)
and as such I can squash them in before pushing.

Tentatively-ACKed-by: me

Michal
Re: [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Stefan Berger 1 year, 4 months ago

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...

    Stefan

> (moving a few lines of code into a different function or dropping it)
> and as such I can squash them in before pushing.
> 
> Tentatively-ACKed-by: me
> 
> Michal
> 
Re: [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Michal Prívozník 1 year, 4 months ago
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@redhat.com>

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

Re: [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Stefan Berger 1 year, 4 months ago

On 11/9/22 06:38, Michal Prívozník wrote:
> On 11/8/22 15:10, Stefan Berger wrote:
>>
>>

>>>>
>>>
>>> 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@redhat.com>
> 
> 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.
> 

I was going to tag it with v0.8.0 tomorrow or on Friday. I supposed you tried the shared storage migration a bit as well?

   Stefan

> Michal
> 
Re: [PATCH v4 0/7] qemu: tpm: Add support for migration across shared storage
Posted by Michal Prívozník 1 year, 4 months ago
On 11/9/22 12:59, Stefan Berger wrote:
> 
> 
> On 11/9/22 06:38, Michal Prívozník wrote:
>> On 11/8/22 15:10, Stefan Berger wrote:
>>>
>>>
> 
>>>>>
>>>>
>>>> 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@redhat.com>
>>
>> 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.
>>
> 
> I was going to tag it with v0.8.0 tomorrow or on Friday. 

Perfect!

> I supposed you
> tried the shared storage migration a bit as well?

Yes, I grabbed master and build swtpm from it. Worked in my testing.

Michal