[PATCH v2 REBASE 0/3] qemu: don't pause VM when creating internal snapshot

Diff to v1 [1]: - fix mistake of dropping hunk that starts CPUs if snapshot was not successful. We still need it if halt upon snapshot is requested. The is series is mostly about the first patch. The others patch are minor cleanups. [1] v1 version https://listman.redhat.com/archives/libvir-list/2022-March/229437.html Maxim Nestratov (1): qemu: don't pause vm when creating internal snapshot Nikolay Shirokovskiy (2): qemu: remove excessive check in qemuSnapshotCreateActiveInternal qemu: remove unnecessary gotos in qemuSnapshotCreateActiveInternal src/qemu/qemu_snapshot.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) -- 2.35.1

From: Maxim Nestratov <mnestratov@virtuozzo.com> The pause was introduced in [1] in ancient times when probably distro's QEMU does not have RESUME event. Yet the event was in the upstream QEMU at the time version according to [2]. So the event is supported since QEMU version 0.13 which is much older then oldest currently supported 2.11.0 version and we can remove manual pause/resume. Except for the halt case. [1] commit 346236fea97602e9e6529c5d41a32ed26b126082 Author: Jiri Denemark <jdenemar@redhat.com> Date: Thu Feb 24 16:46:44 2011 +0100 qemu: Stop guest CPUs before creating a snapshot commit 6ed2c484f261fd8bd217f855b9e5e5f981e63f0a Author: Luiz Capitulino <lcapitulino@redhat.com> Date: Tue Apr 27 20:35:59 2010 -0300 QMP: Introduce RESUME event Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org> --- src/qemu/qemu_snapshot.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b62fab7bb3..fc5b6dc7ce 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -293,16 +293,13 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, virObjectEvent *event = NULL; bool resume = false; virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); + bool halt = !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT); int ret = -1; if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) goto cleanup; - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { - /* savevm monitor command pauses the domain emitting an event which - * confuses libvirt since it's not notified when qemu resumes the - * domain. Thus we stop and start CPUs ourselves. - */ + if (halt) { if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, VIR_ASYNC_JOB_SNAPSHOT) < 0) goto cleanup; @@ -329,7 +326,7 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, if (!(snapdef->cookie = (virObject *) qemuDomainSaveCookieNew(vm))) goto cleanup; - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) { + if (halt) { event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, -- 2.35.1

After [1] it is not possible to get inactive state for domain while communicating with QEMU (see also [2]). Also even if it were possible the qemuProcessStopCPUs would return error and the check would not be reached. [1] commit 81f50cb92d16643bcd749e3ab5b404b8b7cec643 Author: Jiri Denemark <jdenemar@redhat.com> Date: Thu Feb 11 11:20:28 2016 +0100 qemu: Avoid calling qemuProcessStop without a job [2] commit f1ea5bd506b6e00aa8bf55940b147d3c0fe7f124 Author: Ján Tomko <jtomko@redhat.com> Date: Wed Nov 24 13:41:09 2021 +0100 qemu: turn qemuDomainObjExitMonitor into void Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org> --- src/qemu/qemu_snapshot.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index fc5b6dc7ce..df9da4613f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -305,11 +305,6 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, goto cleanup; resume = true; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } } if (qemuDomainObjEnterMonitorAsync(driver, vm, -- 2.35.1

Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org> --- src/qemu/qemu_snapshot.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index df9da4613f..5d622592c3 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -297,21 +297,19 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, int ret = -1; if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) - goto cleanup; + return -1; if (halt) { if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, VIR_ASYNC_JOB_SNAPSHOT) < 0) - goto cleanup; + return -1; resume = true; } if (qemuDomainObjEnterMonitorAsync(driver, vm, - VIR_ASYNC_JOB_SNAPSHOT) < 0) { - resume = false; - goto cleanup; - } + VIR_ASYNC_JOB_SNAPSHOT) < 0) + return -1; ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitor(vm); -- 2.35.1
participants (1)
-
Nikolay Shirokovskiy