This is to delete a snapshot object atomically, the API accepts argments: domain object, snapshot name and flags. Add a new flag VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT, others are same. include/libvirt/libvirt.h.in: Declare virDomainSnapshotDeleteByName Add VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT src/driver.h: (virDrvDomainSnapshotDeleteByName) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 5 +++ src/driver.h | 6 ++++ src/libvirt.c | 73 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 4 files changed, 89 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 574b970..fa90197 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4339,11 +4339,16 @@ typedef enum { VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN = (1 << 0), /* Also delete children */ VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY = (1 << 1), /* Delete just metadata */ VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY = (1 << 2), /* Delete just children */ + VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT = (1 << 3), /* Delete current snapshot */ } virDomainSnapshotDeleteFlags; int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags); +int virDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags); + int virDomainSnapshotRef(virDomainSnapshotPtr snapshot); int virDomainSnapshotFree(virDomainSnapshotPtr snapshot); diff --git a/src/driver.h b/src/driver.h index ec5fc53..24dd262 100644 --- a/src/driver.h +++ b/src/driver.h @@ -801,6 +801,11 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainSnapshotDeleteByName)(virDomainPtr domain, + const char *cmd, + unsigned int flags); + +typedef int (*virDrvDomainQemuMonitorCommand)(virDomainPtr domain, const char *cmd, char **result, @@ -1211,6 +1216,7 @@ struct _virDriver { virDrvDomainSnapshotHasMetadata domainSnapshotHasMetadata; virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; + virDrvDomainSnapshotDeleteByName domainSnapshotDeleteByName; virDrvDomainQemuMonitorCommand domainQemuMonitorCommand; virDrvDomainQemuAttach domainQemuAttach; virDrvDomainQemuAgentCommand domainQemuAgentCommand; diff --git a/src/libvirt.c b/src/libvirt.c index 620dbdd..cf5de87 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19346,6 +19346,10 @@ error: * libvirt metadata to track snapshots, then this flag is silently * ignored. * + * Note that this command is inherently racy: another connection can + * delete the snapshot object between a call to virDomainSnapshotLookupByName() + * and this call. + * * Returns 0 if the selected snapshot(s) were successfully deleted, * -1 on error. */ @@ -19395,6 +19399,75 @@ error: } /** + * virDomainSnapshotDeleteByName: + * @domain: pointer to the domain object + * @name : snapshot name or NULL + * @flags: bitwise-OR of supported virDomainSnapshotDeleteFlags + * + * Delete the snapshot. + * + * If @flags is 0, then just this snapshot is deleted, and changes + * from this snapshot are automatically merged into children + * snapshots. If @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, + * then this snapshot and any descendant snapshots are deleted. If + * @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, then any + * descendant snapshots are deleted, but this snapshot remains. These + * two flags are mutually exclusive. If @flags includes + * VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT, current snapshot is deleted, + * in this case the @name will be ingored. + * + * If @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, then + * any snapshot metadata tracked by libvirt is removed while keeping + * the snapshot contents intact; if a hypervisor does not require any + * libvirt metadata to track snapshots, then this flag is silently + * ignored. + * + * Returns 0 if the selected snapshot(s) were successfully deleted, + * -1 on error. + */ +int +virDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "name=%s, flags=%x", name, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) && + (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) { + virReportInvalidArg(flags, + _("children and children_only flags in %s are " + "mutually exclusive"), + __FUNCTION__); + goto error; + } + + if (domain->conn->driver->domainSnapshotDelete) { + int ret = domain->conn->driver->domainSnapshotDeleteByName(domain, name, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainSnapshotRef: * @snapshot: the snapshot to hold a reference on * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ee2d27..6b4cac2 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -621,4 +621,9 @@ LIBVIRT_1.0.6 { virGetLastErrorMessage; } LIBVIRT_1.0.5; +LIBVIRT_1.0.7 { + global: + virDomainSnapshotDeleteByName; +}LIBVIRT_1.0.6; + # .... define new API here using predicted next version number .... -- 1.8.1.4

On 06/21/2013 05:55 PM, Ján Tomko wrote: Sorry, it is so late to respond. I agree what you pointed out above, the DELETE_CURRENT didn't make itself clear. For SnapshotDeleteByNam(), it accepts NULL as name, so it could be simpler to represent current snapshot object. I will change it in v2. Thanks. Guannan

Refactor orgininal qemuDomainSnapshotDelete() into a common function qemuDomainSnapshotDeleteImpl() which is used by both APIs SnapshotDelete() and SnapshotDeleteByName() --- src/qemu/qemu_driver.c | 102 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c886378..152a1fd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12847,30 +12847,19 @@ qemuDomainSnapshotReparentChildren(void *payload, rep->cfg->snapshotDir); } - -static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, - unsigned int flags) +static int +qemuDomainSnapshotDeleteImpl(virQEMUDriverPtr driver, + virDomainObjPtr *vmptr, + virDomainSnapshotObjPtr snap, + unsigned int flags) { - virQEMUDriverPtr driver = snapshot->domain->conn->privateData; - virDomainObjPtr vm = NULL; + virDomainObjPtr vm = *vmptr; int ret = -1; - virDomainSnapshotObjPtr snap = NULL; virQEMUSnapRemove rem; virQEMUSnapReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); int external = 0; - virQEMUDriverConfigPtr cfg = NULL; - - virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | - VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | - VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1); - - if (!(vm = qemuDomObjFromSnapshot(snapshot))) - return -1; - - cfg = virQEMUDriverGetConfig(driver); - if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) - goto cleanup; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (!metadata_only) { if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) && @@ -12948,12 +12937,86 @@ endjob: vm = NULL; cleanup: + virObjectUnref(cfg); + return ret; +} + + +static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virQEMUDriverPtr driver = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1); + + if (!(vm = qemuDomObjFromSnapshot(snapshot))) + return -1; + + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + ret = qemuDomainSnapshotDeleteImpl(driver, &vm, snap, flags); + +cleanup: if (vm) virObjectUnlock(vm); - virObjectUnref(cfg); return ret; } + +static int +qemuDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags) +{ + virQEMUDriverPtr driver = domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + const char *snapname = name; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY | + VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT, -1); + + if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT) && !snapname) { + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", + _("cannot determine snapshot object")); + goto cleanup; + } + + if (!(vm = qemuDomObjFromDomain(domain))) + goto cleanup; + + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT) { + if (!vm->current_snapshot) { + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", + _("the domain does not have a current snapshot")); + goto cleanup; + } + + snapname = vm->current_snapshot->def->name; + flags &= ~VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT; + } + + if (!(snap = qemuSnapObjFromName(vm, snapname))) + goto cleanup; + + ret = qemuDomainSnapshotDeleteImpl(driver, &vm, snap, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags) { @@ -15333,6 +15396,7 @@ static virDriver qemuDriver = { .domainSnapshotHasMetadata = qemuDomainSnapshotHasMetadata, /* 0.9.13 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */ + .domainSnapshotDeleteByName = qemuDomainSnapshotDeleteByName, /* 1.0.7 */ .domainQemuMonitorCommand = qemuDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = qemuDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */ -- 1.8.1.4

--- tools/virsh-snapshot.c | 64 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 7e75772..4044e9b 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1966,37 +1966,81 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; unsigned int flags = 0; + bool children = vshCommandOptBool(cmd, "children"); + bool metadata = vshCommandOptBool(cmd, "metadata"); + bool children_only = vshCommandOptBool(cmd, "children-only"); + bool current = vshCommandOptBool(cmd, "current"); + + VSH_EXCLUSIVE_OPTIONS_VAR(children, children_only); + + if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &name) < 0) + goto cleanup; + + if (!current && !name){ + vshError(ctl, _("--snapshotname or --current is required")); + goto cleanup; + } dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; + if (children) + flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN; + if (metadata) + flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; + if (children_only) + flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY; + if (current) + flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT; + + if (virDomainSnapshotDeleteByName(dom, name, flags) == 0) + goto finished; + + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + goto error; + +fallback: + /* virDomainSnapshotDelete doesn't support DELETE_CURRENT flag */ + flags &= ~VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT; + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, &snapshot, &name) < 0) goto cleanup; - if (vshCommandOptBool(cmd, "children")) - flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN; - if (vshCommandOptBool(cmd, "children-only")) - flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY; - if (vshCommandOptBool(cmd, "metadata")) - flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; - /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on * older servers that reject the flag, by manually computing the * list of descendants. But that's a lot of code to maintain. */ - if (virDomainSnapshotDelete(snapshot, flags) == 0) { + if (virDomainSnapshotDelete(snapshot, flags) < 0) + goto error; + +finished: + if (name) { if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) vshPrint(ctl, _("Domain snapshot %s children deleted\n"), name); else vshPrint(ctl, _("Domain snapshot %s deleted\n"), name); } else { - vshError(ctl, _("Failed to delete snapshot %s"), name); - goto cleanup; + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) + vshPrint(ctl, _("Domain current snapshot children deleted\n")); + else + vshPrint(ctl, _("Domain current snapshot deleted\n")); } ret = true; +error: + if (ret == false) { + if (name) + vshError(ctl, _("Failed to delete snapshot %s"), name); + else + vshError(ctl, _("Failed to delete current snapshot")); + } + cleanup: if (snapshot) virDomainSnapshotFree(snapshot); -- 1.8.1.4