8:46 a.m.
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
8:46 a.m.
New subject: [PATCH v3 01/10] qemuDomainObjWait: Annotate with G_GNUC_WARN_UNUSED_RESULT
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(a)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
8:46 a.m.
New subject: [PATCH v3 02/10] qemu: monitor: Add plumbing for 'snaphot-save'/'snapshot-delete' QMP commands
From: Nikolai Barybin via Devel <devel(a)lists.libvirt.org>
Signed-off-by: Nikolai Barybin <nikolai.barybin(a)virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
8:46 a.m.
New subject: [PATCH v3 03/10] qemu: blockjob: Add job types for 'snapshot-save/delete'
From: Nikolai Barybin via Devel <devel(a)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(a)virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
8:46 a.m.
New subject: [PATCH v3 04/10] qemu: capabilities: Introduce QEMU_CAPS_SNAPSHOT_INTERNAL_QMP capability
From: Nikolai Barybin via Devel <devel(a)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(a)virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
8:46 a.m.
New subject: [PATCH v3 05/10] qemu: monitor: Store internal snapshot names from 'query-named-block-nodes'
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(a)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
9 a.m.
New subject: [PATCH v3 05/10] qemu: monitor: Store internal snapshot names
from 'query-named-block-nodes'
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(a)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);
}
8:46 a.m.
New subject: [PATCH v3 06/10] qemu snapshot: use QMP snapshot-save for internal snapshots creation
From: Nikolai Barybin via Devel <devel(a)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(a)virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
3:43 a.m.
New subject: [PATCH v3 06/10] qemu snapshot: use QMP snapshot-save for
internal snapshots creation
On 10/3/24 15:46, Peter Krempa wrote:
From: Nikolai Barybin via Devel <devel(a)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(a)virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa(a)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);
+}
+
+
[....]
5:20 a.m.
New subject: [PATCH v3 06/10] qemu snapshot: use QMP snapshot-save for
internal snapshots creation
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(a)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(a)virtuozzo.com>
> Signed-off-by: Peter Krempa <pkrempa(a)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)
8:46 a.m.
New subject: [PATCH v3 07/10] qemu snapshot: use QMP snapshot-delete for internal snapshots deletion
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(a)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
8:46 a.m.
New subject: [PATCH v3 08/10] qemuSnapshotActiveInternalDeleteGetDevices: Add warning when deleting inconsistent snapshot
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(a)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
3:25 a.m.
New subject: [PATCH v3 08/10] qemuSnapshotActiveInternalDeleteGetDevices: Add
warning when deleting inconsistent snapshot
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(a)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);
}
3:37 a.m.
New subject: [PATCH v3 08/10] qemuSnapshotActiveInternalDeleteGetDevices: Add
warning when deleting inconsistent snapshot
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(a)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.
8:46 a.m.
New subject: [PATCH v3 09/10] qemu: snapshot: Allow internal snapshots with PFLASH nvram
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(a)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
8:46 a.m.
New subject: [PATCH v3 10/10] news: mention internal snapshot changes
Signed-off-by: Peter Krempa <pkrempa(a)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
8:14 a.m.
New subject: [PATCH v3 10/10] news: mention internal snapshot changes
On Thu, Oct 03, 2024 at 03:46:36PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)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
3:45 a.m.
New subject: [PATCH v3 00/10] qemu: Rework internal active snapshots to use
QMP commands
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.
8:14 a.m.
New subject: [PATCH v3 00/10] qemu: Rework internal active snapshots to use
QMP commands
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(a)redhat.com>
74
days inactive
80
days old
18 comments
3 participants
participants (3)
-
Nikolai Barybin -
Pavel Hrdina -
Peter Krempa