3:52 a.m.
> > /* 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.
virDomain*Snapshots for APIs that take a virDomainPtr seems consistent too.
> > /* 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?
Several things are not clear about this API to me too. :-)
1) Would discarding also discard all the snapshots depending on the
discarded one. If so, what about a --force flag that you need to pass
if the delete operation would delete other snapshots recursively?
2) Likewise, merging would invalidate all the other snapshots depending
on the base image (and discard them). The same --force option then
could apply as well.
However, I think there is a fundamental difference between the two
cases; only one of them modifies the base image I'm not sure that
something that modifies the base image rightfully belongs into the
virDomainSnapshot* namespace. So my proposal would be
/* Start using the base image again. If snapshot != NULL, merge
the given snapshot in the base image again. */
virDomainDisableSnapshots(virDomainPtr domain,
virDomainSnapshotPtr snapshot, int flags);
/* Discard the given snapshot and, if --force is given, all that
depend on it. Fail if it is active. */
virDomainSnapshotDiscard(virDomainSnapshotPtr snapshot, int flags);
> 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.
If your guest crashes in production (not just a kernel crash, even an
application crash) and you want to restart it from the previous existing
snapshot, you probably still want to keep the other snapshot around so
that you can copy it to another machine, start it there, and do
post-mortem debugging.
In this case, however, you would likely activate another snapshot rather
than deactivating the one that crashed. See below for another
suggestion about how to handle this.
> > 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/>".
Maybe it's worthwhile having a simple virDomainSnapshotCreate() API for
this? (Or at least making the xml argument optional in virsh).
> 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.
If discarding a snapshot also discards the children, it would definitely
make sense to be able to specify the parent.
If you do not want to allow setting the parent, you can also add a flag
--inactive to virsh snapshot-create that would create the snapshot
without making it active. Then you would make <parent> an informational
field about which snapshot was *active* when the new one was created.
In the end the virsh commands would be
virsh snapshot-create <dom> [<xmlfile>] [--compress] [--inactive]
virsh snapshot-list <dom>
virsh snapshot-dumpxml <dom> <name>
virsh snapshot-activate <dom> <snapshotname>
virsh snapshot-discard <dom> <snapshotname> [--force]
virsh snapshot-disable <dom> [<snapshotname>]
What do you think?
Paolo