[PATCH v3 00/10] qemu: Rework internal active snapshots to use QMP commands

Changes to v2: - added few cleanups - added qemumonitorjson test cases for new commands - dropped duplicate capability flags - fixed bugs pointed out in v2 - rewritten logic for selecting snapshot images (both for creation/deletion) - fixed the logic to be fault tolerant same way as 'delvm' - allowed internal snapshots of VMs with UEFI - added explanation to all code with non-obvious implications Nikolai Barybin via Devel (4): qemu: monitor: Add plumbing for 'snaphot-save'/'snapshot-delete' QMP commands qemu: blockjob: Add job types for 'snapshot-save/delete' qemu: capabilities: Introduce QEMU_CAPS_SNAPSHOT_INTERNAL_QMP capability qemu snapshot: use QMP snapshot-save for internal snapshots creation Peter Krempa (6): qemuDomainObjWait: Annotate with G_GNUC_WARN_UNUSED_RESULT qemu: monitor: Store internal snapshot names from 'query-named-block-nodes' qemu snapshot: use QMP snapshot-delete for internal snapshots deletion qemuSnapshotActiveInternalDeleteGetDevices: Add warning when deleting inconsistent snapshot qemu: snapshot: Allow internal snapshots with PFLASH nvram news: mention internal snapshot changes NEWS.rst | 16 + src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 7 + src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_domain.c | 10 + src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_monitor.c | 30 ++ src/qemu/qemu_monitor.h | 17 + src/qemu/qemu_monitor_json.c | 79 ++++ src/qemu/qemu_monitor_json.h | 13 + src/qemu/qemu_snapshot.c | 358 +++++++++++++++++- tests/qemublocktest.c | 11 + .../bitmap/snapshots-internal.json | 298 +++++++++++++++ .../bitmap/snapshots-internal.out | 2 + .../caps_6.0.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 1 + .../caps_6.0.0_x86_64.xml | 1 + .../caps_6.1.0_x86_64.xml | 1 + .../caps_6.2.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 + .../caps_6.2.0_x86_64.xml | 1 + .../caps_7.0.0_aarch64+hvf.xml | 1 + .../caps_7.0.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + .../caps_7.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_riscv64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_aarch64.xml | 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml | 1 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 + .../caps_9.0.0_x86_64.xml | 1 + .../caps_9.1.0_riscv64.xml | 1 + .../caps_9.1.0_x86_64.xml | 1 + tests/qemumonitorjsontest.c | 33 ++ 46 files changed, 895 insertions(+), 22 deletions(-) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.out -- 2.46.0

Callers must handle the return value of this function as the VM might have died. Add compiler annotation to force it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ba357af8f4..c29ecdf238 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1132,7 +1132,8 @@ qemuDomainRemoveLogs(virQEMUDriver *driver, const char *name); int -qemuDomainObjWait(virDomainObj *vm); +qemuDomainObjWait(virDomainObj *vm) + G_GNUC_WARN_UNUSED_RESULT; bool qemuDomainObjIsActive(virDomainObj *vm); -- 2.46.0

From: Nikolai Barybin via Devel <devel@lists.libvirt.org> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 30 ++++++++++++++++++ src/qemu/qemu_monitor.h | 14 +++++++++ src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 13 ++++++++ tests/qemumonitorjsontest.c | 33 ++++++++++++++++++++ 5 files changed, 150 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ada3de474f..fd888e2468 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2764,6 +2764,36 @@ qemuMonitorDeleteSnapshot(qemuMonitor *mon, const char *name) } +int +qemuMonitorSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char *vmstate_disk, + const char **disks) +{ + VIR_DEBUG("jobname='%s', snapshotname='%s', vmstate_disk='%s'", + jobname, snapshotname, vmstate_disk); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSnapshotSave(mon, jobname, snapshotname, vmstate_disk, disks); +} + + +int +qemuMonitorSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char **disks) +{ + VIR_DEBUG("jobname='%s', snapshotname='%s'", jobname, snapshotname); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSnapshotDelete(mon, jobname, snapshotname, disks); +} + + int qemuMonitorBlockdevMirror(qemuMonitor *mon, const char *jobname, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2bb64dd53f..6251f48d28 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1618,3 +1618,17 @@ int qemuMonitorDisplayReload(qemuMonitor *mon, const char *type, bool tlsCerts); + + +int +qemuMonitorSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char *vmstate_disk, + const char **disks); + +int +qemuMonitorSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char **disks); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 75a5b03024..27f74181f6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8701,3 +8701,63 @@ int qemuMonitorJSONDisplayReload(qemuMonitor *mon, return 0; } + + +int +qemuMonitorJSONSnapshotSave(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char *vmstate_disk, + const char **disks) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) devices = virJSONValueNewArray(); + + for (; *disks; disks++) { + if (virJSONValueArrayAppendString(devices, *disks) < 0) + return -1; + } + + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-save", + "s:job-id", jobname, + "s:tag", snapshotname, + "s:vmstate", vmstate_disk, + "a:devices", &devices, + 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, + const char **disks) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + g_autoptr(virJSONValue) devices = virJSONValueNewArray(); + + for (; *disks; disks++) { + if (virJSONValueArrayAppendString(devices, *disks) < 0) + return -1; + } + + if (!(cmd = qemuMonitorJSONMakeCommand("snapshot-delete", + "s:job-id", jobname, + "s:tag", snapshotname, + "a:devices", &devices, + 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 fef81fd911..10491b809b 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 *vmstate_disk, + const char **disks); + +int +qemuMonitorJSONSnapshotDelete(qemuMonitor *mon, + const char *jobname, + const char *snapshotname, + const char **disks); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ab2c445289..fca4890746 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1217,6 +1217,38 @@ testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) return 0; } + +static int +testQemuMonitorJSONqemuMonitorJSONSnapshot(const void *opaque) +{ + const testGenericData *data = opaque; + virDomainXMLOption *xmlopt = data->xmlopt; + g_autoptr(qemuMonitorTest) test = NULL; + const char *disks[] = { "test", "disk", NULL }; + + if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) + return -1; + + if (qemuMonitorTestAddItem(test, "snapshot-save", + "{\"return\":{}}") < 0) + return -1; + + if (qemuMonitorTestAddItem(test, "snapshot-delete", + "{\"return\":{}}") < 0) + return -1; + + if (qemuMonitorJSONSnapshotSave(qemuMonitorTestGetMonitor(test), + "jobname", "snapshotname", "vmstate", disks) < 0) + return -1; + + if (qemuMonitorJSONSnapshotDelete(qemuMonitorTestGetMonitor(test), + "jobname", "snapshotname", disks) < 0) + return -1; + + return 0; +} + + static bool testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a, struct qemuMonitorQueryCpusEntry *b) @@ -2956,6 +2988,7 @@ mymain(void) DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability); DO_TEST(qemuMonitorJSONSendKeyHoldtime); DO_TEST(qemuMonitorJSONNBDServerStart); + DO_TEST(qemuMonitorJSONSnapshot); DO_TEST_CPU_DATA("host"); DO_TEST_CPU_DATA("full"); -- 2.46.0

