Re: [libvirt] [RFC]: Snapshot API

On 03/23/2010 02:35 PM, Matthias Bolte wrote: > 2010/3/23 Chris Lalancette <clalance(a)redhat.com&gt;: >> 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.

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

>> 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.