Re: [libvirt] [RFC]: Snapshot API

>> > /* Return the number of snapshots for this domain */ >> > int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags); >> >> Shouldn't this be called virDomainNumOfSnapshots to be consistent with >> the naming or other num-of functions? > >Gah. I struggled with this; I like the fact that all of the rest of them >start with virDomainSnapshot; this one doesn't fit the mold. I don't care >too much either way. virDomain*Snapshots for APIs that take a virDomainPtr seems consistent too. >> > /* Deactivate a snapshot - with no flags, the snapshot is not used >anymore, >> > * but also not removed. With a MERGE flag, it merges the snapshot >into >> > * the base image. With a DISCARD flag, it deletes the snapshot. >MERGE >> > * and DISCARD are mutually-exclusive. Note that this operation can >> > * generally only happen when the domain is shut down, though this is >> > * hypervisor-specific */ >> > typedef enum { >> > VIR_DOMAIN_SNAPSHOT_DEACTIVATE_MERGE, >> > VIR_DOMAIN_SNAPSHOT_DEACTIVATE_DISCARD, >> > } virDomainSnapshotDeactivate; >> > int virDomainSnapshotDeactivate(virDomainSnapshotPtr snapshot, >> > unsigned int flags); >> >> I'm not sure if virDomainSnapshotDeactivate is a good name. > >I agree it's not a great name. I didn't like Dan's original >proposal of "virDomainSnapshotDelete", though, since it doesn't >exactly seem to fit the situation either. Any more suggestions for >a name? Several things are not clear about this API to me too. :-) 1) Would discarding also discard all the snapshots depending on the discarded one. If so, what about a --force flag that you need to pass if the delete operation would delete other snapshots recursively?

2) Likewise, merging would invalidate all the other snapshots depending on the base image (and discard them). The same --force option then could apply as well.

However, I think there is a fundamental difference between the two cases; only one of them modifies the base image I'm not sure that something that modifies the base image rightfully belongs into the virDomainSnapshot* namespace. So my proposal would be /* Start using the base image again. If snapshot != NULL, merge the given snapshot in the base image again. */ virDomainDisableSnapshots(virDomainPtr domain, virDomainSnapshotPtr snapshot, int flags); /* Discard the given snapshot and, if --force is given, all that depend on it. Fail if it is active. */ virDomainSnapshotDiscard(virDomainSnapshotPtr snapshot, int flags);