From: Nikolai Barybin via Devel <devel@lists.libvirt.org> The snapshot creation/deletion QMP commands use the qemu 'job' API to signal completion thus we need to add corresponding job types. As the job handles everything internally we don't store anything about the job. Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 2 ++ src/qemu/qemu_blockjob.c | 7 +++++++ src/qemu/qemu_blockjob.h | 2 ++ src/qemu/qemu_domain.c | 10 ++++++++++ 4 files changed, 21 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 9bdec92697..e739c097ca 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3775,6 +3775,8 @@ qemuBlockPivot(virDomainObj *vm, case QEMU_BLOCKJOB_TYPE_BACKUP: case QEMU_BLOCKJOB_TYPE_INTERNAL: case QEMU_BLOCKJOB_TYPE_CREATE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: case QEMU_BLOCKJOB_TYPE_BROKEN: virReportError(VIR_ERR_OPERATION_INVALID, _("job type '%1$s' does not support pivot"), diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 652b25540a..6e53603fba 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -68,6 +68,8 @@ VIR_ENUM_IMPL(qemuBlockjob, "backup", "", "create", + "snapshot-save", + "snapshot-delete", "broken"); static virClass *qemuBlockJobDataClass; @@ -1455,6 +1457,11 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobData *job, progressTotal); break; + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: + /* The internal snapshot jobs don't need any extra handling */ + break; + case QEMU_BLOCKJOB_TYPE_BROKEN: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index f1ac43b4c7..6620e08c47 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -65,6 +65,8 @@ typedef enum { /* Additional enum values local to qemu */ QEMU_BLOCKJOB_TYPE_INTERNAL, QEMU_BLOCKJOB_TYPE_CREATE, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE, QEMU_BLOCKJOB_TYPE_BROKEN, QEMU_BLOCKJOB_TYPE_LAST } qemuBlockJobType; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b6762cc372..fc682c0c9d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2478,6 +2478,11 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, } break; + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: + /* No private data for internal snapshot jobs */ + break; + case QEMU_BLOCKJOB_TYPE_BROKEN: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: @@ -3030,6 +3035,11 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobData *job, goto broken; break; + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE: + case QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE: + /* No extra data for internal snapshot jobs. */ + break; + case QEMU_BLOCKJOB_TYPE_BROKEN: case QEMU_BLOCKJOB_TYPE_NONE: case QEMU_BLOCKJOB_TYPE_INTERNAL: -- 2.46.0

From: Nikolai Barybin via Devel <devel@lists.libvirt.org> 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> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 + tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 + 31 files changed, 36 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6a0f3e2ab7..29a847346c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -715,6 +715,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "acpi-erst", /* QEMU_CAPS_DEVICE_ACPI_ERST */ "intel-iommu.dma-translation", /* QEMU_CAPS_INTEL_IOMMU_DMA_TRANSLATION */ "machine-i8042-opt", /* QEMU_CAPS_MACHINE_I8042_OPT */ + + /* 465 */ + "snapshot-internal-qmp", /* QEMU_CAPS_SNAPSHOT_INTERNAL_QMP */ ); @@ -1235,6 +1238,7 @@ 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_INTERNAL_QMP }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 736d34179e..091de8999d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -695,6 +695,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_INTEL_IOMMU_DMA_TRANSLATION, /* intel-iommu.dma-translation */ QEMU_CAPS_MACHINE_I8042_OPT, /* -machine xxx,i8042=on/off; use virQEMUCapsSupportsI8042Toggle() to query this capability */ + /* 465 */ + QEMU_CAPS_SNAPSHOT_INTERNAL_QMP, /* internal snapshot support via QMP commands 'snapshot-save'/'snapshot-delete' */ + 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..a02018a467 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml @@ -138,6 +138,7 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..9d84d3bdc9 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_s390x.xml @@ -94,6 +94,7 @@ <flag name='virtio-crypto'/> <flag name='display-reload'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..a8897fb80b 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml @@ -173,6 +173,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..0f2995a2d3 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml @@ -179,6 +179,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..5624ad95a1 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml @@ -151,6 +151,7 @@ <flag name='usb-mtp'/> <flag name='machine.virt.ras'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..d4172c3146 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml @@ -139,6 +139,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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..582b0e9b52 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml @@ -181,6 +181,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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 13b67cc7bf..8bdb26e8a4 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml @@ -160,6 +160,7 @@ <flag name='machine.virt.ras'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 a9fe9a2014..697c1c255a 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml @@ -160,6 +160,7 @@ <flag name='machine.virt.ras'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 92636bc7d4..e02f42c5e0 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml @@ -158,6 +158,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 23b626aa16..fd317a9afa 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml @@ -189,6 +189,7 @@ <flag name='netdev.user'/> <flag name='acpi-erst'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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 a851d89974..427aa1ace9 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml @@ -159,6 +159,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 81c4cc474b..32e4e8e1bb 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml @@ -194,6 +194,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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 998c548556..56cb66d394 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml @@ -154,6 +154,7 @@ <flag name='usb-mtp'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 18d74d6e8c..0ebcb94a31 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml @@ -198,6 +198,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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 fb52a30f04..025ced01d9 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml @@ -198,6 +198,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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..7959d49c02 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml @@ -141,6 +141,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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 459d05bf43..5f45788b77 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml @@ -202,6 +202,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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..16a3e3fbe0 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -122,6 +122,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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 289c87ff19..7dee7f94c2 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml @@ -204,6 +204,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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 0043443880..78082dbf3f 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml @@ -169,6 +169,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 16377a74e3..912eb45db6 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml @@ -176,6 +176,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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 eb80be7a06..60b980e256 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml @@ -159,6 +159,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <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..8f36104080 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml @@ -123,6 +123,7 @@ <flag name='usb-mtp'/> <flag name='virtio-sound'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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 e56d5dafa4..51e14736cd 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml @@ -207,6 +207,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <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..1b56aebd48 100644 --- a/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml +++ b/tests/qemucapabilitiesdata/caps_9.0.0_sparc.xml @@ -79,6 +79,7 @@ <flag name='blockjob.backing-mask-protocol'/> <flag name='display-reload'/> <flag name='netdev.user'/> + <flag name='snapshot-internal-qmp'/> <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 3f1f518d22..4b64547b11 100644 --- a/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml @@ -209,6 +209,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <version>9000000</version> <microcodeVersion>43100245</microcodeVersion> <package>v9.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml b/tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml index 20cf97cdf9..1e7b1e622b 100644 --- a/tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml @@ -167,6 +167,7 @@ <flag name='virtio-sound'/> <flag name='netdev.user'/> <flag name='acpi-erst'/> + <flag name='snapshot-internal-qmp'/> <version>9001000</version> <microcodeVersion>0</microcodeVersion> <package>v9.1.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 a6b3768dfd..06600f48fb 100644 --- a/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml @@ -207,6 +207,7 @@ <flag name='acpi-erst'/> <flag name='intel-iommu.dma-translation'/> <flag name='machine-i8042-opt'/> + <flag name='snapshot-internal-qmp'/> <version>9001000</version> <microcodeVersion>43100246</microcodeVersion> <package>v9.1.0</package> -- 2.46.0

