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

The is series is mostly about the first patch. The others patch are minor cleanups. Maxim Nestratov (1): qemu: don't pause vm when creating internal snapshot Nikolay Shirokovskiy (2): qemu: remove excessive check in qemuSnapshotCreateActiveInternal qemu: remove unnecessary goto in qemuSnapshotCreateActiveInternal src/qemu/qemu_snapshot.c | 65 +++++++++++----------------------------- 1 file changed, 18 insertions(+), 47 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 | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index a7901779fc..a41c782c7f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -291,23 +291,18 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, { qemuDomainObjPrivate *priv = vm->privateData; 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, QEMU_ASYNC_JOB_SNAPSHOT) < 0) goto cleanup; - resume = true; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest unexpectedly quit")); @@ -316,10 +311,8 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, } if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_SNAPSHOT) < 0) { - resume = false; + QEMU_ASYNC_JOB_SNAPSHOT) < 0) goto cleanup; - } ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitor(driver, vm); @@ -329,29 +322,15 @@ 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, QEMU_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); - resume = false; } cleanup: - if (resume && virDomainObjIsActive(vm) && - qemuProcessStartCPUs(driver, vm, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_SNAPSHOT) < 0) { - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); - if (virGetLastErrorCode() == VIR_ERR_OK) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("resuming after snapshot failed")); - } - } - virObjectEventStateQueue(driver->domainEventState, event); return ret; -- 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 | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index a41c782c7f..8c050cbd75 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -298,17 +298,10 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) goto cleanup; - if (halt) { - if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, - QEMU_ASYNC_JOB_SNAPSHOT) < 0) - goto cleanup; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } - } + if (halt && + qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, + QEMU_ASYNC_JOB_SNAPSHOT) < 0) + goto cleanup; if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_SNAPSHOT) < 0) -- 2.35.1

Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org> --- src/qemu/qemu_snapshot.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 8c050cbd75..18b802e6d5 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -290,43 +290,42 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, unsigned int flags) { qemuDomainObjPrivate *priv = vm->privateData; - virObjectEvent *event = NULL; virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); bool halt = !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT); - int ret = -1; + int rc; if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) - goto cleanup; + return -1; if (halt && qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, QEMU_ASYNC_JOB_SNAPSHOT) < 0) - goto cleanup; + return -1; if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_SNAPSHOT) < 0) - goto cleanup; + return -1; - ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); + rc = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitor(driver, vm); - if (ret < 0) - goto cleanup; + if (rc < 0) + return -1; if (!(snapdef->cookie = (virObject *) qemuDomainSaveCookieNew(vm))) - goto cleanup; + return -1; if (halt) { + virObjectEvent *event; + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + virObjectEventStateQueue(driver->domainEventState, event); qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, QEMU_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); } - cleanup: - virObjectEventStateQueue(driver->domainEventState, event); - - return ret; + return 0; } -- 2.35.1

Please, disregard this series as it has a mistake. I'll send a new one. Nikolay пн, 21 мар. 2022 г. в 18:16, Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org>:
The is series is mostly about the first patch. The others patch are minor cleanups.
Maxim Nestratov (1): qemu: don't pause vm when creating internal snapshot
Nikolay Shirokovskiy (2): qemu: remove excessive check in qemuSnapshotCreateActiveInternal qemu: remove unnecessary goto in qemuSnapshotCreateActiveInternal
src/qemu/qemu_snapshot.c | 65 +++++++++++----------------------------- 1 file changed, 18 insertions(+), 47 deletions(-)
-- 2.35.1
participants (1)
-
Nikolay Shirokovskiy