[PATCH V2 0/4] Rework qemu internal active snapshots to use QMP
Den, Peter, Daniel thank you for your comments! I'm sending v2 of this patchset. Changes since last revision: - dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text - added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE - preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0 - enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM - make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper() Nikolai Barybin (4): qemu monitor: add snaphot-save/delete QMP commands qemu blockjob: add snapshot-save/delete job types qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE qemu snapshot: use QMP snapshot-save/delete for internal snapshots src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 6 +- src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_domain.c | 4 + src/qemu/qemu_monitor.c | 30 +++ src/qemu/qemu_monitor.h | 13 ++ src/qemu/qemu_monitor_json.c | 66 ++++++ src/qemu/qemu_monitor_json.h | 13 ++ src/qemu/qemu_snapshot.c | 207 ++++++++++++++++-- .../caps_6.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 2 + .../caps_6.0.0_x86_64.xml | 2 + .../caps_6.1.0_x86_64.xml | 2 + .../caps_6.2.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 2 + .../caps_6.2.0_x86_64.xml | 2 + .../caps_7.0.0_aarch64+hvf.xml | 2 + .../caps_7.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 2 + .../caps_7.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 2 + .../caps_7.1.0_x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 2 + .../caps_7.2.0_x86_64+hvf.xml | 2 + .../caps_7.2.0_x86_64.xml | 2 + .../caps_8.0.0_riscv64.xml | 2 + .../caps_8.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 2 + .../caps_8.1.0_x86_64.xml | 2 + .../caps_8.2.0_aarch64.xml | 2 + .../caps_8.2.0_armv7l.xml | 2 + .../caps_8.2.0_loongarch64.xml | 2 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 2 + .../caps_8.2.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 2 + .../caps_9.0.0_x86_64.xml | 2 + .../caps_9.1.0_x86_64.xml | 2 + 39 files changed, 391 insertions(+), 14 deletions(-) -- 2.43.5
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_monitor.c | 30 ++++++++++++++++ src/qemu/qemu_monitor.h | 13 +++++++ src/qemu/qemu_monitor_json.c | 66 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 13 +++++++ 4 files changed, 122 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b1c0c6a064..53f5ecf223 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2763,6 +2763,36 @@ qemuMonitorDeleteSnapshot(qemuMonitor *mon, const char *name) } +int +qemuMonitorSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs) +{ + VIR_DEBUG("jobname=%s, snapshotName=%s, vmstateDev=%s, wrdevs=%p", + jobname, snapshotName, vmstateDev, wrdevs); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSnapshotSave(mon, jobname, snapshotName, vmstateDev, wrdevs); +} + + +int +qemuMonitorSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + char **wrdevs) +{ + VIR_DEBUG("jobname=%s, snapshotName=%s, wrdevs=%p", jobname, snapshotName, wrdevs); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSnapshotDelete(mon, jobname, snapshotName, wrdevs); +} + + int qemuMonitorBlockdevMirror(qemuMonitor *mon, const char *jobname, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 76c859a888..27dbb78e06 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1617,3 +1617,16 @@ int qemuMonitorDisplayReload(qemuMonitor *mon, const char *type, bool tlsCerts); + +int +qemuMonitorSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs); + +int +qemuMonitorSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + char **wrdevs); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8a20ce57e6..312a4aee04 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8955,3 +8955,69 @@ int qemuMonitorJSONDisplayReload(qemuMonitor *mon, return 0; } + +int +qemuMonitorJSONSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) wrdev_list = NULL; + size_t i = 0; + + if (wrdevs) { + wrdev_list = virJSONValueNewArray(); + + for (i = 0; wrdevs[i]; i++) + if (virJSONValueArrayAppendString(wrdev_list, wrdevs[i]) < 0) + return -1; + } + + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-save", + "s:job-id", jobname, + "s:tag", snapshotName, + "s:vmstate", vmstateDev, + "A:devices", &wrdev_list, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + return qemuMonitorJSONCheckError(cmd, reply); +} + +int +qemuMonitorJSONSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + char **wrdevs) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) wrdev_list = NULL; + size_t i = 0; + + if (wrdevs) { + wrdev_list = virJSONValueNewArray(); + + for (i = 0; wrdevs[i]; i++) + if (virJSONValueArrayAppendString(wrdev_list, wrdevs[i]) < 0) + return -1; + } + + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-delete", + "s:job-id", jobname, + "s:tag", snapshotName, + "A:devices", &wrdev_list, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + return qemuMonitorJSONCheckError(cmd, reply); +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 921dd34ed2..0245cd54fc 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -825,3 +825,16 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, int qemuMonitorJSONDisplayReload(qemuMonitor *mon, const char *type, bool tlsCerts); + +int +qemuMonitorJSONSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs); + +int +qemuMonitorJSONSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + char **wrdevs); -- 2.43.5
On Wed, Jul 17, 2024 at 21:21:35 +0300, Nikolai Barybin via Devel wrote:
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_monitor.c | 30 ++++++++++++++++ src/qemu/qemu_monitor.h | 13 +++++++ src/qemu/qemu_monitor_json.c | 66 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 13 +++++++ 4 files changed, 122 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b1c0c6a064..53f5ecf223 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2763,6 +2763,36 @@ qemuMonitorDeleteSnapshot(qemuMonitor *mon, const char *name) }
+int +qemuMonitorSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs) +{ + VIR_DEBUG("jobname=%s, snapshotName=%s, vmstateDev=%s, wrdevs=%p", + jobname, snapshotName, vmstateDev, wrdevs);
Logging pointer to wrdevs is not useful as you can't get to the data from logs. In this case we can simply drop it. Also the variable values should be surrounded by quotes ('%s') to visualy separate the value.
+ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSnapshotSave(mon, jobname, snapshotName, vmstateDev, wrdevs); +}
[...]
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8a20ce57e6..312a4aee04 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8955,3 +8955,69 @@ int qemuMonitorJSONDisplayReload(qemuMonitor *mon,
return 0; } + +int +qemuMonitorJSONSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotName, + const char *vmstateDev, + char **wrdevs)
const char **
+{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) wrdev_list = NULL; + size_t i = 0;
A counter variable is not needed to iterate a NULL-terminated string list once.
+ + if (wrdevs) { + wrdev_list = virJSONValueNewArray(); + + for (i = 0; wrdevs[i]; i++) + if (virJSONValueArrayAppendString(wrdev_list, wrdevs[i]) < 0) + return -1; + } + + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-save", + "s:job-id", jobname, + "s:tag", snapshotName, + "s:vmstate", vmstateDev, + "A:devices", &wrdev_list, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + return qemuMonitorJSONCheckError(cmd, reply); +}
Looks good, but it's missing tests in test/qemumonitorjsontest.c. This test case is important because it validates the commands libvirt uses against the QMP schema obtained from qemu. Validation allows us to catch deprecations and changes early.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_block.c | 2 ++ src/qemu/qemu_blockjob.c | 6 +++++- src/qemu/qemu_blockjob.h | 2 ++ src/qemu/qemu_domain.c | 4 ++++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index d6cdf521c4..414803cff2 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3766,6 +3766,8 @@ qemuBlockPivot(virDomainObj *vm, case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_CREATE: case QEMU_BLOCKJOB_TYPE_BROKEN: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: virReportError(VIR_ERR_OPERATION_INVALID, _("job type '%1$s' does not support pivot"), qemuBlockjobTypeToString(job->type)); diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 42856df6d4..0d4fdd64ca 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -68,7 +68,9 @@ VIR_ENUM_IMPL(qemuBlockjob, "backup", "", "create", - "broken"); + "broken", + "snapshot-save", + "snapshot-delete"); static virClass *qemuBlockJobDataClass; @@ -1450,6 +1452,8 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobData *job, break; case QEMU_BLOCKJOB_TYPE_BROKEN: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index f1ac43b4c7..d4a3677050 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -66,6 +66,8 @@ typedef enum { QEMU_BLOCKJOB_TYPE_INTERNAL, QEMU_BLOCKJOB_TYPE_CREATE, QEMU_BLOCKJOB_TYPE_BROKEN, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE, QEMU_BLOCKJOB_TYPE_LAST } qemuBlockJobType; G_STATIC_ASSERT((int)QEMU_BLOCKJOB_TYPE_INTERNAL == VIR_DOMAIN_BLOCK_JOB_TYPE_LAST); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2134b11038..f512921187 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2407,6 +2407,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, break; case QEMU_BLOCKJOB_TYPE_BROKEN: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: @@ -2958,6 +2960,8 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobData *job, break; case QEMU_BLOCKJOB_TYPE_BROKEN: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_LAST: -- 2.43.5
On Wed, Jul 17, 2024 at 21:21:37 +0300, Nikolai Barybin via Devel wrote: The commit message should shortly state the job types we're about to handle so that anyone looking at the commit doesn't need to go digging.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_block.c | 2 ++ src/qemu/qemu_blockjob.c | 6 +++++- src/qemu/qemu_blockjob.h | 2 ++ src/qemu/qemu_domain.c | 4 ++++ 4 files changed, 13 insertions(+), 1 deletion(-)
[...]
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index f1ac43b4c7..d4a3677050 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -66,6 +66,8 @@ typedef enum { QEMU_BLOCKJOB_TYPE_INTERNAL, QEMU_BLOCKJOB_TYPE_CREATE, QEMU_BLOCKJOB_TYPE_BROKEN, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE,
These should go above _BROKEN as _BROKEN is a libvirt-internal state and not mapped to qemu.
The 'snapshot-save/delete' QMP commands were introduced in QEMU 6.0.0, so we add a compatible capability to check if target QEMU binary supports it. Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 2 ++ tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml | 2 ++ tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 2 ++ 30 files changed, 62 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 17db563b09..951dd367b0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -712,6 +712,8 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 460 */ "sev-snp-guest", /* QEMU_CAPS_SEV_SNP_GUEST */ "netdev.user", /* QEMU_CAPS_NETDEV_USER */ + "snapshot-save", /* QEMU_CAPS_SNAPSHOT_SAVE */ + "snapshot-delete", /* QEMU_CAPS_SNAPSHOT_DELETE */ ); @@ -1232,6 +1234,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-stats", QEMU_CAPS_QUERY_STATS }, { "query-stats-schemas", QEMU_CAPS_QUERY_STATS_SCHEMAS }, { "display-reload", QEMU_CAPS_DISPLAY_RELOAD }, + { "snapshot-save", QEMU_CAPS_SNAPSHOT_SAVE }, + { "snapshot-delete", QEMU_CAPS_SNAPSHOT_DELETE }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 910181993c..aad3fd1bf1 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -691,6 +691,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 460 */ QEMU_CAPS_SEV_SNP_GUEST, /* -object sev-snp-guest */ QEMU_CAPS_NETDEV_USER, /* -netdev user */ + QEMU_CAPS_SNAPSHOT_SAVE, /* 'snapshot-save' qmp command is supported */ + QEMU_CAPS_SNAPSHOT_DELETE, /* 'snapshot-delete' qmp command is supported */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml index 83439f3d63..8b5f6c2446 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml @@ -138,6 +138,8 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6000000</version> <microcodeVersion>61700242</microcodeVersion> <package>v6.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml index 332b8091ee..3c6dadd1ac 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml @@ -94,6 +94,8 @@ <flag name='virtio-crypto'/> <flag name='display-reload'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6000000</version> <microcodeVersion>39100242</microcodeVersion> <package>qemu-6.0.0-20210517.1.4ff77070.fc33</package> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml index dc55e0ab3f..74b9883a11 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml @@ -173,6 +173,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6000000</version> <microcodeVersion>43100242</microcodeVersion> <package>v6.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml index 1d5b8ea9d2..5d291ccd13 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml @@ -179,6 +179,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6001000</version> <microcodeVersion>43100243</microcodeVersion> <package>v6.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml index 25049e1ab8..9fa49b4f1b 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml @@ -151,6 +151,8 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6001050</version> <microcodeVersion>61700244</microcodeVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml index e321481949..f471061c1e 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml @@ -139,6 +139,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6002000</version> <microcodeVersion>42900244</microcodeVersion> <package>v6.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml index f8f8bd95f4..c145b01264 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml @@ -181,6 +181,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6002000</version> <microcodeVersion>43100244</microcodeVersion> <package>v6.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml index 69d0701dc9..2f27ace2aa 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml @@ -159,6 +159,8 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6002092</version> <microcodeVersion>61700243</microcodeVersion> <package>v7.0.0-rc2</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml index eaed5601ed..7b31750952 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml @@ -159,6 +159,8 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>6002092</version> <microcodeVersion>61700243</microcodeVersion> <package>v7.0.0-rc2</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml index a49d45a68a..dd757200bc 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml @@ -157,6 +157,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7000000</version> <microcodeVersion>42900243</microcodeVersion> <package>v7.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml index 5c57ae8eb8..cd3ad9f7f3 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml @@ -187,6 +187,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7000000</version> <microcodeVersion>43100243</microcodeVersion> <package>v7.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml index 5aeb56bac6..54fb088ac9 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml @@ -158,6 +158,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7001000</version> <microcodeVersion>42900244</microcodeVersion> <package>v7.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml index 3c589a4d61..5602f462b9 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml @@ -191,6 +191,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7001000</version> <microcodeVersion>43100244</microcodeVersion> <package>v7.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml index c5e1b23fb7..30e699d151 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml @@ -153,6 +153,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7002000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-7.2.0-6.fc37</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml index f78e596fcc..626b3c7c88 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml @@ -195,6 +195,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7002000</version> <microcodeVersion>43100245</microcodeVersion> <package>v7.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml index 9b37253d4f..8a244ea3ea 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml @@ -195,6 +195,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7002000</version> <microcodeVersion>43100245</microcodeVersion> <package>v7.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml b/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml index 03c0e99da5..793e585a2c 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml @@ -141,6 +141,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>7002050</version> <microcodeVersion>0</microcodeVersion> <package>v7.2.0-333-g222059a0fc</package> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml index bd391ca0b6..c4e035bd3b 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml @@ -199,6 +199,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8000000</version> <microcodeVersion>43100244</microcodeVersion> <package>v8.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml index c2211bfb12..9a68e6aa9d 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -122,6 +122,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8001000</version> <microcodeVersion>39100245</microcodeVersion> <package>v8.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml index 4b7809f635..3db9eadc5f 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml @@ -201,6 +201,8 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8001000</version> <microcodeVersion>43100245</microcodeVersion> <package>v8.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml index b856bc55b3..e602256277 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml @@ -168,6 +168,8 @@ <flag name='machine.virt.ras'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8002000</version> <microcodeVersion>61700246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml index 1b95b2de85..5ec368aa71 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml @@ -175,6 +175,8 @@ <flag name='machine.virt.ras'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8002000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-8.2.0-7.fc39</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml index aaf8f32485..ac10292495 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml @@ -158,6 +158,8 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8002000</version> <microcodeVersion>106300246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml index e86088332e..9df61d5fdb 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml @@ -123,6 +123,8 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8002000</version> <microcodeVersion>39100246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml index a1a5d6fd0f..0bbebde3f6 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml @@ -204,6 +204,8 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>8002000</version> <microcodeVersion>43100246</microcodeVersion> <package>v8.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml b/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml index 7fe971fc3a..bc321eb91d 100644 --- a/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml +++ b/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml @@ -79,6 +79,8 @@ <flag name='blockjob.backing-mask-protocol'/> <flag name='display-reload'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>9000000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-9.0.0-1.fc40</package> diff --git a/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml index ab841db53b..ef6c75da39 100644 --- a/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml @@ -206,6 +206,8 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>9000000</version> <microcodeVersion>43100245</microcodeVersion> <package>v9.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml index 05cba09035..4064037c01 100644 --- a/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml @@ -204,6 +204,8 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-save'/> + <flag name='snapshot-delete'/> <version>9000050</version> <microcodeVersion>43100246</microcodeVersion> <package>v9.0.0-1388-g80e8f06021-dirty</package> -- 2.43.5
On Wed, Jul 17, 2024 at 21:21:39 +0300, Nikolai Barybin via Devel wrote:
The 'snapshot-save/delete' QMP commands were introduced in QEMU 6.0.0, so we add a compatible capability to check if target QEMU binary supports it.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++
[...]
30 files changed, 62 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 17db563b09..951dd367b0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -712,6 +712,8 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 460 */ "sev-snp-guest", /* QEMU_CAPS_SEV_SNP_GUEST */ "netdev.user", /* QEMU_CAPS_NETDEV_USER */ + "snapshot-save", /* QEMU_CAPS_SNAPSHOT_SAVE */ + "snapshot-delete", /* QEMU_CAPS_SNAPSHOT_DELETE */
Reallistically only one of them is needed as there won't be a qemu which would have just one. I'll call it QEMU_CAPS_SNAPSHOT_INTERNAL_QMP.
);
@@ -1232,6 +1234,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-stats", QEMU_CAPS_QUERY_STATS }, { "query-stats-schemas", QEMU_CAPS_QUERY_STATS_SCHEMAS }, { "display-reload", QEMU_CAPS_DISPLAY_RELOAD }, + { "snapshot-save", QEMU_CAPS_SNAPSHOT_SAVE }, + { "snapshot-delete", QEMU_CAPS_SNAPSHOT_DELETE }, };
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 910181993c..aad3fd1bf1 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -691,6 +691,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 460 */ QEMU_CAPS_SEV_SNP_GUEST, /* -object sev-snp-guest */ QEMU_CAPS_NETDEV_USER, /* -netdev user */ + QEMU_CAPS_SNAPSHOT_SAVE, /* 'snapshot-save' qmp command is supported */ + QEMU_CAPS_SNAPSHOT_DELETE, /* 'snapshot-delete' qmp command is supported */
QEMU_CAPS_LAST /* this must always be the last item */
The usage of HMP commands are highly discouraged by qemu. Moreover, current snapshot creation routine does not provide flexibility in choosing target device for VM state snapshot. This patch makes use of QMP commands snapshot-save/delete and by default chooses first writable non-shared qcow2 disk (if present) as target for VM state. Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_snapshot.c | 207 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 194 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f5260c4a22..42d9385fd5 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -308,6 +308,114 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, return ret; } +static int +qemuSnapshotActiveInternalGetWrdevListHelper(virDomainObj *vm, + char ***wrdevs) +{ + size_t wrdevCount = 0; + size_t i = 0; + g_auto(GStrv) wrdevsInternal = NULL; + + *wrdevs = NULL; + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDef *disk = vm->def->disks[i]; + if (!disk->src->readonly && disk->src->shared) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found shared writable disk, VM snapshotting has no sense")); + return -1; + } + + if (!disk->src->readonly && !disk->src->shared && + disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found writable non-qcow2 disk, snapshotting forbidden")); + return -1; + } + + if (!disk->src->readonly && !disk->src->shared && + disk->src->format == VIR_STORAGE_FILE_QCOW2) { + if (wrdevCount == 0) + wrdevsInternal = g_new0(char *, vm->def->ndisks + 2); + + wrdevsInternal[wrdevCount++] = g_strdup(disk->src->nodenameformat); + } + } + + if (wrdevCount > 0 && vm->def->os.loader && vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format == VIR_STORAGE_FILE_QCOW2) + wrdevsInternal[wrdevCount] = g_strdup(vm->def->os.loader->nvram->nodenameformat); + + if (wrdevCount == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no writable device found for internal snapshot creation/deletion")); + return -1; + } + + *wrdevs = g_steal_pointer(&wrdevsInternal); + return 0; +} + + +static int +qemuSnapshotCreateActiveInternalDone(virDomainObj *vm) +{ + qemuBlockJobData *job = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + + if (!(job = virHashLookup(priv->blockjobs, g_strdup_printf("snapsave%d", vm->def->id)))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to lookup blockjob 'snapsave%1$d'"), vm->def->id); + return -1; + } + + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_NONE); + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("snapshot-save job failed: %1$s"), NULLSTR(job->errmsg)); + return -1; + } + + return job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED ? 1 : 0; +} + + +static int +qemuSnapshotCreateActiveInternalStart(virDomainObj *vm, + const char *name) +{ + qemuBlockJobData *job = NULL; + g_auto(GStrv) wrdevs = NULL; + int ret = -1; + int rc = 0; + + if (qemuSnapshotActiveInternalGetWrdevListHelper(vm, &wrdevs) < 0) + return -1; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + g_strdup_printf("snapsave%d", vm->def->id)))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create new blockjob")); + return -1; + } + + qemuBlockJobSyncBegin(job); + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { + ret = -1; + goto cleanup; + } + + rc = qemuMonitorSnapshotSave(qemuDomainGetMonitor(vm), job->name, + name, wrdevs[0], wrdevs); + qemuDomainObjExitMonitor(vm); + if (rc == 0) { + qemuBlockJobStarted(job, vm); + ret = 0; + } + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret; +} + /* The domain is expected to be locked and active. */ static int @@ -321,6 +429,7 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, bool resume = false; virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); int ret = -1; + int rv = 0; if (!qemuMigrationSrcIsAllowed(vm, false, VIR_ASYNC_JOB_SNAPSHOT, 0)) goto cleanup; @@ -342,15 +451,30 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, } } - if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { - resume = false; - goto cleanup; - } + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_SAVE)) { + if ((ret = qemuSnapshotCreateActiveInternalStart(vm, snap->def->name)) < 0) { + resume = false; + goto cleanup; + } - ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(vm); - if (ret < 0) - goto cleanup; + while ((rv = qemuSnapshotCreateActiveInternalDone(vm)) != 1) { + if (rv < 0 || qemuDomainObjWait(vm) < 0) { + ret = -1; + goto cleanup; + } + } + /* Legacy support for QEMU versions < 6.0. */ + } else { + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { + resume = false; + goto cleanup; + } + + ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); + qemuDomainObjExitMonitor(vm); + if (ret < 0) + goto cleanup; + } if (!(snapdef->cookie = (virObject *) qemuDomainSaveCookieNew(vm))) goto cleanup; @@ -3603,6 +3727,54 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm, } +static int +qemuSnapshotDiscardActiveInternal(virDomainObj *vm, + const char *name) +{ + qemuBlockJobData *job = NULL; + g_auto(GStrv) wrdevs = NULL; + int ret = -1; + int rc = 0; + + if (qemuSnapshotActiveInternalGetWrdevListHelper(vm, &wrdevs) < 0) + return -1; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE, + g_strdup_printf("snapdelete%d", vm->def->id)))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create new blockjob")); + return -1; + } + + qemuBlockJobSyncBegin(job); + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + goto cleanup; + + rc = qemuMonitorSnapshotDelete(qemuDomainGetMonitor(vm), job->name, name, wrdevs); + qemuDomainObjExitMonitor(vm); + if (rc == 0) { + qemuBlockJobStarted(job, vm); + ret = 0; + } else { + goto cleanup; + } + + while (job->state != VIR_DOMAIN_BLOCK_JOB_COMPLETED) { + qemuDomainObjWait(vm); + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_NONE); + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("snapshot-delete job failed: %1$s"), NULLSTR(job->errmsg)); + ret = -1; + break; + } + } + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret; +} + + /* Discard one snapshot (or its metadata), without reparenting any children. */ static int qemuSnapshotDiscardImpl(virQEMUDriver *driver, @@ -3642,14 +3814,23 @@ qemuSnapshotDiscardImpl(virQEMUDriver *driver, if (qemuSnapshotDiscardExternal(vm, snap, externalData) < 0) return -1; } else { + qemuDomainObjPrivate *priv = vm->privateData; + /* Similarly as internal snapshot creation we would use a regular job * here so set a mask to forbid any other job. */ qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_NONE); - if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) - return -1; - /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(qemuDomainGetMonitor(vm), snap->def->name); - qemuDomainObjExitMonitor(vm); + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_DELETE)) { + qemuSnapshotDiscardActiveInternal(vm, snap->def->name); + /* Legacy support for QEMU versions < 6.0. */ + } else { + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + return -1; + /* we continue on even in the face of error */ + qemuMonitorDeleteSnapshot(qemuDomainGetMonitor(vm), snap->def->name); + qemuDomainObjExitMonitor(vm); + } + qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_DEFAULT_MASK); } } -- 2.43.5
On Wed, Jul 17, 2024 at 21:21:41 +0300, Nikolai Barybin via Devel wrote:
The usage of HMP commands are highly discouraged by qemu. Moreover, current snapshot creation routine does not provide flexibility in choosing target device for VM state snapshot.
This patch makes use of QMP commands snapshot-save/delete and by default chooses first writable non-shared qcow2 disk (if present) as target for VM state.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/qemu/qemu_snapshot.c | 207 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 194 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f5260c4a22..42d9385fd5 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -308,6 +308,114 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, return ret; }
+static int +qemuSnapshotActiveInternalGetWrdevListHelper(virDomainObj *vm, + char ***wrdevs) +{ + size_t wrdevCount = 0; + size_t i = 0; + g_auto(GStrv) wrdevsInternal = NULL; + + *wrdevs = NULL; + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDef *disk = vm->def->disks[i]; + if (!disk->src->readonly && disk->src->shared) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found shared writable disk, VM snapshotting has no sense")); + return -1; + } + + if (!disk->src->readonly && !disk->src->shared && + disk->src->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found writable non-qcow2 disk, snapshotting forbidden")); + return -1; + } + + if (!disk->src->readonly && !disk->src->shared && + disk->src->format == VIR_STORAGE_FILE_QCOW2) { + if (wrdevCount == 0) + wrdevsInternal = g_new0(char *, vm->def->ndisks + 2); + + wrdevsInternal[wrdevCount++] = g_strdup(disk->src->nodenameformat); + } + }
1) many of these are already checked in the functions which make sure that the snapshot is possible. 2) Use of VIR_ERR_INTERNAL_ERROR is not appropriate in any of the cases above. 3) the code is structured weirdly repeating the same checks mutliple times which makes it hard to understand and impossible to extend. This function will need to be rewritten. It also must not use the VM definition but rather the snapshot definition instead which already has the checked configuration. Also as I'll explain below it's unlikely that this logic will be usable for snapshot deletion due to the possibiliuty that the VM definition was changed and thus will require much more thoroguh checking.
+ if (wrdevCount > 0 && vm->def->os.loader && vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format == VIR_STORAGE_FILE_QCOW2) + wrdevsInternal[wrdevCount] = g_strdup(vm->def->os.loader->nvram->nodenameformat); + + if (wrdevCount == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
Definitely not an internal error. User requested something that can't be done.
+ _("no writable device found for internal snapshot creation/deletion")); + return -1; + } + + *wrdevs = g_steal_pointer(&wrdevsInternal); + return 0; +} + + +static int +qemuSnapshotCreateActiveInternalDone(virDomainObj *vm) +{ + qemuBlockJobData *job = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + + if (!(job = virHashLookup(priv->blockjobs, g_strdup_printf("snapsave%d", vm->def->id)))) {
Since the code which eventually calls this has to create the job in the first place, you can simply pass in the job struct instead of looking it up all the time. In the deletion code you did exactly that thing even.
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to lookup blockjob 'snapsave%1$d'"), vm->def->id); + return -1; + } + + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_NONE); + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_INTERNAL_ERROR,
This'd be more likely an OPERATION_FAILED.
+ _("snapshot-save job failed: %1$s"), NULLSTR(job->errmsg)); + return -1; + } + + return job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED ? 1 : 0; +} + + +static int +qemuSnapshotCreateActiveInternalStart(virDomainObj *vm, + const char *name) +{ + qemuBlockJobData *job = NULL; + g_auto(GStrv) wrdevs = NULL; + int ret = -1; + int rc = 0; + + if (qemuSnapshotActiveInternalGetWrdevListHelper(vm, &wrdevs) < 0) + return -1; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + g_strdup_printf("snapsave%d", vm->def->id)))) {
The buffer allocated by g_strdup_printf() is leaked as qemuBlockJobDiskNew() just copies the passed values. This already reports errors ..
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create new blockjob"));
.. so you overwrite it with a much worse error message.
+ return -1; + } + + qemuBlockJobSyncBegin(job); + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { + ret = -1; + goto cleanup; + } + + rc = qemuMonitorSnapshotSave(qemuDomainGetMonitor(vm), job->name, + name, wrdevs[0], wrdevs); + qemuDomainObjExitMonitor(vm); + if (rc == 0) { + qemuBlockJobStarted(job, vm); + ret = 0; + } + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret;
This should return the job so that the caller can use it.
+} +
/* The domain is expected to be locked and active. */ static int @@ -321,6 +429,7 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, bool resume = false; virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); int ret = -1; + int rv = 0;
if (!qemuMigrationSrcIsAllowed(vm, false, VIR_ASYNC_JOB_SNAPSHOT, 0)) goto cleanup; @@ -342,15 +451,30 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, } }
- if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { - resume = false; - goto cleanup; - } + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_SAVE)) { + if ((ret = qemuSnapshotCreateActiveInternalStart(vm, snap->def->name)) < 0) { + resume = false;
+ goto cleanup; + }
- ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(vm); - if (ret < 0) - goto cleanup; + while ((rv = qemuSnapshotCreateActiveInternalDone(vm)) != 1) { + if (rv < 0 || qemuDomainObjWait(vm) < 0) { + ret = -1; + goto cleanup; + } + } + /* Legacy support for QEMU versions < 6.0. */
This comment belongs to the other block.
+ } else { + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { + resume = false; + goto cleanup; + } + + ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); + qemuDomainObjExitMonitor(vm); + if (ret < 0) + goto cleanup; + }
if (!(snapdef->cookie = (virObject *) qemuDomainSaveCookieNew(vm))) goto cleanup; @@ -3603,6 +3727,54 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm, }
+static int +qemuSnapshotDiscardActiveInternal(virDomainObj *vm, + const char *name) +{ + qemuBlockJobData *job = NULL; + g_auto(GStrv) wrdevs = NULL; + int ret = -1; + int rc = 0; + + if (qemuSnapshotActiveInternalGetWrdevListHelper(vm, &wrdevs) < 0)
When discarding snapshots using the active definition may be wrong as the disk sources may have changed. This will need a much more thorough check and use the data from the snapshot definition as well. This is what I need to figure out before having a working version and it doesn't make much sense for me to explain everything here and then for you to update the code.
+ return -1; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE, + g_strdup_printf("snapdelete%d", vm->def->id)))) {
Same memleak as noted above
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create new blockjob"));
Same error overwrite.
+ return -1; + } + + qemuBlockJobSyncBegin(job); + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + goto cleanup; + + rc = qemuMonitorSnapshotDelete(qemuDomainGetMonitor(vm), job->name, name, wrdevs); + qemuDomainObjExitMonitor(vm); + if (rc == 0) { + qemuBlockJobStarted(job, vm); + ret = 0; + } else { + goto cleanup; + } + + while (job->state != VIR_DOMAIN_BLOCK_JOB_COMPLETED) { + qemuDomainObjWait(vm); + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_NONE); + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_INTERNAL_ERROR,
Once again wrong error code.
+ _("snapshot-delete job failed: %1$s"), NULLSTR(job->errmsg)); + ret = -1; + break; + } + } + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret; +} + + /* Discard one snapshot (or its metadata), without reparenting any children. */ static int qemuSnapshotDiscardImpl(virQEMUDriver *driver, @@ -3642,14 +3814,23 @@ qemuSnapshotDiscardImpl(virQEMUDriver *driver, if (qemuSnapshotDiscardExternal(vm, snap, externalData) < 0) return -1; } else { + qemuDomainObjPrivate *priv = vm->privateData; + /* Similarly as internal snapshot creation we would use a regular job * here so set a mask to forbid any other job. */ qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_NONE); - if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) - return -1; - /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(qemuDomainGetMonitor(vm), snap->def->name); - qemuDomainObjExitMonitor(vm); + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_DELETE)) { + qemuSnapshotDiscardActiveInternal(vm, snap->def->name); + /* Legacy support for QEMU versions < 6.0. */ + } else { + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + return -1; + /* we continue on even in the face of error */ + qemuMonitorDeleteSnapshot(qemuDomainGetMonitor(vm), snap->def->name); + qemuDomainObjExitMonitor(vm); + } + qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_DEFAULT_MASK); } } -- 2.43.5
Hello everyone! This is a ping email On 7/17/24 20:21, Nikolai Barybin wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper()
Nikolai Barybin (4): qemu monitor: add snaphot-save/delete QMP commands qemu blockjob: add snapshot-save/delete job types qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE qemu snapshot: use QMP snapshot-save/delete for internal snapshots
src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 6 +- src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_domain.c | 4 + src/qemu/qemu_monitor.c | 30 +++ src/qemu/qemu_monitor.h | 13 ++ src/qemu/qemu_monitor_json.c | 66 ++++++ src/qemu/qemu_monitor_json.h | 13 ++ src/qemu/qemu_snapshot.c | 207 ++++++++++++++++-- .../caps_6.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 2 + .../caps_6.0.0_x86_64.xml | 2 + .../caps_6.1.0_x86_64.xml | 2 + .../caps_6.2.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 2 + .../caps_6.2.0_x86_64.xml | 2 + .../caps_7.0.0_aarch64+hvf.xml | 2 + .../caps_7.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 2 + .../caps_7.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 2 + .../caps_7.1.0_x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 2 + .../caps_7.2.0_x86_64+hvf.xml | 2 + .../caps_7.2.0_x86_64.xml | 2 + .../caps_8.0.0_riscv64.xml | 2 + .../caps_8.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 2 + .../caps_8.1.0_x86_64.xml | 2 + .../caps_8.2.0_aarch64.xml | 2 + .../caps_8.2.0_armv7l.xml | 2 + .../caps_8.2.0_loongarch64.xml | 2 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 2 + .../caps_8.2.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 2 + .../caps_9.0.0_x86_64.xml | 2 + .../caps_9.1.0_x86_64.xml | 2 + 39 files changed, 391 insertions(+), 14 deletions(-)
On 7/17/24 20:21, Nikolai Barybin wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper()
Nikolai Barybin (4): qemu monitor: add snaphot-save/delete QMP commands qemu blockjob: add snapshot-save/delete job types qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE qemu snapshot: use QMP snapshot-save/delete for internal snapshots
src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 6 +- src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_domain.c | 4 + src/qemu/qemu_monitor.c | 30 +++ src/qemu/qemu_monitor.h | 13 ++ src/qemu/qemu_monitor_json.c | 66 ++++++ src/qemu/qemu_monitor_json.h | 13 ++ src/qemu/qemu_snapshot.c | 207 ++++++++++++++++-- .../caps_6.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 2 + .../caps_6.0.0_x86_64.xml | 2 + .../caps_6.1.0_x86_64.xml | 2 + .../caps_6.2.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 2 + .../caps_6.2.0_x86_64.xml | 2 + .../caps_7.0.0_aarch64+hvf.xml | 2 + .../caps_7.0.0_aarch64.xml | 2 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 2 + .../caps_7.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 2 + .../caps_7.1.0_x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 2 + .../caps_7.2.0_x86_64+hvf.xml | 2 + .../caps_7.2.0_x86_64.xml | 2 + .../caps_8.0.0_riscv64.xml | 2 + .../caps_8.0.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 2 + .../caps_8.1.0_x86_64.xml | 2 + .../caps_8.2.0_aarch64.xml | 2 + .../caps_8.2.0_armv7l.xml | 2 + .../caps_8.2.0_loongarch64.xml | 2 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 2 + .../caps_8.2.0_x86_64.xml | 2 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 2 + .../caps_9.0.0_x86_64.xml | 2 + .../caps_9.1.0_x86_64.xml | 2 + 39 files changed, 391 insertions(+), 14 deletions(-)
Hello everyone! This is a ping email.
On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper()
Note that I'll clean up the patches, so there's no need for you to send another version. My review will follow regardless as documentation of what I'll be changing.
On 8/14/24 14:03, Peter Krempa wrote:
On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper() Note that I'll clean up the patches, so there's no need for you to send another version. My review will follow regardless as documentation of what I'll be changing.
Peter, thank you for your review! Do you have ETA for these changes to appear in master branch? We need them in our product.
On Thu, Aug 29, 2024 at 11:09:23 +0200, Nikolai Barybin wrote:
On 8/14/24 14:03, Peter Krempa wrote:
On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper() Note that I'll clean up the patches, so there's no need for you to send another version. My review will follow regardless as documentation of what I'll be changing.
Peter, thank you for your review! Do you have ETA for these changes to appear in master branch? We need them in our product.
I'm halfway through fixing the last patch as I had other things to do as well. I'll be sending them out hopefully next week.
On 8/29/24 11:12, Peter Krempa wrote:
On Thu, Aug 29, 2024 at 11:09:23 +0200, Nikolai Barybin wrote:
On 8/14/24 14:03, Peter Krempa wrote:
On Wed, Jul 17, 2024 at 21:21:33 +0300, Nikolai Barybin via Devel wrote:
Den, Peter, Daniel thank you for your comments!
I'm sending v2 of this patchset.
Changes since last revision:
- dropped [PATCH 4/4] qemu monitor: reap qemu_monitor_text
- added new patch: qemu capabilities: add QEMU_CAPS_SNAPSHOT_SAVE/_DELETE
- preserved old-style snapshotting (HMP savevm) in case we have QEMU < 6.0
- enhanced requirements for allowing snapshotting. All writable disks should be qcow2, non-shared. If such disks exist and we have qcow2 NVRAM, add NVRAM device to the list of wrdevs. But never save vmstate to NVRAM
- make char** wrdevs list allocation inside qemuSnapshotActiveInternalGetWrdevListHelper() Note that I'll clean up the patches, so there's no need for you to send another version. My review will follow regardless as documentation of what I'll be changing.
Peter, thank you for your review! Do you have ETA for these changes to appear in master branch? We need them in our product. I'm halfway through fixing the last patch as I had other things to do as well.
I'll be sending them out hopefully next week.
Hello Peter! Do you have any updates regarding merging this series? P. S. Sorry, I forgot to forward previous message to devel@list and sent in personally to you
participants (2)
-
Nikolai Barybin -
Peter Krempa