Store the names of internal snapshots present in supported images in the data we dump from 'query-named-block-nodes' so that the upcoming changes to the internal snapshot code can access it. To test this we use the bitmap detection test cases which can be easily extended to dump this data. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 19 ++ tests/qemublocktest.c | 11 + .../bitmap/snapshots-internal.json | 298 ++++++++++++++++++ .../bitmap/snapshots-internal.out | 2 + 5 files changed, 333 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.out diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6251f48d28..4341519cfe 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -709,6 +709,9 @@ struct _qemuBlockNamedNodeData { qemuBlockNamedNodeDataBitmap **bitmaps; size_t nbitmaps; + /* NULL terminated string list of internal snapshot names */ + char **snapshots; + /* the cluster size of the image is valid only when > 0 */ unsigned long long clusterSize; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 27f74181f6..2c3fce9002 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2631,6 +2631,7 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, GHashTable *nodes = opaque; virJSONValue *img; virJSONValue *bitmaps; + virJSONValue *snapshots; virJSONValue *format_specific; const char *nodename; g_autoptr(qemuBlockNamedNodeData) ent = NULL; @@ -2654,6 +2655,24 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps"))) qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent); + if ((snapshots = virJSONValueObjectGetArray(img, "snapshots"))) { + size_t nsnapshots = virJSONValueArraySize(snapshots); + size_t nsnapnames = 0; + size_t i; + + ent->snapshots = g_new0(char *, nsnapshots + 1); + + for (i = 0; i < nsnapshots; i++) { + virJSONValue *snapshot = virJSONValueArrayGet(snapshots, i); + const char *name = virJSONValueObjectGetString(snapshot, "name"); + + if (!name) + continue; + + ent->snapshots[nsnapnames++] = g_strdup(name); + } + } + /* query qcow2 format specific props */ if ((format_specific = virJSONValueObjectGetObject(img, "format-specific")) && STREQ_NULLABLE(virJSONValueObjectGetString(format_specific, "type"), "qcow2")) { diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 6c4e735466..60ac929e68 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -594,6 +594,15 @@ testQemuDetectBitmapsWorker(GHashTable *nodedata, bitmap->granularity, bitmap->dirtybytes); } + if (data->snapshots) { + char **sn; + + virBufferAddLit(buf, "internal snapshots:"); + + for (sn = data->snapshots; *sn; sn++) + virBufferAsprintf(buf, " '%s'", *sn); + } + virBufferAdjustIndent(buf, -1); } @@ -1201,6 +1210,7 @@ mymain(void) TEST_IMAGE_CREATE("network-rbd-qcow2", NULL); TEST_IMAGE_CREATE("network-ssh-qcow2", NULL); + /* The following group also tests internal snapshot detection */ #define TEST_BITMAP_DETECT(testname) \ do { \ if (virTestRun("bitmap detect " testname, \ @@ -1213,6 +1223,7 @@ mymain(void) TEST_BITMAP_DETECT("basic"); TEST_BITMAP_DETECT("snapshots"); TEST_BITMAP_DETECT("synthetic"); + TEST_BITMAP_DETECT("snapshots-internal"); #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \ do { \ diff --git a/tests/qemublocktestdata/bitmap/snapshots-internal.json b/tests/qemublocktestdata/bitmap/snapshots-internal.json new file mode 100644 index 0000000000..2198f8f364 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-internal.json @@ -0,0 +1,298 @@ +[ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "snapshots": [ + { + "vm-clock-nsec": 992826708, + "name": "1727868651", + "date-sec": 1727868651, + "date-nsec": 317899000, + "vm-clock-sec": 466, + "id": "1", + "vm-state-size": 57440493 + }, + { + "vm-clock-nsec": 159450672, + "name": "1727872064", + "date-sec": 1727872064, + "date-nsec": 991275000, + "vm-clock-sec": 2431, + "id": "2", + "vm-state-size": 57342213 + } + ], + "virtual-size": 104857600, + "filename": "/tmp/internal-snaps.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 115228672, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "compression-type": "zlib", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false, + "extended-l2": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/internal-snaps.qcow2" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 115409408, + "filename": "/tmp/internal-snaps.qcow2", + "format": "file", + "actual-size": 115228672, + "format-specific": { + "type": "file", + "data": { + + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/internal-snaps.qcow2" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 708837376, + "filename": "/var/lib/libvirt/images/systemrescuecd-amd64-6.1.2.iso", + "format": "file", + "actual-size": 708841472, + "format-specific": { + "type": "file", + "data": { + + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-2-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/var/lib/libvirt/images/systemrescuecd-amd64-6.1.2.iso" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "snapshots": [ + { + "vm-clock-nsec": 992826708, + "name": "1727868651", + "date-sec": 1727868651, + "date-nsec": 317899000, + "vm-clock-sec": 466, + "id": "1", + "vm-state-size": 0 + }, + { + "vm-clock-nsec": 159450672, + "name": "1727872064", + "date-sec": 1727872064, + "date-nsec": 991275000, + "vm-clock-sec": 2431, + "id": "2", + "vm-state-size": 0 + } + ], + "virtual-size": 540672, + "filename": "/var/lib/libvirt/qemu/nvram/int-q35_VARS.qcow2", + "cluster-size": 4096, + "format": "qcow2", + "actual-size": 602112, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "compression-type": "zlib", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false, + "extended-l2": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-pflash1-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/var/lib/libvirt/qemu/nvram/int-q35_VARS.qcow2" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 598528, + "filename": "/var/lib/libvirt/qemu/nvram/int-q35_VARS.qcow2", + "format": "file", + "actual-size": 602112, + "format-specific": { + "type": "file", + "data": { + + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-pflash1-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/var/lib/libvirt/qemu/nvram/int-q35_VARS.qcow2" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 3653632, + "filename": "/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2", + "cluster-size": 4096, + "format": "qcow2", + "actual-size": 3678208, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "compression-type": "zlib", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false, + "extended-l2": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-pflash0-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2" + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 3678208, + "filename": "/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2", + "format": "file", + "actual-size": 3678208, + "format-specific": { + "type": "file", + "data": { + + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-pflash0-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2" + } +] diff --git a/tests/qemublocktestdata/bitmap/snapshots-internal.out b/tests/qemublocktestdata/bitmap/snapshots-internal.out new file mode 100644 index 0000000000..f2fb0a1dcc --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-internal.out @@ -0,0 +1,2 @@ +libvirt-1-format: + internal snapshots: '1727868651' '1727872064' -- 2.46.0

On Thu, Oct 03, 2024 at 15:46:31 +0200, Peter Krempa wrote:
Store the names of internal snapshots present in supported images in the data we dump from 'query-named-block-nodes' so that the upcoming changes to the internal snapshot code can access it.
To test this we use the bitmap detection test cases which can be easily extended to dump this data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 19 ++ tests/qemublocktest.c | 11 + .../bitmap/snapshots-internal.json | 298 ++++++++++++++++++ .../bitmap/snapshots-internal.out | 2 + 5 files changed, 333 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.out
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6251f48d28..4341519cfe 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -709,6 +709,9 @@ struct _qemuBlockNamedNodeData { qemuBlockNamedNodeDataBitmap **bitmaps; size_t nbitmaps;
+ /* NULL terminated string list of internal snapshot names */ + char **snapshots; + /* the cluster size of the image is valid only when > 0 */ unsigned long long clusterSize;
The upstream pipeline detected that I've forgot to clear the snapshots string list in qemuMonitorJSONBlockNamedNodeDataFree. Consider the following squashed in: diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2c3fce9002..c594b33106 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2569,6 +2569,7 @@ qemuMonitorJSONBlockNamedNodeDataFree(qemuBlockNamedNodeData *data) for (i = 0; i < data->nbitmaps; i++) qemuMonitorJSONBlockNamedNodeDataBitmapFree(data->bitmaps[i]); + g_strfreev(data->snapshots); g_free(data->bitmaps); g_free(data); }

From: Nikolai Barybin via Devel <devel@lists.libvirt.org> 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 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> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 124 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 116 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f5260c4a22..d4602d386f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -309,6 +309,99 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, } +static char ** +qemuSnapshotActiveInternalCreateGetDevices(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); + size_t ndevs = 0; + size_t i = 0; + + /* This relies on @snapdef being aligned and validated via + * virDomainSnapshotAlignDisks() and qemuSnapshotPrepare(), which also + * ensures that all disks are backed by qcow2. */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + virDomainDiskDef *domdisk = vm->def->disks[i]; + + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + devices[ndevs++] = g_strdup(qemuBlockStorageSourceGetFormatNodename(domdisk->src)); + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + continue; + } + } + + if (vm->def->os.loader && + vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format == VIR_STORAGE_FILE_QCOW2) { + devices[ndevs++] = g_strdup(qemuBlockStorageSourceGetFormatNodename(vm->def->os.loader->nvram)); + } + + return g_steal_pointer(&devices); +} + + +static int +qemuSnapshotCreateActiveInternalDone(virDomainObj *vm, + qemuBlockJobData *job) +{ + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_SNAPSHOT); + + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("snapshot-save job failed: %1$s"), NULLSTR(job->errmsg)); + return -1; + } + + return job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED ? 1 : 0; +} + + +static qemuBlockJobData * +qemuSnapshotCreateActiveInternalStart(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + g_autofree char *jobname = g_strdup_printf("internal-snapshot-save-%s", snapdef->parent.name); + qemuBlockJobData *job = NULL; + g_auto(GStrv) devices = NULL; + int rc = 0; + + if (!(devices = qemuSnapshotActiveInternalCreateGetDevices(vm, snapdef))) + return NULL; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_SAVE, + jobname))) + return NULL; + + qemuBlockJobSyncBegin(job); + + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + goto error; + + rc = qemuMonitorSnapshotSave(qemuDomainGetMonitor(vm), jobname, snapdef->parent.name, + devices[0], (const char **) devices); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto error; + + qemuBlockJobStarted(job, vm); + + return job; + + error: + qemuBlockJobStartupFinalize(vm, job); + return NULL; +} + + /* The domain is expected to be locked and active. */ static int qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, @@ -321,6 +414,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 +436,29 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, } } - if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) { - resume = false; - goto cleanup; - } + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_INTERNAL_QMP)) { + g_autoptr(qemuBlockJobData) job = NULL; - ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitor(vm); - if (ret < 0) - goto cleanup; + if (!(job = qemuSnapshotCreateActiveInternalStart(vm, snapdef))) + goto cleanup; + + while ((rv = qemuSnapshotCreateActiveInternalDone(vm, job)) != 1) { + if (rv < 0 || qemuDomainObjWait(vm) < 0) + goto cleanup; + } + + ret = 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; -- 2.46.0

On 10/3/24 15:46, Peter Krempa wrote:
From: Nikolai Barybin via Devel <devel@lists.libvirt.org>
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 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> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 124 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 116 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f5260c4a22..d4602d386f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -309,6 +309,99 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, }
+static char ** +qemuSnapshotActiveInternalCreateGetDevices(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); + size_t ndevs = 0; + size_t i = 0; + + /* This relies on @snapdef being aligned and validated via + * virDomainSnapshotAlignDisks() and qemuSnapshotPrepare(), which also + * ensures that all disks are backed by qcow2. */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + virDomainDiskDef *domdisk = vm->def->disks[i]; + Could you clarify why to use 2 different types of addressing in 2 consecutive lines of code? ( '+i' and [i]) + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + devices[ndevs++] = g_strdup(qemuBlockStorageSourceGetFormatNodename(domdisk->src)); + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + continue; + } + } + + if (vm->def->os.loader && + vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format == VIR_STORAGE_FILE_QCOW2) { + devices[ndevs++] = g_strdup(qemuBlockStorageSourceGetFormatNodename(vm->def->os.loader->nvram)); + } + + return g_steal_pointer(&devices); +} + + [....]

