Re: [PATCH 1/2] qemu: Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags

On 10/3/22 11:20, Michal Prívozník wrote: > On 8/23/22 18:28, Stefan Berger wrote: >> Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags to >> qemuDomainUndefineFlags() >> API and --tpm and --keep-tpm to 'virsh undefine'. Pass the >> virDomainUndefineFlagsValues via qemuDomainRemoveInactive() >> from qemuDomainUndefineFlags() all the way down to >> qemuTPMEmulatorCleanupHost() and delete TPM storage there considering >> that >> the UNDEFINE_TPM flag has priority over the persistent_state attribute >> from the domain XML. Pass 0 in all other API call sites to >> qemuDomainRemoveInactive() for now. >> >> Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; >> --- >>   include/libvirt/libvirt-domain.h |  6 ++++++ >>   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        | 12 ++++++------ >>   src/qemu/qemu_process.c          |  4 ++-- >>   src/qemu/qemu_snapshot.c         |  4 ++-- >>   src/qemu/qemu_tpm.c              | 14 ++++++++++---- >>   src/qemu/qemu_tpm.h              |  3 ++- >>   tools/virsh-domain.c             | 15 +++++++++++++++ >>   12 files changed, 77 insertions(+), 35 deletions(-) >> >> diff --git a/include/libvirt/libvirt-domain.h >> b/include/libvirt/libvirt-domain.h >> index 7430a08619..5f12c673d6 100644 >> --- a/include/libvirt/libvirt-domain.h >> +++ b/include/libvirt/libvirt-domain.h >> @@ -2267,9 +2267,15 @@ typedef enum { >>       VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA = (1 << 4), /* If last >> use of domain, >>                                                               then >> also remove any >>                                                               >> checkpoint metadata (Since: 5.6.0) */ >> +    VIR_DOMAIN_UNDEFINE_TPM                = (1 << 5), /* Also >> remove any >> +                                                          TPM state >> (Since: 8.8.0) */ >> +    VIR_DOMAIN_UNDEFINE_KEEP_TPM           = (1 << 6), /* Keep TPM >> state (Since: 8.8.0) */ >> +    VIR_DOMAIN_UNDEFINE_TPM_MASK           = (3 << 5), /* TPM flags >> mask (Since: 8.8.0) */ > > I believe this _MASK is not something we want to expose to users. It's > not like both _KEEP_TPM and _TPM can be passed at the same time. I will remove it... > >> +    /* Future undefine control flags should come here. */ >>   } virDomainUndefineFlagsValues; >>     + >>   int                     virDomainUndefineFlags   (virDomainPtr domain, >>                                                     unsigned int flags); >>   int                     virConnectNumOfDefinedDomains  >> (virConnectPtr conn); >> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >> index d5fef76211..47eabd0eec 100644 >> --- a/src/qemu/qemu_domain.c >> +++ b/src/qemu/qemu_domain.c >> @@ -7143,7 +7143,8 @@ >> qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver, >>     static void >>   qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, >> -                               virDomainObj *vm) >> +                               virDomainObj *vm, >> +                               virDomainUndefineFlagsValues flags) > > I'd rather use unsigned int for flags. In the end, > qemuDomainUndefineFlags() uses uint and passes it to > qemuDomainRemoveInactive() so there's not much value in keeping the type. Should I rename flags to undefine_flags in this patch just so these flags are different from other sets of flags? To make them distinguishable was the primary reason to keep the type around.