5:13 p.m.
On 03/30/2010 04:40 PM, Jiri Denemark wrote:
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?
Oops, yeah, you are right, I just forgot to change the comment. It now says:
/* Take a snapshot of the current VM state. */
...
> /* 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
Yeah, you are right, we can't just declare this. It's also an open question
to me what qemu does about merging (though bugs with loadvm/delvm are preventing
me from testing this at the moment).
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
The problem with declaring these semantics is that they are somewhat confusing, so
application developers will probably get them wrong. On the other hand it does
allow us to declare a semantic that all 3 hypervisors probably can support,
unlike the options below.
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.
The problem with this one is that it will be difficult for application writers
to write a single application that handles all of the hypervisors. Imagine trying
to write a GUI around this, and you'll see what I mean. If we were to add 3 new
API's like this, we would also probably want to add some data to the capabilities
XML for the particular hypervisors so you could grey out specific options in a GUI.
On the other hand, it does solve the problem with merging parents vs. children, and
also diffuses Paolo's concern about the "SnapshotDelete" API sometimes
deleting data
and sometimes modifying the base image.
And the third option I see would be distinguishing merge direction using new
flags.
This one is like the second option, in that you don't know which particular
directions a particular hypervisor supports. You'd still need to add
capabilities XML for each hypervisor.
I have to say that after thinking about these 3 options, I like the first
option the best. While it's slightly confusing, it is a good semantic. I'll
update the documentation for this.
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.
Yeah, that is true, which is why I'm waffling with it. In general it seems like
superflous
information, except in the one case of virtualbox duplicate names (ESX doesn't allow
duplicate names, I don't think, and qemu blows away duplicates). My inclination is
to declare the semantics of duplicate names to be undefined, since it doesn't seem
to be a very useful feature.
--
Chris Lalancette