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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/3/22 21:39, Stefan Berger wrote:
> 
> 
> 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@xxxxxxxxxxxxx>
>>> ---
>>>   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.

Good point. Leave the type then.

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux