7:33 a.m.
Pavel Hrdina (14):
qemu_snapshot: revert: fix incorrect jump to cleanup
qemu_snapshot: revert: drop unused variable
qemu_snapshot: revert: use g_autoptr
qemu_snapshot: revert: jump to endjob instead of calling
qemuProcessEndJob
qemu_snapshot: revert: remove cleanup label
qemu_snapshot: revert: move validation to separate function
qemu_snapshot: revert: move config prepare code to separate function
qemu_snapshot: revert: emit event right after they are created
qemu_snapshot: revert: refactor cleanup section
qemu_snapshot: revert: move saving metadata to separate function
qemu_snapshot: revert: save metadata within qemu process job
qemu_snapshot: revert: move active snapshot revert to separate
function
qemu_snapshot: revert: rename qemuSnapshotRevertInactive
qemu_snapshot: revert: move inactive snapshot to separate function
src/qemu/qemu_snapshot.c | 554 ++++++++++++++++++++++-----------------
1 file changed, 320 insertions(+), 234 deletions(-)
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 01/14] qemu_snapshot: revert: fix incorrect jump to cleanup
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index edf5511e42..64b5838db8 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2087,7 +2087,7 @@ qemuSnapshotRevert(virDomainObj *vm,
*/
if (cookie &&
qemuDomainFixupCPUs(vm, &cookie->cpu) < 0)
- goto cleanup;
+ goto endjob;
rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
cookie ? cookie->cpu : NULL,
--
2.31.1
10:49 a.m.
New subject: [libvirt PATCH 01/14] qemu_snapshot: revert: fix incorrect jump to cleanup
On a Thursday in 2021, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
Fixes: 6a6f6b91e0e76480ea961f83135efcb4faf3284a
---
src/qemu/qemu_snapshot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:33 a.m.
New subject: [libvirt PATCH 02/14] qemu_snapshot: revert: drop unused variable
Commit <f33ce12e9cd9cab7e6022e91d3765c33d99bf777> dropped unused code
but missed one variable.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 64b5838db8..413b31bae5 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1944,7 +1944,6 @@ qemuSnapshotRevert(virDomainObj *vm,
virDomainDef *inactiveConfig = NULL;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainSaveCookie *cookie;
- virCPUDef *origCPU = NULL;
unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
bool defined = false;
@@ -2234,7 +2233,6 @@ qemuSnapshotRevert(virDomainObj *vm,
}
virObjectEventStateQueue(driver->domainEventState, event);
virObjectEventStateQueue(driver->domainEventState, event2);
- virCPUDefFree(origCPU);
virDomainDefFree(config);
virDomainDefFree(inactiveConfig);
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 03/14] qemu_snapshot: revert: use g_autoptr
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 413b31bae5..5ac1c99a68 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1940,8 +1940,8 @@ qemuSnapshotRevert(virDomainObj *vm,
int detail;
qemuDomainObjPrivate *priv = vm->privateData;
int rc;
- virDomainDef *config = NULL;
- virDomainDef *inactiveConfig = NULL;
+ g_autoptr(virDomainDef) config = NULL;
+ g_autoptr(virDomainDef) inactiveConfig = NULL;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainSaveCookie *cookie;
unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
@@ -2233,8 +2233,6 @@ qemuSnapshotRevert(virDomainObj *vm,
}
virObjectEventStateQueue(driver->domainEventState, event);
virObjectEventStateQueue(driver->domainEventState, event2);
- virDomainDefFree(config);
- virDomainDefFree(inactiveConfig);
return ret;
}
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 04/14] qemu_snapshot: revert: jump to endjob instead of calling qemuProcessEndJob
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5ac1c99a68..8191733a6d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2149,8 +2149,7 @@ qemuSnapshotRevert(virDomainObj *vm,
if (qemuSnapshotRevertInactive(driver, vm, snap) < 0) {
qemuDomainRemoveInactive(driver, vm);
- qemuProcessEndJob(driver, vm);
- goto cleanup;
+ goto endjob;
}
if (inactiveConfig) {
@@ -2173,8 +2172,7 @@ qemuSnapshotRevert(virDomainObj *vm,
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
if (rc < 0) {
qemuDomainRemoveInactive(driver, vm);
- qemuProcessEndJob(driver, vm);
- goto cleanup;
+ goto endjob;
}
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 05/14] qemu_snapshot: revert: remove cleanup label
Now the cleanup label is not necessary so we can drop it.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8191733a6d..beeb012431 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1967,13 +1967,13 @@ qemuSnapshotRevert(virDomainObj *vm,
if (qemuDomainHasBlockjob(vm, false)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("domain has active block job"));
- goto cleanup;
+ return -1;
}
if (qemuProcessBeginJob(driver, vm,
VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
flags) < 0)
- goto cleanup;
+ return -1;
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto endjob;
@@ -2210,7 +2210,6 @@ qemuSnapshotRevert(virDomainObj *vm,
endjob:
qemuProcessEndJob(driver, vm);
- cleanup:
if (ret == 0) {
qemuSnapshotSetCurrent(vm, snap);
if (qemuDomainSnapshotWriteMetadata(vm, snap,
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 06/14] qemu_snapshot: revert: move validation to separate function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 78 ++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index beeb012431..cc219f8b64 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1897,6 +1897,49 @@ qemuSnapshotCreateXML(virDomainPtr domain,
}
+static int
+qemuSnapshotRevertValidate(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ virDomainSnapshotDef *snapdef,
+ unsigned int flags)
+{
+ if (!vm->persistent &&
+ snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING &&
+ snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED &&
+ (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
+ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient domain needs to request run or pause to revert
to inactive snapshot"));
+ return -1;
+ }
+
+ if (virDomainSnapshotIsExternal(snap)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("revert to external snapshot not supported yet"));
+ return -1;
+ }
+
+ if (!snap->def->dom) {
+ virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
+ _("snapshot '%s' lacks domain '%s' rollback
info"),
+ snap->def->name, vm->def->name);
+ return -1;
+ }
+
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
+ if (vm->hasManagedSave &&
+ !(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+ snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
+ virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+ _("snapshot without memory state, removal of existing
managed saved state strongly recommended to avoid corruption"));
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotRevertInactive(virQEMUDriver *driver,
@@ -1979,41 +2022,8 @@ qemuSnapshotRevert(virDomainObj *vm,
goto endjob;
snapdef = virDomainSnapshotObjGetDef(snap);
- if (!vm->persistent &&
- snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING &&
- snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED &&
- (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
- VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("transient domain needs to request run or pause "
- "to revert to inactive snapshot"));
+ if (qemuSnapshotRevertValidate(vm, snap, snapdef, flags) < 0)
goto endjob;
- }
-
- if (virDomainSnapshotIsExternal(snap)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("revert to external snapshot not supported yet"));
- goto endjob;
- }
-
- if (!snap->def->dom) {
- virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
- _("snapshot '%s' lacks domain '%s' rollback
info"),
- snap->def->name, vm->def->name);
- goto endjob;
- }
-
- if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
- if (vm->hasManagedSave &&
- !(snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
- snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
- virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
- _("snapshot without memory state, removal of "
- "existing managed saved state strongly "
- "recommended to avoid corruption"));
- goto endjob;
- }
- }
config = virDomainDefCopy(snap->def->dom,
driver->xmlopt, priv->qemuCaps, true);
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 07/14] qemu_snapshot: revert: move config prepare code to separate function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 95 +++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index cc219f8b64..589daaa73e 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1940,6 +1940,63 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
}
+static int
+qemuSnapshotRevertPrep(virDomainMomentObj *snap,
+ virDomainSnapshotDef *snapdef,
+ virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainDef **retConfig,
+ virDomainDef **retInactiveConfig)
+{
+ qemuDomainObjPrivate *priv = vm->privateData;
+ g_autoptr(virDomainDef) config = NULL;
+ g_autoptr(virDomainDef) inactiveConfig = NULL;
+
+ config = virDomainDefCopy(snap->def->dom,
+ driver->xmlopt, priv->qemuCaps, true);
+ if (!config)
+ return -1;
+
+ if (STRNEQ(config->name, vm->def->name)) {
+ VIR_FREE(config->name);
+ config->name = g_strdup(vm->def->name);
+ }
+
+ if (snap->def->inactiveDom) {
+ inactiveConfig = virDomainDefCopy(snap->def->inactiveDom,
+ driver->xmlopt, priv->qemuCaps, true);
+ if (!inactiveConfig)
+ return -1;
+
+ if (STRNEQ(inactiveConfig->name, vm->def->name)) {
+ VIR_FREE(inactiveConfig->name);
+ inactiveConfig->name = g_strdup(vm->def->name);
+ }
+ } else {
+ /* Inactive domain definition is missing:
+ * - either this is an old active snapshot and we need to copy the
+ * active definition as an inactive one
+ * - or this is an inactive snapshot which means config contains the
+ * inactive definition.
+ */
+ if (snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
+ snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) {
+ inactiveConfig = virDomainDefCopy(snap->def->dom,
+ driver->xmlopt, priv->qemuCaps,
true);
+ if (!inactiveConfig)
+ return -1;
+ } else {
+ inactiveConfig = g_steal_pointer(&config);
+ }
+ }
+
+ *retConfig = g_steal_pointer(&config);
+ *retInactiveConfig = g_steal_pointer(&inactiveConfig);
+
+ return 0;
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotRevertInactive(virQEMUDriver *driver,
@@ -1981,7 +2038,6 @@ qemuSnapshotRevert(virDomainObj *vm,
virObjectEvent *event = NULL;
virObjectEvent *event2 = NULL;
int detail;
- qemuDomainObjPrivate *priv = vm->privateData;
int rc;
g_autoptr(virDomainDef) config = NULL;
g_autoptr(virDomainDef) inactiveConfig = NULL;
@@ -2025,42 +2081,9 @@ qemuSnapshotRevert(virDomainObj *vm,
if (qemuSnapshotRevertValidate(vm, snap, snapdef, flags) < 0)
goto endjob;
- config = virDomainDefCopy(snap->def->dom,
- driver->xmlopt, priv->qemuCaps, true);
- if (!config)
+ if (qemuSnapshotRevertPrep(snap, snapdef, driver, vm,
+ &config, &inactiveConfig) < 0) {
goto endjob;
-
- if (STRNEQ(config->name, vm->def->name)) {
- VIR_FREE(config->name);
- config->name = g_strdup(vm->def->name);
- }
-
- if (snap->def->inactiveDom) {
- inactiveConfig = virDomainDefCopy(snap->def->inactiveDom,
- driver->xmlopt, priv->qemuCaps, true);
- if (!inactiveConfig)
- goto endjob;
-
- if (STRNEQ(inactiveConfig->name, vm->def->name)) {
- VIR_FREE(inactiveConfig->name);
- inactiveConfig->name = g_strdup(vm->def->name);
- }
- } else {
- /* Inactive domain definition is missing:
- * - either this is an old active snapshot and we need to copy the
- * active definition as an inactive one
- * - or this is an inactive snapshot which means config contains the
- * inactive definition.
- */
- if (snapdef->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
- snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED) {
- inactiveConfig = virDomainDefCopy(snap->def->dom,
- driver->xmlopt, priv->qemuCaps,
true);
- if (!inactiveConfig)
- goto endjob;
- } else {
- inactiveConfig = g_steal_pointer(&config);
- }
}
cookie = (qemuDomainSaveCookie *) snapdef->cookie;
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 08/14] qemu_snapshot: revert: emit event right after they are created
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 589daaa73e..950b1a65ff 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2131,6 +2131,7 @@ qemuSnapshotRevert(virDomainObj *vm,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
if (rc < 0)
goto endjob;
@@ -2145,6 +2146,7 @@ qemuSnapshotRevert(virDomainObj *vm,
event2 = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
detail);
+ virObjectEventStateQueue(driver->domainEventState, event2);
} else {
/* Transitions 2, 5, 8 */
if (!virDomainObjIsActive(vm)) {
@@ -2178,6 +2180,7 @@ qemuSnapshotRevert(virDomainObj *vm,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
}
if (qemuSnapshotRevertInactive(driver, vm, snap) < 0) {
@@ -2197,7 +2200,6 @@ qemuSnapshotRevert(virDomainObj *vm,
start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
- virObjectEventStateQueue(driver->domainEventState, event);
rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
@@ -2211,11 +2213,13 @@ qemuSnapshotRevert(virDomainObj *vm,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
if (paused) {
detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
event2 = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
detail);
+ virObjectEventStateQueue(driver->domainEventState, event2);
}
}
break;
@@ -2261,8 +2265,6 @@ qemuSnapshotRevert(virDomainObj *vm,
VIR_DOMAIN_EVENT_DEFINED,
detail));
}
- virObjectEventStateQueue(driver->domainEventState, event);
- virObjectEventStateQueue(driver->domainEventState, event2);
return ret;
}
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 09/14] qemu_snapshot: revert: refactor cleanup section
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 950b1a65ff..1d4af221e2 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2256,14 +2256,18 @@ qemuSnapshotRevert(virDomainObj *vm,
ret = -1;
}
}
- if (ret == 0 && defined && vm->persistent &&
- !(ret = virDomainDefSave(vm->newDef ? vm->newDef : vm->def,
- driver->xmlopt, cfg->configDir))) {
- detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT;
- virObjectEventStateQueue(driver->domainEventState,
- virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_DEFINED,
- detail));
+ if (ret == 0 && defined && vm->persistent) {
+ virDomainDef *saveDef = vm->newDef ? vm->newDef : vm->def;
+
+ ret = virDomainDefSave(saveDef, driver->xmlopt, cfg->configDir);
+
+ if (ret == 0) {
+ detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
}
return ret;
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 10/14] qemu_snapshot: revert: move saving metadata to separate function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 58 +++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 1d4af221e2..8ad441e41a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1997,6 +1997,40 @@ qemuSnapshotRevertPrep(virDomainMomentObj *snap,
}
+static int
+qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ virQEMUDriver *driver,
+ virQEMUDriverConfig *cfg,
+ bool defined)
+{
+ qemuSnapshotSetCurrent(vm, snap);
+ if (qemuDomainSnapshotWriteMetadata(vm, snap,
+ driver->xmlopt,
+ cfg->snapshotDir) < 0) {
+ virDomainSnapshotSetCurrent(vm->snapshots, NULL);
+ return -1;
+ }
+
+ if (defined && vm->persistent) {
+ int detail;
+ virObjectEvent *event = NULL;
+ virDomainDef *saveDef = vm->newDef ? vm->newDef : vm->def;
+
+ if (virDomainDefSave(saveDef, driver->xmlopt, cfg->configDir) < 0)
+ return -1;
+
+ detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
+
+ return 0;
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotRevertInactive(virQEMUDriver *driver,
@@ -2247,28 +2281,8 @@ qemuSnapshotRevert(virDomainObj *vm,
endjob:
qemuProcessEndJob(driver, vm);
- if (ret == 0) {
- qemuSnapshotSetCurrent(vm, snap);
- if (qemuDomainSnapshotWriteMetadata(vm, snap,
- driver->xmlopt,
- cfg->snapshotDir) < 0) {
- virDomainSnapshotSetCurrent(vm->snapshots, NULL);
- ret = -1;
- }
- }
- if (ret == 0 && defined && vm->persistent) {
- virDomainDef *saveDef = vm->newDef ? vm->newDef : vm->def;
-
- ret = virDomainDefSave(saveDef, driver->xmlopt, cfg->configDir);
-
- if (ret == 0) {
- detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_DEFINED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event);
- }
- }
+ if (ret == 0)
+ ret = qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
return ret;
}
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 11/14] qemu_snapshot: revert: save metadata within qemu process job
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8ad441e41a..5c974280af 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2276,14 +2276,11 @@ qemuSnapshotRevert(virDomainObj *vm,
goto endjob;
}
- ret = 0;
+ ret = qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
endjob:
qemuProcessEndJob(driver, vm);
- if (ret == 0)
- ret = qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
-
return ret;
}
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 12/14] qemu_snapshot: revert: move active snapshot revert to separate function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 171 ++++++++++++++++++++++-----------------
1 file changed, 98 insertions(+), 73 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5c974280af..4f0e484ab5 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2031,6 +2031,99 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+static int
+qemuSnapshotRevertActive(virDomainObj *vm,
+ virDomainSnapshotPtr snapshot,
+ virDomainMomentObj *snap,
+ virDomainSnapshotDef *snapdef,
+ virQEMUDriver *driver,
+ virQEMUDriverConfig *cfg,
+ virDomainDef **config,
+ virDomainDef **inactiveConfig,
+ unsigned int start_flags,
+ unsigned int flags)
+{
+ virObjectEvent *event = NULL;
+ virObjectEvent *event2 = NULL;
+ int detail;
+ bool defined = false;
+ qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
+ int rc;
+
+ start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
+
+ /* Transitions 2, 3, 5, 6, 8, 9 */
+ if (virDomainObjIsActive(vm)) {
+ /* Transitions 5, 6, 8, 9 */
+ qemuProcessStop(driver, vm,
+ VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
+ QEMU_ASYNC_JOB_START, 0);
+ virDomainAuditStop(vm, "from-snapshot");
+ detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
+
+ if (*inactiveConfig) {
+ virDomainObjAssignDef(vm, inactiveConfig, false, NULL);
+ defined = true;
+ }
+
+ virDomainObjAssignDef(vm, config, true, NULL);
+
+ /* No cookie means libvirt which saved the domain was too old to
+ * mess up the CPU definitions.
+ */
+ if (cookie &&
+ qemuDomainFixupCPUs(vm, &cookie->cpu) < 0)
+ return -1;
+
+ rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
+ cookie ? cookie->cpu : NULL,
+ QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ start_flags);
+ virDomainAuditStart(vm, "from-snapshot", rc >= 0);
+ detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STARTED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ if (rc < 0)
+ return -1;
+
+ /* Touch up domain state. */
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
+ (snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED ||
+ (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
+ /* Transitions 3, 6, 9 */
+ virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
+ VIR_DOMAIN_PAUSED_FROM_SNAPSHOT);
+ detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+ event2 = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event2);
+ } else {
+ /* Transitions 2, 5, 8 */
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("guest unexpectedly quit"));
+ return -1;
+ }
+ rc = qemuProcessStartCPUs(driver, vm,
+ VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
+ QEMU_ASYNC_JOB_START);
+ if (rc < 0)
+ return -1;
+ }
+
+ return qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotRevertInactive(virQEMUDriver *driver,
@@ -2076,7 +2169,6 @@ qemuSnapshotRevert(virDomainObj *vm,
g_autoptr(virDomainDef) config = NULL;
g_autoptr(virDomainDef) inactiveConfig = NULL;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
- qemuDomainSaveCookie *cookie;
unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
bool defined = false;
@@ -2120,81 +2212,14 @@ qemuSnapshotRevert(virDomainObj *vm,
goto endjob;
}
- cookie = (qemuDomainSaveCookie *) snapdef->cookie;
-
switch ((virDomainSnapshotState) snapdef->state) {
case VIR_DOMAIN_SNAPSHOT_RUNNING:
case VIR_DOMAIN_SNAPSHOT_PAUSED:
- start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
-
- /* Transitions 2, 3, 5, 6, 8, 9 */
- if (virDomainObjIsActive(vm)) {
- /* Transitions 5, 6, 8, 9 */
- qemuProcessStop(driver, vm,
- VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
- QEMU_ASYNC_JOB_START, 0);
- virDomainAuditStop(vm, "from-snapshot");
- detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event);
- }
-
- if (inactiveConfig) {
- virDomainObjAssignDef(vm, &inactiveConfig, false, NULL);
- defined = true;
- }
-
- virDomainObjAssignDef(vm, &config, true, NULL);
-
- /* No cookie means libvirt which saved the domain was too old to
- * mess up the CPU definitions.
- */
- if (cookie &&
- qemuDomainFixupCPUs(vm, &cookie->cpu) < 0)
- goto endjob;
-
- rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
- cookie ? cookie->cpu : NULL,
- QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- start_flags);
- virDomainAuditStart(vm, "from-snapshot", rc >= 0);
- detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STARTED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event);
- if (rc < 0)
- goto endjob;
-
- /* Touch up domain state. */
- if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
- (snapdef->state == VIR_DOMAIN_SNAPSHOT_PAUSED ||
- (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
- /* Transitions 3, 6, 9 */
- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
- VIR_DOMAIN_PAUSED_FROM_SNAPSHOT);
- detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
- event2 = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event2);
- } else {
- /* Transitions 2, 5, 8 */
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
- goto endjob;
- }
- rc = qemuProcessStartCPUs(driver, vm,
- VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
- QEMU_ASYNC_JOB_START);
- if (rc < 0)
- goto endjob;
- }
- break;
+ ret = qemuSnapshotRevertActive(vm, snapshot, snap, snapdef,
+ driver, cfg,
+ &config, &inactiveConfig,
+ start_flags, flags);
+ goto endjob;
case VIR_DOMAIN_SNAPSHOT_SHUTDOWN:
case VIR_DOMAIN_SNAPSHOT_SHUTOFF:
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 13/14] qemu_snapshot: revert: rename qemuSnapshotRevertInactive
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 4f0e484ab5..6b300d808a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2126,9 +2126,9 @@ qemuSnapshotRevertActive(virDomainObj *vm,
/* The domain is expected to be locked and inactive. */
static int
-qemuSnapshotRevertInactive(virQEMUDriver *driver,
- virDomainObj *vm,
- virDomainMomentObj *snap)
+qemuSnapshotInternalRevertInactive(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainMomentObj *snap)
{
size_t i;
@@ -2242,7 +2242,7 @@ qemuSnapshotRevert(virDomainObj *vm,
virObjectEventStateQueue(driver->domainEventState, event);
}
- if (qemuSnapshotRevertInactive(driver, vm, snap) < 0) {
+ if (qemuSnapshotInternalRevertInactive(driver, vm, snap) < 0) {
qemuDomainRemoveInactive(driver, vm);
goto endjob;
}
--
2.31.1
7:33 a.m.
New subject: [libvirt PATCH 14/14] qemu_snapshot: revert: move inactive snapshot to separate function
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 148 ++++++++++++++++++++++-----------------
1 file changed, 83 insertions(+), 65 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 6b300d808a..1a32b15f51 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2153,6 +2153,84 @@ qemuSnapshotInternalRevertInactive(virQEMUDriver *driver,
}
+static int
+qemuSnapshotRevertInactive(virDomainObj *vm,
+ virDomainSnapshotPtr snapshot,
+ virDomainMomentObj *snap,
+ virQEMUDriver *driver,
+ virQEMUDriverConfig *cfg,
+ virDomainDef **inactiveConfig,
+ unsigned int start_flags,
+ unsigned int flags)
+{
+ virObjectEvent *event = NULL;
+ virObjectEvent *event2 = NULL;
+ int detail;
+ bool defined = false;
+ int rc;
+
+ /* Transitions 1, 4, 7 */
+ /* Newer qemu -loadvm refuses to revert to the state of a snapshot
+ * created by qemu-img snapshot -c. If the domain is running, we
+ * must take it offline; then do the revert using qemu-img.
+ */
+
+ if (virDomainObjIsActive(vm)) {
+ /* Transitions 4, 7 */
+ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
+ QEMU_ASYNC_JOB_START, 0);
+ virDomainAuditStop(vm, "from-snapshot");
+ detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
+
+ if (qemuSnapshotInternalRevertInactive(driver, vm, snap) < 0) {
+ qemuDomainRemoveInactive(driver, vm);
+ return -1;
+ }
+
+ if (*inactiveConfig) {
+ virDomainObjAssignDef(vm, inactiveConfig, false, NULL);
+ return -1;
+ }
+
+ if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
+ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
+ /* Flush first event, now do transition 2 or 3 */
+ bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
+
+ start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
+
+ rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
+ QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ start_flags);
+ virDomainAuditStart(vm, "from-snapshot", rc >= 0);
+ if (rc < 0) {
+ qemuDomainRemoveInactive(driver, vm);
+ return -1;
+ }
+ detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STARTED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ if (paused) {
+ detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+ event2 = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ detail);
+ virObjectEventStateQueue(driver->domainEventState, event2);
+ }
+ }
+
+ return qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
+}
+
+
int
qemuSnapshotRevert(virDomainObj *vm,
virDomainSnapshotPtr snapshot,
@@ -2162,15 +2240,10 @@ qemuSnapshotRevert(virDomainObj *vm,
int ret = -1;
virDomainMomentObj *snap = NULL;
virDomainSnapshotDef *snapdef;
- virObjectEvent *event = NULL;
- virObjectEvent *event2 = NULL;
- int detail;
- int rc;
g_autoptr(virDomainDef) config = NULL;
g_autoptr(virDomainDef) inactiveConfig = NULL;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID;
- bool defined = false;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
@@ -2224,64 +2297,11 @@ qemuSnapshotRevert(virDomainObj *vm,
case VIR_DOMAIN_SNAPSHOT_SHUTDOWN:
case VIR_DOMAIN_SNAPSHOT_SHUTOFF:
case VIR_DOMAIN_SNAPSHOT_CRASHED:
- /* Transitions 1, 4, 7 */
- /* Newer qemu -loadvm refuses to revert to the state of a snapshot
- * created by qemu-img snapshot -c. If the domain is running, we
- * must take it offline; then do the revert using qemu-img.
- */
-
- if (virDomainObjIsActive(vm)) {
- /* Transitions 4, 7 */
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
- QEMU_ASYNC_JOB_START, 0);
- virDomainAuditStop(vm, "from-snapshot");
- detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event);
- }
-
- if (qemuSnapshotInternalRevertInactive(driver, vm, snap) < 0) {
- qemuDomainRemoveInactive(driver, vm);
- goto endjob;
- }
-
- if (inactiveConfig) {
- virDomainObjAssignDef(vm, &inactiveConfig, false, NULL);
- defined = true;
- }
-
- if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
- VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
- /* Flush first event, now do transition 2 or 3 */
- bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
-
- start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
-
- rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
- QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- start_flags);
- virDomainAuditStart(vm, "from-snapshot", rc >= 0);
- if (rc < 0) {
- qemuDomainRemoveInactive(driver, vm);
- goto endjob;
- }
- detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STARTED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event);
- if (paused) {
- detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
- event2 = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- detail);
- virObjectEventStateQueue(driver->domainEventState, event2);
- }
- }
- break;
+ ret = qemuSnapshotRevertInactive(vm, snapshot, snap,
+ driver, cfg,
+ &inactiveConfig,
+ start_flags, flags);
+ goto endjob;
case VIR_DOMAIN_SNAPSHOT_PMSUSPENDED:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -2301,8 +2321,6 @@ qemuSnapshotRevert(virDomainObj *vm,
goto endjob;
}
- ret = qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined);
-
endjob:
qemuProcessEndJob(driver, vm);
--
2.31.1
11:06 a.m.
On a Thursday in 2021, Pavel Hrdina wrote:
Pavel Hrdina (14):
qemu_snapshot: revert: fix incorrect jump to cleanup
qemu_snapshot: revert: drop unused variable
qemu_snapshot: revert: use g_autoptr
qemu_snapshot: revert: jump to endjob instead of calling
qemuProcessEndJob
qemu_snapshot: revert: remove cleanup label
qemu_snapshot: revert: move validation to separate function
qemu_snapshot: revert: move config prepare code to separate function
qemu_snapshot: revert: emit event right after they are created
qemu_snapshot: revert: refactor cleanup section
qemu_snapshot: revert: move saving metadata to separate function
qemu_snapshot: revert: save metadata within qemu process job
qemu_snapshot: revert: move active snapshot revert to separate
function
qemu_snapshot: revert: rename qemuSnapshotRevertInactive
qemu_snapshot: revert: move inactive snapshot to separate function
src/qemu/qemu_snapshot.c | 554 ++++++++++++++++++++++-----------------
1 file changed, 320 insertions(+), 234 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1123
days inactive
1123
days old
16 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Hrdina