On Fri, Oct 04, 2024 at 10:43:59 +0200, Nikolai Barybin wrote:
On 10/3/24 15:46, Peter Krempa wrote:
From: Nikolai Barybin via Devel <devel@lists.libvirt.org>
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 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> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 124 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 116 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f5260c4a22..d4602d386f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -309,6 +309,99 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, }
+static char ** +qemuSnapshotActiveInternalCreateGetDevices(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); + size_t ndevs = 0; + size_t i = 0; + + /* This relies on @snapdef being aligned and validated via + * virDomainSnapshotAlignDisks() and qemuSnapshotPrepare(), which also + * ensures that all disks are backed by qcow2. */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + virDomainDiskDef *domdisk = vm->def->disks[i]; + Could you clarify why to use 2 different types of addressing in 2 consecutive lines of code? ( '+i' and [i])
Because : In struct _virDomainSnapshotDef { virDomainSnapshotDiskDef *disks; and in struct _virDomainDef { virDomainDiskDef **disks; the snapshot definition is a array of 'virDomainSnapshotDiskDef' structs, whereas domain definition has an array of pointers to the 'virDomainDiskDef' struct. The two lines differ in the extra dereference (hidden inside the [] operator)

Switch to using the modern QMP command. As the user visible logic when deleting internal snapshots using the old 'delvm' command was very lax in terms of catching inconsistencies between the snapshot metadata and on-disk state we re-implement this behaviour even using the new command. We could improve the validation but that'd go at the cost of possible failures which users might not expect. As 'delvm' was simply ignoring any kind of failure the selection of devices to delete the snapshot from is based on querying qemu first which top level images do have the internal snapshot and then continuing only on those. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 148 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 143 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index d4602d386f..4927ca0bfc 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3711,6 +3711,134 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm, } +static char ** +qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + /* In contrast to internal snapshot *creation* we can't always rely on the + * metadata to give us the full status of the disk images we'd need to + * delete the snapshot from, as users might have edited the VM XML, + * unplugged or plugged in disks and/or did many other kinds of modification. + * + * In order for this code to behave the same as it did with the 'delvm' HMP + * command, which simply deleted the snapshot where it found them and + * ignored any failures we'll detect the images in qemu first and use + * that information as source of truth for now instead of introducing + * other failure scenarios. + */ + g_autoptr(GHashTable) blockNamedNodeData = NULL; + const char *snapname = snapdef->parent.name; + g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); + size_t ndevs = 0; + size_t i = 0; + + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, VIR_ASYNC_JOB_SNAPSHOT))) + return NULL; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDef *domdisk = vm->def->disks[i]; + const char *format_nodename; + qemuBlockNamedNodeData *d; + + /* readonly disks will not have permissions to delete the snapshot, and + * in fact should not have an internal snapshot */ + if (domdisk->src->readonly) + continue; + + /* This effectively filters out empty and 'raw' disks */ + if (!(format_nodename = qemuBlockStorageSourceGetFormatNodename(domdisk->src))) + continue; + + /* the data should always be present */ + if (!(d = virHashLookup(blockNamedNodeData, format_nodename))) + continue; + + /* there might be no snapshot for given disk with given name */ + if (!d->snapshots || + !g_strv_contains((const char **) d->snapshots, snapname)) + continue; + + devices[ndevs++] = g_strdup(format_nodename); + } + + if (vm->def->os.loader && + vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format == VIR_STORAGE_FILE_QCOW2) { + const char *format_nodename; + qemuBlockNamedNodeData *d; + + if ((format_nodename = qemuBlockStorageSourceGetFormatNodename(vm->def->os.loader->nvram)) && + (d = virHashLookup(blockNamedNodeData, format_nodename)) && + d->snapshots && + g_strv_contains((const char **) d->snapshots, snapname)) { + devices[ndevs++] = g_strdup(format_nodename); + } + } + + return g_steal_pointer(&devices); +} + + +static int +qemuSnapshotDiscardActiveInternal(virDomainObj *vm, + virDomainSnapshotDef *snapdef) +{ + g_autofree char *jobname = g_strdup_printf("internal-snapshot-delete-%s", snapdef->parent.name); + qemuBlockJobData *job = NULL; + g_auto(GStrv) devices = NULL; + int ret = -1; + int rc = 0; + + if (!(devices = qemuSnapshotActiveInternalDeleteGetDevices(vm, snapdef))) + return -1; + + /* It's possible that no devices were selected */ + if (devices[0] == NULL) + return 0; + + if (!(job = qemuBlockJobDiskNew(vm, NULL, QEMU_BLOCKJOB_TYPE_SNAPSHOT_DELETE, + jobname))) + return -1; + + qemuBlockJobSyncBegin(job); + + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_SNAPSHOT) < 0) + goto cleanup; + + rc = qemuMonitorSnapshotDelete(qemuDomainGetMonitor(vm), job->name, + snapdef->parent.name, (const char **) devices); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto cleanup; + + qemuBlockJobStarted(job, vm); + + while (true) { + qemuBlockJobUpdate(vm, job, VIR_ASYNC_JOB_SNAPSHOT); + + if (job->state == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("deletion of internal snapshot '%1$s' job failed: %2$s"), + snapdef->parent.name, NULLSTR(job->errmsg)); + goto cleanup; + } + + if (job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED) + break; + + if (qemuDomainObjWait(vm) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + qemuBlockJobStartupFinalize(vm, job); + return ret; +} + + /* Discard one snapshot (or its metadata), without reparenting any children. */ static int qemuSnapshotDiscardImpl(virQEMUDriver *driver, @@ -3750,14 +3878,24 @@ qemuSnapshotDiscardImpl(virQEMUDriver *driver, if (qemuSnapshotDiscardExternal(vm, snap, externalData) < 0) return -1; } else { + virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); + 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_INTERNAL_QMP)) { + if (qemuSnapshotDiscardActiveInternal(vm, snapdef) < 0) + return -1; + } 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.46.0

