[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [RFC]: Snapshot API v3



Hi.

...
> /* NOTE: struct _virDomainSnapshot is a private structure, ala
>  * struct _virDomain.
>  */
> typedef struct _virDomainSnapshot virDomainSnapshot;
> 
> /* Take a snapshot of the current VM state.  Throws an error if
>  * the VM is not currently running */
> virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain,
>                                                 const char *xmlDesc,
>                                                 unsigned int flags);

This is probably a leftover from previous versions, but... why do we restrict
this API only for running VMs?

...
> /* Delete 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 parent snapshot (or the base image, if there is no parent snapshot).
>  * Note that if other snapshots would be discarded because of this
>  * MERGE action, this operation will fail.  If that is really what is intended,
>  * use MERGE_FORCE.
>  *
>  * With a DISCARD flag, it deletes the snapshot.  Note that if children snapshots
>  * would be discarded because of this delete action, this operation will
>  * fail.  If this is really what is intended, use DISCARD_FORCE.
>  *
>  * MERGE, MERGE_FORCE, DISCARD, and DISCARD_FORCE are mutually-exclusive.
>  *
>  * Note that this operation can happen when the domain is running or shut
>  * down, though this is hypervisor specific */
> typedef enum {
>     VIR_DOMAIN_SNAPSHOT_DELETE_MERGE,
>     VIR_DOMAIN_SNAPSHOT_DELETE_MERGE_FORCE,
>     VIR_DOMAIN_SNAPSHOT_DELETE_DISCARD,
>     VIR_DOMAIN_SNAPSHOT_DELETE_DISCARD_FORCE,
> } virDomainSnapshotDelete;

Merging a snapshot into its parent is probably not the best semantics for
MERGE flag as hypervisors differ in the way merging is implemented. As you
also mention below, VirtualBox merges into all children instead of a parent.
We should allow for both cases. However it influences several things. Firstly,
it makes MERGE_FORCE unnecessary for child merging, which is not a big deal as
it can just be treated in the same way as MERGE. Secondly, it makes a huge
difference when deleting a snapshot with no child. In one case it results in
changes being merged and in other case it results changes begin dropped.

One option is to refine the semantics to something like:

- MERGE: merge changes into other snapshot(s) and fail if it would require any
  snapshot to be discarded (even the one which was supposed to be merged)
- MERGE_FORCE: really merge even discarding other snapshots but fail if the
  snapshot itself would actually be discarded
- DISCARD: discard the snapshot and fail if other snapshots would be discarded
- DISCARD_FORCE: discard, no matter what

Another option would be to introduce several different APIs for merging into
children, merging into parent, and discarding. That would allow drivers to
implement only supported methods. Even all of them for a very flexible
hypervisor.

And the third option I see would be distinguishing merge direction using new
flags.

Personally, I like the second option best as it provides the easiest way for
application to detect unsupported behavior.

...
> Possible issues:
> 1)  I don't see a way to support "managed" save/restore and snapshotting with
> this API.  I think we'll have to have a separate API for managed save/restore.
> 2)  What is the semantic for deleting snapshots from a running domain?
> Virtualbox seems to not allow you to manipulate snapshots while the domain is
> running.  Qemu does allow this, but it's currently unclear what the exact
> semantics are.  VMware seems to allow manipulation of snapshots while the
> domain is running.
> 3)  Do we need a snapshot UUID?  Virtualbox allows you to have multiple snapshots
> with the same name, differentiated by UUID.  Confusingly, they also have a
> "FindByName" method that returns the first depth-first search snapshot that matches
> a given name.  For qemu, if you specify the same name twice it overwrites the previous
> one with the new one.  I don't know what ESX does here.

Libvirt uses/generates UUIDs for almost everything (networks, vms, ...) so it
might be more consistent to have UUID in snapshot as well.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]