From: Or Ozeri <oro(a)il.ibm.com&gt; libvirt supports taking external disk snapshots on a running VM, using qemu's "blockdev-snapshot" command. qemu also supports "blockdev-snapshot-internal-sync" to do the same for internal snapshots. This commit wraps this (old) qemu capability to allow future libvirt users to take internal disk snapshots on a running VM. This will only work for disk types which support internal snapshots, and thus we require the disk type to be part of a white list of known types. For this commit, the list of supported formats is empty. An upcoming commit will allow RBD disks to use this new capability. Signed-off-by: Or Ozeri <oro(a)il.ibm.com&gt; --- src/qemu/qemu_monitor.c | 9 +++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 14 ++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_snapshot.c | 72 ++++++++++++------- .../disk_snapshot.xml | 2 +- .../disk_snapshot.xml | 2 +- .../disk_snapshot_redefine.xml | 2 +- tests/qemumonitorjsontest.c | 1 + 9 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 38f89167e0..f6dab34243 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4225,6 +4225,15 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue *actions, } +int +qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions, + const char *device, + const char *name) +{ + return qemuMonitorJSONTransactionInternalSnapshotBlockdev(actions, device, name); +} + + int qemuMonitorTransactionBackup(virJSONValue *actions, const char *device, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2d16214ba2..1bfd1ccbc2 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1411,6 +1411,11 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue *actions, const char *node, const char *overlay); +int +qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions, + const char *device, + const char *name); + typedef enum { QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_NONE = 0, QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_INCREMENTAL, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index db99017555..002a6caa52 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8307,6 +8307,20 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue *actions, NULL); } + +int +qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions, + const char *device, + const char *name) +{ + return qemuMonitorJSONTransactionAdd(actions, + "blockdev-snapshot-internal-sync", + "s:device", device, + "s:name", name, + NULL); +} + + VIR_ENUM_DECL(qemuMonitorTransactionBackupSyncMode); VIR_ENUM_IMPL(qemuMonitorTransactionBackupSyncMode, QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_LAST, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 6f376cf9b7..313004f327 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -779,6 +779,11 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue *actions, const char *node, const char *overlay); +int +qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions, + const char *device, + const char *name); + int qemuMonitorJSONTransactionBackup(virJSONValue *actions, const char *device, diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index c1855b3028..e82352ba7d 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -736,7 +736,7 @@ qemuSnapshotPrepare(virDomainObj *vm, } /* disk snapshot requires at least one disk */ - if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && !external) { + if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && !external && !found_internal) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk-only snapshots require at least " "one disk to be selected for snapshot")); @@ -852,6 +852,7 @@ qemuSnapshotDiskCleanup(qemuSnapshotDiskData *data, struct _qemuSnapshotDiskContext { qemuSnapshotDiskData *dd; size_t ndd; + bool has_internal; virJSONValue *actions; @@ -1070,17 +1071,17 @@ qemuSnapshotDiskPrepareOne(qemuSnapshotDiskContext *snapctxt, /** - * qemuSnapshotDiskPrepareActiveExternal: + * qemuSnapshotDiskPrepareActive: * * Collects and prepares a list of structures that hold information about disks * that are selected for the snapshot. */ static qemuSnapshotDiskContext * -qemuSnapshotDiskPrepareActiveExternal(virDomainObj *vm, - virDomainMomentObj *snap, - bool reuse, - GHashTable *blockNamedNodeData, - virDomainAsyncJob asyncJob) +qemuSnapshotDiskPrepareActive(virDomainObj *vm, + virDomainMomentObj *snap, + bool reuse, + GHashTable *blockNamedNodeData, + virDomainAsyncJob asyncJob) { g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; size_t i; @@ -1089,16 +1090,33 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObj *vm, snapctxt = qemuSnapshotDiskContextNew(snapdef->ndisks, vm, asyncJob); for (i = 0; i < snapdef->ndisks; i++) { - if (snapdef->disks[i].snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) - continue; + switch (snapdef->disks[i].snapshot) { + case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: { + if (qemuSnapshotDiskPrepareOne(snapctxt, + vm->def->disks[i], + snapdef->disks + i, + blockNamedNodeData, + reuse, + true) < 0) + return NULL; + break; + } - if (qemuSnapshotDiskPrepareOne(snapctxt, - vm->def->disks[i], - snapdef->disks + i, - blockNamedNodeData, - reuse, - true) < 0) - return NULL; + case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: { + snapctxt->has_internal = true; + if (qemuMonitorTransactionInternalSnapshotBlockdev(snapctxt->actions, + vm->def->disks[i]->src->nodeformat, + snapdef->disks[i].snapshot_name) < 0) + return NULL; + break; + } + + case VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT: + case VIR_DOMAIN_SNAPSHOT_LOCATION_NO: + case VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL: + case VIR_DOMAIN_SNAPSHOT_LOCATION_LAST: + continue; + } } return g_steal_pointer(&snapctxt); @@ -1182,7 +1200,7 @@ qemuSnapshotDiskCreate(qemuSnapshotDiskContext *snapctxt) int rc; /* check whether there's anything to do */ - if (snapctxt->ndd == 0) + if (snapctxt->ndd == 0 && !snapctxt->has_internal) return 0; if (qemuDomainObjEnterMonitorAsync(snapctxt->vm, snapctxt->asyncJob) < 0) @@ -1215,11 +1233,11 @@ qemuSnapshotDiskCreate(qemuSnapshotDiskContext *snapctxt) /* The domain is expected to be locked and active. */ static int -qemuSnapshotCreateActiveExternalDisks(virDomainObj *vm, - virDomainMomentObj *snap, - GHashTable *blockNamedNodeData, - unsigned int flags, - virDomainAsyncJob asyncJob) +qemuSnapshotCreateActiveDisks(virDomainObj *vm, + virDomainMomentObj *snap, + GHashTable *blockNamedNodeData, + unsigned int flags, + virDomainAsyncJob asyncJob) { bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL; @@ -1229,8 +1247,8 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObj *vm, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (!(snapctxt = qemuSnapshotDiskPrepareActiveExternal(vm, snap, reuse, - blockNamedNodeData, asyncJob))) + if (!(snapctxt = qemuSnapshotDiskPrepareActive(vm, snap, reuse, + blockNamedNodeData, asyncJob))) return -1; if (qemuSnapshotDiskCreate(snapctxt) < 0) @@ -1370,9 +1388,9 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, /* the domain is now paused if a memory snapshot was requested */ - if ((ret = qemuSnapshotCreateActiveExternalDisks(vm, snap, - blockNamedNodeData, flags, - VIR_ASYNC_JOB_SNAPSHOT)) < 0) + if ((ret = qemuSnapshotCreateActiveDisks(vm, snap, + blockNamedNodeData, flags, + VIR_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; /* the snapshot is complete now */ diff --git a/tests/qemudomainsnapshotxml2xmlin/disk_snapshot.xml b/tests/qemudomainsnapshotxml2xmlin/disk_snapshot.xml index cf5ea0814e..87b6251a7f 100644 --- a/tests/qemudomainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/qemudomainsnapshotxml2xmlin/disk_snapshot.xml @@ -4,7 +4,7 @@ <disks> <disk name='/dev/HostVG/QEMUGuest1'/> <disk name='hdb' snapshot='no'/> - <disk name='hdc' snapshot='internal'/> + <disk name='hdc' snapshot='internal' snapshotName='snap1'/> <disk name='hdd' snapshot='external'> <source/> <driver type='qed'/> diff --git a/tests/qemudomainsnapshotxml2xmlout/disk_snapshot.xml b/tests/qemudomainsnapshotxml2xmlout/disk_snapshot.xml index 76c543d25c..6cf93183d5 100644 --- a/tests/qemudomainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/qemudomainsnapshotxml2xmlout/disk_snapshot.xml @@ -5,7 +5,7 @@ <disks> <disk name='/dev/HostVG/QEMUGuest1'/> <disk name='hdb' snapshot='no'/> - <disk name='hdc' snapshot='internal'/> + <disk name='hdc' snapshot='internal' snapshotName='snap1'/> <disk name='hdd' snapshot='external' type='file'> <driver type='qed'/> </disk> diff --git a/tests/qemudomainsnapshotxml2xmlout/disk_snapshot_redefine.xml b/tests/qemudomainsnapshotxml2xmlout/disk_snapshot_redefine.xml index 24b41ba7c5..f574793edf 100644 --- a/tests/qemudomainsnapshotxml2xmlout/disk_snapshot_redefine.xml +++ b/tests/qemudomainsnapshotxml2xmlout/disk_snapshot_redefine.xml @@ -10,7 +10,7 @@ <disks> <disk name='hda' snapshot='no'/> <disk name='hdb' snapshot='no'/> - <disk name='hdc' snapshot='internal'/> + <disk name='hdc' snapshot='internal' snapshotName='snap1'/> <disk name='hdd' snapshot='external' type='file'> <driver type='qed'/> <source file='/path/to/generated4'/> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 1db1f2b949..1269c74e43 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2587,6 +2587,7 @@ testQemuMonitorJSONTransaction(const void *opaque) qemuMonitorTransactionBitmapDisable(actions, "node4", "bitmap4") < 0 || qemuMonitorTransactionBitmapMerge(actions, "node5", "bitmap5", &mergebitmaps) < 0 || qemuMonitorTransactionSnapshotBlockdev(actions, "node7", "overlay7") < 0 || + qemuMonitorTransactionInternalSnapshotBlockdev(actions, "device1", "snapshot1") < 0 || qemuMonitorTransactionBackup(actions, "dev8", "job8", "target8", "bitmap8", QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_NONE) < 0 || qemuMonitorTransactionBackup(actions, "dev9", "job9", "target9", "bitmap9", -- 2.25.1