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

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.