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.
qemuTPMEmulatorDeleteStorage(tpm);
+ } else if (!tpm->data.emulator.persistent_state &&
+ (flags & VIR_DOMAIN_UNDEFINE_TPM_MASK) == 0) {
+ qemuTPMEmulatorDeleteStorage(tpm);
Here, we should do something similar to NVRAM: fail if the storage
exists and KEEP_TPM wasn't specified. Which is going to break existing
workloads where undefine worked even without the flag. So maybe just
need this?
if ((tpm->data.emulator.persistent_state && flags &
VIR_DOMAIN_UNDEFINE_TPM) ||
!tpm->data.emulator.persistent_state && !(flags &
VIR_DOMAIN_UNDEFINE_KEEP_TPM)) {
qemuTPMEmulatorDeleteStorage(tpm);
}
I'll have a look at this.
+ }
}
@@ -993,9 +998,10 @@ qemuExtTPMPrepareHost(virQEMUDriver *driver,
void
-qemuExtTPMCleanupHost(virDomainTPMDef *tpm)
+qemuExtTPMCleanupHost(virDomainTPMDef *tpm,
+ virDomainUndefineFlagsValues flags)
{
- qemuTPMEmulatorCleanupHost(tpm);
+ qemuTPMEmulatorCleanupHost(tpm, flags);
}
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index 9951f025a6..f068f3ca5a 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -35,7 +35,8 @@ int qemuExtTPMPrepareHost(virQEMUDriver *driver,
ATTRIBUTE_NONNULL(3)
G_GNUC_WARN_UNUSED_RESULT;
-void qemuExtTPMCleanupHost(virDomainTPMDef *tpm)
+void qemuExtTPMCleanupHost(virDomainTPMDef *tpm,
+ virDomainUndefineFlagsValues flags)
ATTRIBUTE_NONNULL(1);
int qemuExtTPMStart(virQEMUDriver *driver,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d2ea4d1c7b..ff9c081d71 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3650,6 +3650,14 @@ static const vshCmdOptDef opts_undefine[] = {
.type = VSH_OT_BOOL,
.help = N_("keep nvram file")
},
+ {.name = "tpm",
+ .type = VSH_OT_BOOL,
+ .help = N_("remove TPM state")
+ },
+ {.name = "keep-tpm",
+ .type = VSH_OT_BOOL,
+ .help = N_("keep TPM state")
+ },
These new arguments should be documented in virsh manpage.
Yes, right.
Stefan
Michal
