On Tue, Mar 23, 2010 at 03:23:36PM -0400, Chris Lalancette wrote: > On 03/23/2010 02:35 PM, Matthias Bolte wrote: > > 2010/3/23 Chris Lalancette <clalance@xxxxxxxxxx>: > >> 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. Actually, I don't think this makes sense even for QEMU. I think we just need an atomic API again virDomain that starts from a snapshot virDomainCreateAtSnapshot(virDomainPtr, virDomainSnapshotPtr snapshot); > >> /* 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? Agreed, deactivate isn't a good name. This really is about deleting a virDomainSnapshotPtr object instance. The flags control what happens to the data associated with the snapshot, either the data is thrown away (DISCARD), or the data is merged into the earlier snapshot object instance (MERGE). So, I still think virDomainSnapshotDelete is the right name here. > >> 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. Agreed, <parent> should just be a output only value. Only the name/description & compession really make sense. Everything else is automatically determined Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list