As explained in the commit which added the new internal snapshot deletion code we don't want to do any form of strict checking whether the libvirt metadata is consistent with the on-disk state as we didn't historically do that. In order to be able to spot the cases add a warning into the logs if such state is encountered. While warnings are easy to miss it's the only reasonable way to do that. Users will be encouraged to file an issue with the information, without requiring them to enable debug logs as the reproduction of that issue may include very old historical state. The checker is deliberately added separately so that it can be easily reverted once it's no longer needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 4927ca0bfc..02c876c881 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3731,6 +3731,14 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); size_t ndevs = 0; size_t i = 0; + /* variables below are used for checking of corner cases */ + g_autoptr(GHashTable) foundDisks = virHashNew(NULL); + g_auto(virBuffer) errMissingSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errUnexpectedSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errExtraDisks = VIR_BUFFER_INITIALIZER; + GHashTableIter htitr; + void *key; + bool warn = false; if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, VIR_ASYNC_JOB_SNAPSHOT))) return NULL; @@ -3759,6 +3767,7 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, continue; devices[ndevs++] = g_strdup(format_nodename); + g_hash_table_insert(foundDisks, g_strdup(domdisk->dst), NULL); } if (vm->def->os.loader && @@ -3775,6 +3784,51 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, } } + /* We currently don't want this code to become stricter than what 'delvm' + * did thus we'll report if the on-disk state mismatches the snapshot + * definition only as a warning */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + g_hash_table_remove(foundDisks, snapdisk->name); + } else { + virBufferAsprintf(&errMissingSnap, "%s ", snapdisk->name); + warn = true; + } + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + virBufferAsprintf(&errUnexpectedSnap, "%s ", snapdisk->name); + warn = true; + } else { + g_hash_table_remove(foundDisks, snapdisk->name); + } + } + } + + g_hash_table_iter_init(&htitr, foundDisks); + + while (g_hash_table_iter_next(&htitr, &key, NULL)) { + warn = true; + virBufferAsprintf(&errExtraDisks, "%s ", (const char *) key); + } + + if (warn) { + VIR_WARN("inconsistent internal snapshot state (deletion): VM='%s' snapshot='%s' missing='%s' unexpected='%s' extra='%s", + vm->def->name, snapname, + virBufferCurrentContent(&errMissingSnap), + virBufferCurrentContent(&errUnexpectedSnap), + virBufferCurrentContent(&errExtraDisks)); + } + return g_steal_pointer(&devices); } -- 2.46.0

