8:38 a.m.
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
8:38 a.m.
New subject: [PATCH v2 1/2] qemu: Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags
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>
---
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
8:38 a.m.
New subject: [PATCH v2 2/2] qemu: tpm: Remove TPM state after successful migration
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(a)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
9:39 a.m.
New subject: [PATCH v2 2/2] qemu: tpm: Remove TPM state after successful migration
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(a)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
9:39 a.m.
New subject: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
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(a)redhat.com>
and pushed.
Michal
10:33 a.m.
New subject: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
On 10/4/22 10:39, Michal Prívozník wrote:
>
Reviewed-by: Michal Privoznik <mprivozn(a)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
10:48 a.m.
New subject: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
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(a)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
11:19 a.m.
New subject: [PATCH v2 0/2] qemu: tpm: Improve TPM state files management
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(a)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
745
days inactive
745
days old
7 comments
2 participants
participants (2)
-
Michal Prívozník -
Stefan Berger