[libvirt] [PATCH v2 0/5]Atomic API to delete snapshot object

v1: https://www.redhat.com/archives/libvir-list/2013-June/msg00573.html v1->v2: Remove VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT flag (name == NULL) means deleting current snapshot object Rebase work Add a new snapshot API to delete snapshot object atomically int virDomainSnapshotDeleteByName(virDomainPtr domain, const char *name, unsigned int flags); The existing virDomainSnapshotDelete API accepts the snapshot object being deleted as an argument that would be not API atomic. Guannan Ren(5) [PATCH v2 1/5] snapshot: define new API virDomainSnapshotDeleteByName [PATCH v2 2/5] auto generate RPC calls for remoteDomainSnapshotDeleteByName [PATCH v2 3/5] qemu: implement SnapshotDeleteByName [PATCH v2 4/5] python: make auto-generated function name nicer [PATCH v2 5/5] virsh: use virDomainSnapshotDeleteByName in virsh include/libvirt/libvirt.h.in | 4 ++++ python/generator.py | 3 +++ src/driver.h | 6 ++++++ src/libvirt.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/qemu/qemu_driver.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 6 ++++++ tools/virsh-snapshot.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- 10 files changed, 245 insertions(+), 33 deletions(-)

This is to delete a snapshot object atomically, the API accepts argments: domain object, snapshot name and flags. When name is NULL, the current snapshot object will be deleted include/libvirt/libvirt.h.in: Declare virDomainSnapshotDeleteByName 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 | 4 +++ src/driver.h | 6 ++++ src/libvirt.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 86 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b791125..e7e75c0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4433,6 +4433,10 @@ typedef enum { 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 31851cb..8651603 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, @@ -1272,6 +1277,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 bc1694a..aa04959 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20158,6 +20158,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. */ @@ -20207,6 +20211,73 @@ 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_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 7c6edf6..24826b8 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -627,4 +627,9 @@ LIBVIRT_1.1.0 { virDomainMigrateToURI3; } LIBVIRT_1.0.6; +LIBVIRT_1.1.1 { + global: + virDomainSnapshotDeleteByName; +}LIBVIRT_1.1.0; + # .... define new API here using predicted next version number .... -- 1.8.1.4

Let gendispatch.pl generate codes for both server side and client side. *src/remote/remote_driver.c: Add remoteDomainSnapshotDeleteByName into remote driver *src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME and its argument structs *src/remote_protocol-structs: edit it to match --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7a0c1f6..fe7b836 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6721,6 +6721,7 @@ static virDriver remote_driver = { .domainSnapshotIsCurrent = remoteDomainSnapshotIsCurrent, /* 0.9.13 */ .domainSnapshotHasMetadata = remoteDomainSnapshotHasMetadata, /* 0.9.13 */ .domainSnapshotDelete = remoteDomainSnapshotDelete, /* 0.8.0 */ + .domainSnapshotDeleteByName = remoteDomainSnapshotDeleteByName, /* 1.1.1 */ .domainQemuMonitorCommand = remoteDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = remoteDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = remoteDomainQemuAgentCommand, /* 0.10.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2e9dc1d..aa3266b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2475,6 +2475,12 @@ struct remote_domain_snapshot_delete_args { unsigned int flags; }; +struct remote_domain_snapshot_delete_by_name_args { + remote_nonnull_domain dom; + remote_string name; + unsigned int flags; +}; + struct remote_domain_open_console_args { remote_nonnull_domain dom; remote_string dev_name; @@ -4944,6 +4950,12 @@ enum remote_procedure { * @generate: none * @acl: domain:migrate */ - REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307 + REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, + + /** + * @generate: both + * @acl: domain:snapshot + */ + REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME = 308 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e38d24a..d9f5a68 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1904,6 +1904,11 @@ struct remote_domain_snapshot_delete_args { remote_nonnull_domain_snapshot snap; u_int flags; }; +struct remote_domain_snapshot_delete_by_name_args { + remote_nonnull_domain dom; + remote_string name; + u_int flags; +}; struct remote_domain_open_console_args { remote_nonnull_domain dom; remote_string dev_name; @@ -2601,4 +2606,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3_PARAMS = 305, REMOTE_PROC_DOMAIN_MIGRATE_FINISH3_PARAMS = 306, REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, + REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE_BY_NAME = 308, }; -- 1.8.1.4

Refactor orgininal qemuDomainSnapshotDelete() into a common function qemuDomainSnapshotDeleteImpl() which is used by both APIs SnapshotDelete() and SnapshotDeleteByName() --- src/qemu/qemu_driver.c | 103 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f51e766..e494a8a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13387,34 +13387,20 @@ 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 (virDomainSnapshotDeleteEnsureACL(snapshot->domain->conn, vm->def) < 0) - goto cleanup; - if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) - goto cleanup; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (!metadata_only) { if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) && @@ -13492,9 +13478,81 @@ endjob: vm = NULL; cleanup: + virObjectUnref(cfg); + return ret; +} + +static +int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn = snapshot->domain->conn; + virQEMUDriverPtr driver = 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 (virDomainSnapshotDeleteEnsureACL(conn, vm->def) < 0) + goto cleanup; + + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + ret = qemuDomainSnapshotDeleteImpl(driver, &vm, snap, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +qemuDomainSnapshotDeleteByName(virDomainPtr domain, + const char *name, + unsigned int flags) +{ + virConnectPtr conn = domain->conn; + virQEMUDriverPtr driver = 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, -1); + + if (!(vm = qemuDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainSnapshotDeleteByNameEnsureACL(conn, vm->def) < 0) + goto cleanup; + + if (name == NULL) { + 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; + } + + if (!(snap = qemuSnapObjFromName(vm, snapname))) + goto cleanup; + + ret = qemuDomainSnapshotDeleteImpl(driver, &vm, snap, flags); + +cleanup: if (vm) virObjectUnlock(vm); - virObjectUnref(cfg); return ret; } @@ -16012,6 +16070,7 @@ static virDriver qemuDriver = { .domainSnapshotHasMetadata = qemuDomainSnapshotHasMetadata, /* 0.9.13 */ .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */ + .domainSnapshotDeleteByName = qemuDomainSnapshotDeleteByName, /* 1.1.1 */ .domainQemuMonitorCommand = qemuDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = qemuDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */ -- 1.8.1.4

--- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/generator.py b/python/generator.py index da642eb..91f17de 100755 --- a/python/generator.py +++ b/python/generator.py @@ -1078,6 +1078,9 @@ def nameFixup(name, classe, type, file): elif name[0:12] == "virDomainGet": func = name[12:] func = string.lower(func[0:1]) + func[1:] + elif name[0:29] == "virDomainSnapshotDeleteByName": + func = name[9:] + func = string.lower(func[0:1]) + func[1:] elif name[0:29] == "virDomainSnapshotLookupByName": func = name[9:] func = string.lower(func[0:1]) + func[1:] -- 1.8.1.4

--- tools/virsh-snapshot.c | 65 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 7e75772..15bdcd7 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1966,37 +1966,82 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; unsigned int flags = 0; + bool snapshotname = false; + 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) { + snapshotname = true; + VSH_EXCLUSIVE_OPTIONS_VAR(current, snapshotname); + } 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 || name) { + if (virDomainSnapshotDeleteByName(dom, name, flags) == 0) + goto finished; + } else { + vshError(ctl, _("--snapshotname or --current is required")); + goto cleanup; + } + + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + goto error; + +fallback: 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