On 10/3/24 15:46, Peter Krempa wrote:
As explained in the commit which added the new internal snapshot deletion code we don't want to do any form of strict checking whether the libvirt metadata is consistent with the on-disk state as we didn't historically do that.
In order to be able to spot the cases add a warning into the logs if such state is encountered. While warnings are easy to miss it's the only reasonable way to do that. Users will be encouraged to file an issue with the information, without requiring them to enable debug logs as the reproduction of that issue may include very old historical state.
The checker is deliberately added separately so that it can be easily reverted once it's no longer needed.
Signed-off-by: Peter Krempa<pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 4927ca0bfc..02c876c881 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3731,6 +3731,14 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); size_t ndevs = 0; size_t i = 0; + /* variables below are used for checking of corner cases */ + g_autoptr(GHashTable) foundDisks = virHashNew(NULL); + g_auto(virBuffer) errMissingSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errUnexpectedSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errExtraDisks = VIR_BUFFER_INITIALIZER; + GHashTableIter htitr; + void *key; + bool warn = false;
if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, VIR_ASYNC_JOB_SNAPSHOT))) return NULL; @@ -3759,6 +3767,7 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, continue;
devices[ndevs++] = g_strdup(format_nodename); + g_hash_table_insert(foundDisks, g_strdup(domdisk->dst), NULL); }
if (vm->def->os.loader && @@ -3775,6 +3784,51 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, } }
+ /* We currently don't want this code to become stricter than what 'delvm' + * did thus we'll report if the on-disk state mismatches the snapshot + * definition only as a warning */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + g_hash_table_remove(foundDisks, snapdisk->name); + } else { + virBufferAsprintf(&errMissingSnap, "%s ", snapdisk->name); + warn = true; + } + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + virBufferAsprintf(&errUnexpectedSnap, "%s ", snapdisk->name); + warn = true; + } else { + g_hash_table_remove(foundDisks, snapdisk->name); + }
This if-else statement looks illogical or I don't understand how it works. Probably you meant if (!g_hash_table_contains... ?
+ } + } + + g_hash_table_iter_init(&htitr, foundDisks); + + while (g_hash_table_iter_next(&htitr, &key, NULL)) { + warn = true; + virBufferAsprintf(&errExtraDisks, "%s ", (const char *) key); + } + + if (warn) { + VIR_WARN("inconsistent internal snapshot state (deletion): VM='%s' snapshot='%s' missing='%s' unexpected='%s' extra='%s", + vm->def->name, snapname, + virBufferCurrentContent(&errMissingSnap), + virBufferCurrentContent(&errUnexpectedSnap), + virBufferCurrentContent(&errExtraDisks)); + } + return g_steal_pointer(&devices); }

