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

Re: [libvirt] [RFC]: Snapshot API



On 03/23/2010 02:35 PM, Matthias Bolte wrote:
> 2010/3/23 Chris Lalancette <clalance redhat com>:
>> Hello,
>>     As some of you know, I've been working on a new snapshot API.
>> This API is heavily based on DanB's earlier API proposed here:
>>
>> https://www.redhat.com/archives/libvir-list/2010-January/msg00626.html
>>
>> I've made some modifications to make it more libvirt-ish, and to add a
>> couple of features I think it lacked.  The full documentation is below.
>> Any comments are appreciated.
>>
>> /* NOTE: struct _virDomainSnapshot is a private structure, ala
>>  * struct _virDomain.
>>  */
>> typedef struct _virDomainSnapshot virDomainSnapshot;
>>
>> /* Take a snapshot of the current VM state */
>> virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain,
>>                                                const char *xmlDesc,
>>                                                unsigned int flags);
>>
>> /* Dump the XML of a snapshot */
>> /* NOTE: see below for proposed XML */
>> char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
>>                                  unsigned int flags);
>>
>> /* 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.

> 
>>
>> /* Get the names of all snapshots for this domain */
>> int virDomainListSnapshotNames(virDomainPtr domain, char **names, int nameslen,
>>                               unsigned int flags);
>>
>> /* Get a handle to a named snapshot */
>> virDomainSnapshotPtr virDomainSnapshotLookupByName(virDomainPtr domain,
>>                                                   const char *name,
>>                                                   unsigned int flags);
>>
>> /* Set this snapshot as the current one for a domain, to be
>>  * used next time domain is started */
>> int virDomainSnapshotActivate(virDomainSnapshotPtr snapshot,
>>                              unsigned int flags);
> 
> This delayed semantic cannot be implemented for ESX. ESX can revert to
> a snapshot immediately only. I think the same holds true for
> VirtualBox.
> 
> Maybe I misunterstand this. What should happen if you call activate on
> a running domain?

Ah, I see what you mean.  I understood the intent from DanB's initial
email differently.  The way I understood it is to make this snapshot
the current snapshot for a shut-off domain, so that when it is started,
it is started with this snapshot.  That way, you could choose from among
a tree of snapshots as to which one you want to boot right now.  If you
ran this against a running guest, it would throw an error.  Maybe DanB
can tell us which of these interpretations is correct, though.

Your point (about reverting to a state before the snapshot) is covered
by deactivate (discussed below).

> 
>> /* 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?

> 
> Why would I deactivate a snapshot, but not merge/discard it? What's
> the use case for this?

The use case I was thinking about mostly involved debugging (and comes
from my experience debugging qemu guests).  Say you took a snapshot of
a guest, and after running for a while discovered some sort of bug in
the guest software.  You might then keep the memory image of that snapshot
around so that you could run "crash" or "gdb" against it to extract some data.
It's kind of an esoteric use-case, I'll admit, but I have found it
somewhat useful in the past.

> 
>> int virDomainSnapshotFree(virDomainSnapshotPtr snapshot);
>>
>> NOTE: During snapshot creation, *none* of the fields are required.  That is,
>> you can call virDomainSnapshotCreateXML() with an XML of "<domainsnapshot/>".
>> In this case, the individual driver will make up a <name> for you, the
>> <creationdate> will be set to the current time+date, <description> will be
>> empty, <state> will be "off", <compression> will be empty, and <parent> will
>> be empty.  If you do want to specify some fields during
>> virDomainSnapshotCreateXML(), note that the only ones that are settable are
>> <name>, <description>, <compression>, and <parent>; the rest are ignored,
> 
> How can <parent> be settable? If I have snapshots A and B
> 
>   A -> B -> current state
> 
> and I create a new snapshot C, then B will be the parent of C.
> 
>   A -> B -> C -> current state
> 
> If I create another snapshot D now and specify A to be its parent,
> what's supposed to happen then?

You are right, that doesn't make that much sense.  I have to admit that
the tree structure is the part I thought about least, so I'll take that
part back.  <parent> is just going to be an informational field about
which snapshot was current (if any) when this one was created.

-- 
Chris Lalancette


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