On Mon, Jul 01, 2013 at 07:47:06PM +0800, Guannan Ren wrote:
v1: https://www.redhat.com/archives/libvir-list/2013-June/msg00573.html
v1->v2: Remove VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT flag (name == NULL) means deleting current snapshot object Rebase work
Add a new snapshot API to delete snapshot object atomically
int virDomainSnapshotDeleteByName(virDomainPtr domain, const char *name, unsigned int flags);
The existing virDomainSnapshotDelete API accepts the snapshot object being deleted as an argument that would be not API atomic.
You can already do: ss = virDomainSnapshotLookupByName(dom, name); virDomainSnapshotDelete(ss, flags); and your patch is just enabling: virDomainSnapshotDeleteByName(dom, name, flags); I really don't see any reason to add this new API, as it does not offer any functionality that was not already available. NACK unless there's better justification of why this is needed. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/01/2013 07:51 PM, Daniel P. Berrange wrote:
On Mon, Jul 01, 2013 at 07:47:06PM +0800, Guannan Ren wrote:
v1: https://www.redhat.com/archives/libvir-list/2013-June/msg00573.html
v1->v2: Remove VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT flag (name == NULL) means deleting current snapshot object Rebase work
Add a new snapshot API to delete snapshot object atomically
int virDomainSnapshotDeleteByName(virDomainPtr domain, const char *name, unsigned int flags);
The existing virDomainSnapshotDelete API accepts the snapshot object being deleted as an argument that would be not API atomic. You can already do:
ss = virDomainSnapshotLookupByName(dom, name); virDomainSnapshotDelete(ss, flags);
Yeah, right now, virsh tool uses this format to delete a snapshot.
and your patch is just enabling:
virDomainSnapshotDeleteByName(dom, name, flags);
I really don't see any reason to add this new API, as it does not offer any functionality that was not already available.
NACK unless there's better justification of why this is needed.
Daniel
This patchset just try to follow the scenario of *LIstAll* APIs for atomic operation for SnapshotDelete. I don't know if this is necessary in practice. just in theory.

On Mon, Jul 01, 2013 at 08:43:16PM +0800, Guannan Ren wrote:
On 07/01/2013 07:51 PM, Daniel P. Berrange wrote:
On Mon, Jul 01, 2013 at 07:47:06PM +0800, Guannan Ren wrote:
v1: https://www.redhat.com/archives/libvir-list/2013-June/msg00573.html
v1->v2: Remove VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT flag (name == NULL) means deleting current snapshot object Rebase work
Add a new snapshot API to delete snapshot object atomically
int virDomainSnapshotDeleteByName(virDomainPtr domain, const char *name, unsigned int flags);
The existing virDomainSnapshotDelete API accepts the snapshot object being deleted as an argument that would be not API atomic. You can already do:
ss = virDomainSnapshotLookupByName(dom, name); virDomainSnapshotDelete(ss, flags);
Yeah, right now, virsh tool uses this format to delete a snapshot.
and your patch is just enabling:
virDomainSnapshotDeleteByName(dom, name, flags);
I really don't see any reason to add this new API, as it does not offer any functionality that was not already available.
NACK unless there's better justification of why this is needed.
Daniel
This patchset just try to follow the scenario of *LIstAll* APIs for atomic operation for SnapshotDelete. I don't know if this is necessary in practice. just in theory.
That is a completely different scenario. We had two APIs for each object eg virConnectListDomainIDs virConnectListDefinedDomains if you ran both those methods, at the same time as a VM moved from shutoff -> running, in between calling virConnectListDomainIDs and virConnectListDomainIDs, you would loose it entirely. This does not apply to the virDomainSnapshotDeleteByName method. So again NACK to this series. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Guannan Ren