On Fri, Oct 04, 2024 at 10:25:07 +0200, Nikolai Barybin wrote:
On 10/3/24 15:46, Peter Krempa wrote:
As explained in the commit which added the new internal snapshot deletion code we don't want to do any form of strict checking whether the libvirt metadata is consistent with the on-disk state as we didn't historically do that.
In order to be able to spot the cases add a warning into the logs if such state is encountered. While warnings are easy to miss it's the only reasonable way to do that. Users will be encouraged to file an issue with the information, without requiring them to enable debug logs as the reproduction of that issue may include very old historical state.
The checker is deliberately added separately so that it can be easily reverted once it's no longer needed.
Signed-off-by: Peter Krempa<pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 4927ca0bfc..02c876c881 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3731,6 +3731,14 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2); size_t ndevs = 0; size_t i = 0; + /* variables below are used for checking of corner cases */ + g_autoptr(GHashTable) foundDisks = virHashNew(NULL); + g_auto(virBuffer) errMissingSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errUnexpectedSnap = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) errExtraDisks = VIR_BUFFER_INITIALIZER; + GHashTableIter htitr; + void *key; + bool warn = false;
if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, VIR_ASYNC_JOB_SNAPSHOT))) return NULL; @@ -3759,6 +3767,7 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, continue;
devices[ndevs++] = g_strdup(format_nodename); + g_hash_table_insert(foundDisks, g_strdup(domdisk->dst), NULL); }
if (vm->def->os.loader && @@ -3775,6 +3784,51 @@ qemuSnapshotActiveInternalDeleteGetDevices(virDomainObj *vm, } }
+ /* We currently don't want this code to become stricter than what 'delvm' + * did thus we'll report if the on-disk state mismatches the snapshot + * definition only as a warning */ + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i; + + switch (snapdisk->snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + g_hash_table_remove(foundDisks, snapdisk->name); + } else { + virBufferAsprintf(&errMissingSnap, "%s ", snapdisk->name); + warn = true; + } + break; + + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + if (g_hash_table_contains(foundDisks, snapdisk->name)) { + virBufferAsprintf(&errUnexpectedSnap, "%s ", snapdisk->name); + warn = true; + } else { + g_hash_table_remove(foundDisks, snapdisk->name); + }
This if-else statement looks illogical or I don't understand how it works. Probably you meant
if (!g_hash_table_contains... ?
Yes there is a problem but the condition is correct. The code is supposted to prepare the warning if the snapshot metadata doesn't say that a disk should have an intenal snapshot but the disk image does have it. What's incorrect is that it shouldn't have the " } else {" line and the entry should be removed from the hash table when the warning is prepared ...
+ } + } + + g_hash_table_iter_init(&htitr, foundDisks); + + while (g_hash_table_iter_next(&htitr, &key, NULL)) { + warn = true; + virBufferAsprintf(&errExtraDisks, "%s ", (const char *) key);
... so that it doesn't trigger this.
+ } + + if (warn) { + VIR_WARN("inconsistent internal snapshot state (deletion): VM='%s' snapshot='%s' missing='%s' unexpected='%s' extra='%s", + vm->def->name, snapname, + virBufferCurrentContent(&errMissingSnap), + virBufferCurrentContent(&errUnexpectedSnap), + virBufferCurrentContent(&errExtraDisks)); + } + return g_steal_pointer(&devices); }
I'll delete the '} else {' line on my local branch.

With the new snapshot QMP command we can select which block device backend receives the VM state and thus the main issue with internal snapshots with pflash was addressed. Thus we can relax the check and allow snapshots if the pflash nvram is on qcow2. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 02c876c881..a36dff9dc8 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -798,6 +798,7 @@ qemuSnapshotPrepare(virDomainObj *vm, bool *has_manual, unsigned int *flags) { + qemuDomainObjPrivate *priv = vm->privateData; size_t i; bool active = virDomainObjIsActive(vm); bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; @@ -899,7 +900,8 @@ qemuSnapshotPrepare(virDomainObj *vm, return -1; } - /* internal snapshots + pflash based loader have the following problems: + /* internal snapshots + pflash based loader have the following problems when + * using the old HMP 'savevm' command: * - if the variable store is raw, the snapshot fails * - allowing a qcow2 image as the varstore would make it eligible to receive * the vmstate dump, which would make it huge @@ -910,14 +912,28 @@ qemuSnapshotPrepare(virDomainObj *vm, * not an issue. Allow this as there are existing users of this case. * * Avoid the issues by forbidding internal snapshot with pflash if the - * VM is active. + * VM is active when using 'savevm'. + * + * With the new QMP commands we can control where the VM state (memory) + * image goes and thus can allow snapshots, but we'll still require that the + * varstore is in qcow2 format. */ - if (active && - found_internal && - virDomainDefHasOldStyleUEFI(vm->def)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("internal snapshots of a VM with pflash based firmware are not supported")); - return -1; + if (active && found_internal) { + if (virDomainDefHasOldStyleUEFI(vm->def) && + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SNAPSHOT_INTERNAL_QMP)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("internal snapshots of a VM with pflash based firmware are not supported with this qemu")); + return -1; + } + + if (vm->def->os.loader && + vm->def->os.loader->nvram && + vm->def->os.loader->nvram->format != VIR_STORAGE_FILE_QCOW2) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("internal snapshots of a VM with pflash based firmware require QCOW2 nvram format")); + return -1; + } + } /* Alter flags to let later users know what we learned. */ -- 2.46.0

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 437e032b90..7efc440202 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -19,6 +19,22 @@ v10.9.0 (unreleased) * **Improvements** + * qemu: internal snapshot improvements + + The qemu internal snapshot handling code was updated to use modern commands + which avoid the problems the old ones had, preventing use of internal + snapshots on VMs with UEFI NVRAM. Internal snapshots of VMs using UEFI are + now possible provided that the NVRAM is in ``qcow2`` format. + + The new code also allows better control when deleting snapshots. To prevent + possible regressions no strict checking is done, but in case a inconsistent + state is encountered a log message is added:: + + warning : qemuSnapshotActiveInternalDeleteGetDevices:3841 : inconsistent internal snapshot state (deletion): VM='snap' snapshot='1727959843' missing='vda ' unexpected='' extra='' + + Users are encouraged to report any occurence of the above message along + with steps they took to the upstream tracker. + * **Bug fixes** -- 2.46.0

