1. Patchew
  2. Libvirt
  3. View series

[PATCH v2 0/2] qemu: tpm: Improve TPM state files management

Stefan Berger posted 2 patches 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20221004133814.3304915-1-stefanb@linux.ibm.com
docs/manpages/virsh.rst          |  6 ++++++
include/libvirt/libvirt-domain.h |  4 ++++
src/qemu/qemu_domain.c           | 12 +++++++-----
src/qemu/qemu_domain.h           |  3 ++-
src/qemu/qemu_driver.c           | 31 ++++++++++++++++++++-----------
src/qemu/qemu_extdevice.c        |  5 +++--
src/qemu/qemu_extdevice.h        |  3 ++-
src/qemu/qemu_migration.c        | 13 +++++++------
src/qemu/qemu_process.c          |  4 ++--
src/qemu/qemu_snapshot.c         |  4 ++--
src/qemu/qemu_tpm.c              | 18 ++++++++++++++----
src/qemu/qemu_tpm.h              |  3 ++-
tools/virsh-domain.c             | 15 +++++++++++++++
13 files changed, 86 insertions(+), 35 deletions(-)
[PATCH v2 0/2] qemu: tpm: Improve TPM state files management
Posted by Stefan Berger 1 year, 6 months ago 
This series of patches adds the --keep-tpm and --tpm flags to virsh for
keeping and removing the TPM state directory structure when a VM is
undefined. It also fixes the removal of state when a VM is migrated so
that the state files are removed on the source upon successful
migration and deleted on the destination after migration failure.

Regards,
   Stefan

v2:
  - addressed Michal's comments on v1

Stefan Berger (2):
  qemu: Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags
  qemu: tpm: Remove TPM state after successful migration

 docs/manpages/virsh.rst          |  6 ++++++
 include/libvirt/libvirt-domain.h |  4 ++++
 src/qemu/qemu_domain.c           | 12 +++++++-----
 src/qemu/qemu_domain.h           |  3 ++-
 src/qemu/qemu_driver.c           | 31 ++++++++++++++++++++-----------
 src/qemu/qemu_extdevice.c        |  5 +++--
 src/qemu/qemu_extdevice.h        |  3 ++-
 src/qemu/qemu_migration.c        | 13 +++++++------
 src/qemu/qemu_process.c          |  4 ++--
 src/qemu/qemu_snapshot.c         |  4 ++--
 src/qemu/qemu_tpm.c              | 18 ++++++++++++++----
 src/qemu/qemu_tpm.h              |  3 ++-
 tools/virsh-domain.c             | 15 +++++++++++++++
 13 files changed, 86 insertions(+), 35 deletions(-)

-- 
2.37.3
Re: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
Posted by Michal Prívozník 1 year, 6 months ago 
On 10/4/22 15:38, Stefan Berger wrote:
> This series of patches adds the --keep-tpm and --tpm flags to virsh for
> keeping and removing the TPM state directory structure when a VM is
> undefined. It also fixes the removal of state when a VM is migrated so
> that the state files are removed on the source upon successful
> migration and deleted on the destination after migration failure.
> 
> Regards,
>    Stefan
> 
> v2:
>   - addressed Michal's comments on v1
> 
> Stefan Berger (2):
>   qemu: Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags
>   qemu: tpm: Remove TPM state after successful migration
> 
>  docs/manpages/virsh.rst          |  6 ++++++
>  include/libvirt/libvirt-domain.h |  4 ++++
>  src/qemu/qemu_domain.c           | 12 +++++++-----
>  src/qemu/qemu_domain.h           |  3 ++-
>  src/qemu/qemu_driver.c           | 31 ++++++++++++++++++++-----------
>  src/qemu/qemu_extdevice.c        |  5 +++--
>  src/qemu/qemu_extdevice.h        |  3 ++-
>  src/qemu/qemu_migration.c        | 13 +++++++------
>  src/qemu/qemu_process.c          |  4 ++--
>  src/qemu/qemu_snapshot.c         |  4 ++--
>  src/qemu/qemu_tpm.c              | 18 ++++++++++++++----
>  src/qemu/qemu_tpm.h              |  3 ++-
>  tools/virsh-domain.c             | 15 +++++++++++++++
>  13 files changed, 86 insertions(+), 35 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal
Re: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
Posted by Stefan Berger 1 year, 6 months ago 

On 10/4/22 10:39, Michal Prívozník wrote:

>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> and pushed.

Thanks.

Regarding shared storage and migration. Should shared storage support be implemented using an XML attribute or through a new  migration option (--tpm-shared-storage)? The previously proposed implementation used an XML attribute that may be easier to accommodate by higher layers that then don't need to support a new migration option just a slighly different XML but that may not be how it should be done...

   Stefan

> 
> Michal
>
Re: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
Posted by Michal Prívozník 1 year, 6 months ago 
On 10/4/22 17:33, Stefan Berger wrote:
> 
> 
> On 10/4/22 10:39, Michal Prívozník wrote:
> 
>>>
>>
>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>>
>> and pushed.
> 
> Thanks.
> 
> Regarding shared storage and migration. Should shared storage support be
> implemented using an XML attribute or through a new  migration option
> (--tpm-shared-storage)? The previously proposed implementation used an
> XML attribute that may be easier to accommodate by higher layers that
> then don't need to support a new migration option just a slighly
> different XML but that may not be how it should be done...

Yeah, I've been meaning to look into that discussion and patches. I
don't know if it was suggested, but maybe a flag to migration APIs? We
do storage migration that way. Or do we need to pass the flag to swtpm
even way before migration is started (e.g. on domain startup)?

Michal
Re: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
Posted by Stefan Berger 1 year, 6 months ago 

On 10/4/22 11:48, Michal Prívozník wrote:
> On 10/4/22 17:33, Stefan Berger wrote:
>>
>>
>> On 10/4/22 10:39, Michal Prívozník wrote:
>>
>>>>
>>>
>>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>>>
>>> and pushed.
>>
>> Thanks.
>>
>> Regarding shared storage and migration. Should shared storage support be
>> implemented using an XML attribute or through a new  migration option
>> (--tpm-shared-storage)? The previously proposed implementation used an
>> XML attribute that may be easier to accommodate by higher layers that
>> then don't need to support a new migration option just a slighly
>> different XML but that may not be how it should be done...
> 
> Yeah, I've been meaning to look into that discussion and patches. I
> don't know if it was suggested, but maybe a flag to migration APIs? We
> do storage migration that way. Or do we need to pass the flag to swtpm
> even way before migration is started (e.g. on domain startup)?

I think that should not be necessary.

swtpm now has options to support shared storage setups:
--migration [incoming][,release-lock-outgoing]
                  : Incoming migration defers locking of storage backend
                    until the TPM state is received; release-lock-outgoing
                    releases the storage lock on outgoing migration

If we need to support shared storage migration using an option then we will have to add --migration release-lock-outgoing to the command line of every instance of swtpm that 's being started and that supports this option. When migration then is done across shared storage using the new command line option then we have to query on source and destination whether the above options are indeed supported and if that's not the case on either side reject/fail the migration. So, an older versions of swtpm on either side will cause a rejection of the migration across shared storage then as expected...

I suppose migration flags passed on the source side will become available on the destination side. Need to test this...



> 
> Michal
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.