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

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



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


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