On Thu, Oct 03, 2024 at 03:46:36PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index 437e032b90..7efc440202 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -19,6 +19,22 @@ v10.9.0 (unreleased)
* **Improvements**
+ * qemu: internal snapshot improvements + + The qemu internal snapshot handling code was updated to use modern commands + which avoid the problems the old ones had, preventing use of internal + snapshots on VMs with UEFI NVRAM. Internal snapshots of VMs using UEFI are + now possible provided that the NVRAM is in ``qcow2`` format. + + The new code also allows better control when deleting snapshots. To prevent + possible regressions no strict checking is done, but in case a inconsistent
s/a inconsistent/an inconsistent/
+ state is encountered a log message is added:: + + warning : qemuSnapshotActiveInternalDeleteGetDevices:3841 : inconsistent internal snapshot state (deletion): VM='snap' snapshot='1727959843' missing='vda ' unexpected='' extra='' + + Users are encouraged to report any occurence of the above message along + with steps they took to the upstream tracker. + * **Bug fixes**
-- 2.46.0

On 10/3/24 15:46, Peter Krempa wrote:
Changes to v2: - added few cleanups - added qemumonitorjson test cases for new commands - dropped duplicate capability flags - fixed bugs pointed out in v2 - rewritten logic for selecting snapshot images (both for creation/deletion) - fixed the logic to be fault tolerant same way as 'delvm' - allowed internal snapshots of VMs with UEFI - added explanation to all code with non-obvious implications
Nikolai Barybin via Devel (4): qemu: monitor: Add plumbing for 'snaphot-save'/'snapshot-delete' QMP commands qemu: blockjob: Add job types for 'snapshot-save/delete' qemu: capabilities: Introduce QEMU_CAPS_SNAPSHOT_INTERNAL_QMP capability qemu snapshot: use QMP snapshot-save for internal snapshots creation
Peter Krempa (6): qemuDomainObjWait: Annotate with G_GNUC_WARN_UNUSED_RESULT qemu: monitor: Store internal snapshot names from 'query-named-block-nodes' qemu snapshot: use QMP snapshot-delete for internal snapshots deletion qemuSnapshotActiveInternalDeleteGetDevices: Add warning when deleting inconsistent snapshot qemu: snapshot: Allow internal snapshots with PFLASH nvram news: mention internal snapshot changes
NEWS.rst | 16 + src/qemu/qemu_block.c | 2 + src/qemu/qemu_blockjob.c | 7 + src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_domain.c | 10 + src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_monitor.c | 30 ++ src/qemu/qemu_monitor.h | 17 + src/qemu/qemu_monitor_json.c | 79 ++++ src/qemu/qemu_monitor_json.h | 13 + src/qemu/qemu_snapshot.c | 358 +++++++++++++++++- tests/qemublocktest.c | 11 + .../bitmap/snapshots-internal.json | 298 +++++++++++++++ .../bitmap/snapshots-internal.out | 2 + .../caps_6.0.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.0.0_s390x.xml | 1 + .../caps_6.0.0_x86_64.xml | 1 + .../caps_6.1.0_x86_64.xml | 1 + .../caps_6.2.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 + .../caps_6.2.0_x86_64.xml | 1 + .../caps_7.0.0_aarch64+hvf.xml | 1 + .../caps_7.0.0_aarch64.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + .../caps_7.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_riscv64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_aarch64.xml | 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml | 1 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_9.0.0_sparc.xml | 1 + .../caps_9.0.0_x86_64.xml | 1 + .../caps_9.1.0_riscv64.xml | 1 + .../caps_9.1.0_x86_64.xml | 1 + tests/qemumonitorjsontest.c | 33 ++ 46 files changed, 895 insertions(+), 22 deletions(-) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-internal.out
I reviewed and built all the patches and everything looks fine except for minor comments I left below.

On Thu, Oct 03, 2024 at 03:46:26PM +0200, Peter Krempa wrote:
Changes to v2: - added few cleanups - added qemumonitorjson test cases for new commands - dropped duplicate capability flags - fixed bugs pointed out in v2 - rewritten logic for selecting snapshot images (both for creation/deletion) - fixed the logic to be fault tolerant same way as 'delvm' - allowed internal snapshots of VMs with UEFI - added explanation to all code with non-obvious implications
Nikolai Barybin via Devel (4): qemu: monitor: Add plumbing for 'snaphot-save'/'snapshot-delete' QMP commands qemu: blockjob: Add job types for 'snapshot-save/delete' qemu: capabilities: Introduce QEMU_CAPS_SNAPSHOT_INTERNAL_QMP capability qemu snapshot: use QMP snapshot-save for internal snapshots creation
Peter Krempa (6): qemuDomainObjWait: Annotate with G_GNUC_WARN_UNUSED_RESULT qemu: monitor: Store internal snapshot names from 'query-named-block-nodes' qemu snapshot: use QMP snapshot-delete for internal snapshots deletion qemuSnapshotActiveInternalDeleteGetDevices: Add warning when deleting inconsistent snapshot qemu: snapshot: Allow internal snapshots with PFLASH nvram news: mention internal snapshot changes
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (3)
-
Nikolai Barybin
-
Pavel Hrdina
-
Peter Krempa