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

[Daniel P. Berrange]

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 :|