[PATCH v2 0/2] qemu: tpm: Improve TPM state files management
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
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@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 | 12 ++++++------ 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, 85 insertions(+), 35 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 45469f2f35..5d11c48803 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4486,6 +4486,7 @@ undefine [--checkpoints-metadata] [--nvram] [--keep-nvram] [ {--storage volumes | --remove-all-storage [--delete-storage-volume-snapshots]} --wipe-storage] + [--tpm] [--keep-tpm] Undefine a domain. If the domain is running, this converts it to a transient domain, without stopping it. If the domain is inactive, @@ -4537,6 +4538,11 @@ failure. The flag *--wipe-storage* specifies that the storage volumes should be wiped before removal. +*--tpm* and *--keep-tpm* specify accordingly to delete or keep a TPM's +persistent state directory structure and files. If the flags are omitted +then the persistent_state attribute in the TPM emulator definition in the +domain XML determines whether the TPM state is kept. + NOTE: For an inactive domain, the domain name or UUID must be used as the *domain*. diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7430a08619..8357aea797 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2267,6 +2267,10 @@ 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.9.0) */ + VIR_DOMAIN_UNDEFINE_KEEP_TPM = (1 << 6), /* Keep TPM state (Since: 8.9.0) */ + /* Future undefine control flags should come here. */ } virDomainUndefineFlagsValues; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee024d17cd..858d14af6a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7168,7 +7168,8 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver, static void qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + virDomainUndefineFlagsValues flags) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *snapDir = NULL; @@ -7194,7 +7195,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, if (rmdir(chkDir) < 0 && errno != ENOENT) VIR_WARN("unable to remove checkpoint directory %s", chkDir); } - qemuExtDevicesCleanupHost(driver, vm->def); + qemuExtDevicesCleanupHost(driver, vm->def, flags); } @@ -7205,14 +7206,15 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, */ void qemuDomainRemoveInactive(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + virDomainUndefineFlagsValues flags) { if (vm->persistent) { /* Short-circuit, we don't want to remove a persistent domain */ return; } - qemuDomainRemoveInactiveCommon(driver, vm); + qemuDomainRemoveInactiveCommon(driver, vm, flags); virDomainObjListRemove(driver->domains, vm); } @@ -7234,7 +7236,7 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, return; } - qemuDomainRemoveInactiveCommon(driver, vm); + qemuDomainRemoveInactiveCommon(driver, vm, 0); virDomainObjListRemoveLocked(driver->domains, vm); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ef149b9fa9..a22deaf113 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -681,7 +681,8 @@ int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver, virDomainObj *vm); void qemuDomainRemoveInactive(virQEMUDriver *driver, - virDomainObj *vm); + virDomainObj *vm, + virDomainUndefineFlagsValues flags); void qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3db4592945..40d23b5723 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1626,7 +1626,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, goto cleanup; if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, flags) < 0) { - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); goto cleanup; } @@ -1635,7 +1635,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); qemuProcessEndJob(vm); goto cleanup; } @@ -2118,7 +2118,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: if (ret == 0) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); virDomainObjEndJob(vm); cleanup: @@ -2738,7 +2738,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, } virDomainObjEndAsyncJob(vm); if (ret == 0) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); cleanup: virQEMUSaveDataFree(data); @@ -3278,7 +3278,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, virDomainObjEndAsyncJob(vm); if (ret == 0 && flags & VIR_DUMP_CRASH) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); cleanup: virDomainObjEndAPI(&vm); @@ -3590,7 +3590,7 @@ processGuestPanicEvent(virQEMUDriver *driver, endjob: virDomainObjEndAsyncJob(vm); if (removeInactive) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } @@ -4068,7 +4068,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); endjob: - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); virDomainObjEndJob(vm); } @@ -6000,7 +6000,7 @@ qemuDomainRestoreInternal(virConnectPtr conn, virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); if (vm && ret < 0) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); virDomainObjEndAPI(&vm); return ret; } @@ -6690,7 +6690,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, } else { /* Brand new domain. Remove it */ VIR_INFO("Deleting domain '%s'", vm->def->name); - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } } @@ -6723,7 +6723,9 @@ qemuDomainUndefineFlags(virDomainPtr dom, VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA | VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA | VIR_DOMAIN_UNDEFINE_NVRAM | - VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, -1); + VIR_DOMAIN_UNDEFINE_KEEP_NVRAM | + VIR_DOMAIN_UNDEFINE_TPM | + VIR_DOMAIN_UNDEFINE_KEEP_TPM, -1); if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM) && (flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) { @@ -6732,6 +6734,13 @@ qemuDomainUndefineFlags(virDomainPtr dom, return -1; } + if ((flags & VIR_DOMAIN_UNDEFINE_TPM) && + (flags & VIR_DOMAIN_UNDEFINE_KEEP_TPM)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot both keep and delete TPM")); + return -1; + } + if (!(vm = qemuDomainObjFromDomain(dom))) return -1; @@ -6830,7 +6839,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, */ vm->persistent = 0; if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, flags); ret = 0; endjob: diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index b8e3c1000a..24a57b0f74 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -151,7 +151,8 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, void qemuExtDevicesCleanupHost(virQEMUDriver *driver, - virDomainDef *def) + virDomainDef *def, + virDomainUndefineFlagsValues flags) { size_t i; @@ -159,7 +160,7 @@ qemuExtDevicesCleanupHost(virQEMUDriver *driver, return; for (i = 0; i < def->ntpms; i++) { - qemuExtTPMCleanupHost(def->tpms[i]); + qemuExtTPMCleanupHost(def->tpms[i], flags); } } diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 43d2a4dfff..6b05b59cd6 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -41,7 +41,8 @@ int qemuExtDevicesPrepareHost(virQEMUDriver *driver, G_GNUC_WARN_UNUSED_RESULT; void qemuExtDevicesCleanupHost(virQEMUDriver *driver, - virDomainDef *def) + virDomainDef *def, + virDomainUndefineFlagsValues flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtDevicesStart(virQEMUDriver *driver, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c63b00c922..126a4f6d38 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3391,7 +3391,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, * and there is no 'goto cleanup;' in the middle of those */ VIR_FREE(priv->origname); virDomainObjRemoveTransientDef(vm); - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } virDomainObjEndAPI(&vm); virErrorRestore(&origErr); @@ -4036,7 +4036,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } cleanup: @@ -6039,7 +6039,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } virErrorRestore(&orig_err); @@ -6166,7 +6166,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); return ret; } @@ -6702,7 +6702,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); virErrorRestore(&orig_err); return NULL; @@ -6839,7 +6839,7 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 998f4aa63c..97336e2622 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8442,7 +8442,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - qemuDomainRemoveInactive(driver, dom); + qemuDomainRemoveInactive(driver, dom, 0); virDomainObjEndJob(dom); @@ -8905,7 +8905,7 @@ qemuProcessReconnect(void *opaque) if (jobStarted) virDomainObjEndJob(obj); if (!virDomainObjIsActive(obj)) - qemuDomainRemoveInactive(driver, obj); + qemuDomainRemoveInactive(driver, obj, 0); virDomainObjEndAPI(&obj); virIdentitySetCurrent(NULL); return; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index d2835ab1a8..06b5c180ff 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2103,7 +2103,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, } if (qemuSnapshotInternalRevertInactive(driver, vm, snap) < 0) { - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); return -1; } @@ -2125,7 +2125,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm, 0); return -1; } detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 20e38ceaa2..dc09c94a4d 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -693,14 +693,23 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, /** * qemuTPMEmulatorCleanupHost: * @tpm: TPM definition + * @flags: flags indicating whether to keep or remove TPM persistent state * * Clean up persistent storage for the swtpm. */ static void -qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm) +qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm, + virDomainUndefineFlagsValues flags) { - if (!tpm->data.emulator.persistent_state) + /* + * remove TPM state if: + * - persistent_state flag is set and the UNDEFINE_TPM flag is set + * - persistent_state flag is not set and the KEEP_TPM flag is not set + */ + if ((tpm->data.emulator.persistent_state && (flags & VIR_DOMAIN_UNDEFINE_TPM)) || + (!tpm->data.emulator.persistent_state && !(flags & VIR_DOMAIN_UNDEFINE_KEEP_TPM))) { qemuTPMEmulatorDeleteStorage(tpm); + } } @@ -991,9 +1000,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 20aadb59f3..2d22547cc6 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") + }, {.name = NULL} }; @@ -3677,6 +3685,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) bool delete_snapshots = vshCommandOptBool(cmd, "delete-storage-volume-snapshots"); bool nvram = vshCommandOptBool(cmd, "nvram"); bool keep_nvram = vshCommandOptBool(cmd, "keep-nvram"); + bool tpm = vshCommandOptBool(cmd, "tpm"); + bool keep_tpm = vshCommandOptBool(cmd, "keep-tpm"); /* Positive if these items exist. */ int has_managed_save = 0; int has_snapshots_metadata = 0; @@ -3702,6 +3712,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) VSH_REQUIRE_OPTION("delete-storage-volume-snapshots", "remove-all-storage"); VSH_EXCLUSIVE_OPTIONS("nvram", "keep-nvram"); + VSH_EXCLUSIVE_OPTIONS("tpm", "keep-tpm"); ignore_value(vshCommandOptStringQuiet(ctl, cmd, "storage", &vol_string)); @@ -3729,6 +3740,10 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_UNDEFINE_NVRAM; if (keep_nvram) flags |= VIR_DOMAIN_UNDEFINE_KEEP_NVRAM; + if (tpm) + flags |= VIR_DOMAIN_UNDEFINE_TPM; + if (keep_tpm) + flags |= VIR_DOMAIN_UNDEFINE_KEEP_TPM; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; -- 2.37.3
This patch 'fixes' the behavior of the persistent_state TPM domain XML attribute that intends to preserve the state of the TPM but should not keep the state around on all the hosts a VM has been migrated to. It removes the TPM state directory structure from the source host upon successful migration when non-shared storage is used. Similarly, it removes it from the destination host upon migration failure when non-shared storage is used. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_migration.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 126a4f6d38..3ad8f2cfd7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -38,6 +38,7 @@ #include "qemu_security.h" #include "qemu_slirp.h" #include "qemu_block.h" +#include "qemu_tpm.h" #include "domain_audit.h" #include "virlog.h" @@ -4036,7 +4037,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM); } cleanup: @@ -6702,7 +6703,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM); virErrorRestore(&orig_err); return NULL; -- 2.37.3
On 10/4/22 15:38, Stefan Berger wrote:
This patch 'fixes' the behavior of the persistent_state TPM domain XML attribute that intends to preserve the state of the TPM but should not keep the state around on all the hosts a VM has been migrated to. It removes the TPM state directory structure from the source host upon successful migration when non-shared storage is used. Similarly, it removes it from the destination host upon migration failure when non-shared storage is used.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_migration.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 126a4f6d38..3ad8f2cfd7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -38,6 +38,7 @@ #include "qemu_security.h" #include "qemu_slirp.h" #include "qemu_block.h" +#include "qemu_tpm.h"
This is not needed.
#include "domain_audit.h" #include "virlog.h" @@ -4036,7 +4037,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM); }
cleanup: @@ -6702,7 +6703,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, }
if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM);
virErrorRestore(&orig_err); return NULL;
Michal
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
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
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
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
participants (2)
-
Michal Prívozník -
Stefan Berger