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

Re: [libvirt] RFC: API additions for enhanced snapshot support



On Thu, Jun 16, 2011 at 6:41 AM, Eric Blake <eblake redhat com> wrote:

Robert, Fernando, Jagane: I have CCed you because we have discussed
snapshot APIs and I thought you'd be interested in Eric's work to
build them for libvirt.

Does each volume have its own independent snapshot namespace?  It may
be wise to document that snapshot namespaces are *not* independent
because storage backends may not be able to provide these semantics.

> /* Create a snapshot of a storage volume.  XML is optional, if non-NULL,
> it would be a new top-level element <volsnapshot> which is similar to
> the top-level <domainsnapshot> for virDomainSnapshotCreateXML, to
> specify name and description. Flags is 0 for now. */
> virStorageVolSnapshotPtr virDomainSnapshotCreateXML(virStorageVolPtr
> vol, const char *xml, unsigned int flags);
> [For qcow2, this would be implemented with 'qemu-img snapshot -c',
> similar to what virDomainSnapshotXML already does on inactive domains.
> Later, we can add LVM and btrfs support, or even allow full file copies
> of any file type.  Also in the future, we could enhance XML to take a
> new element that describes a relationship between the name of the
> original and of the snapshot, in the case where a new filename has to be
> created to complete the snapshot process.]

I found this formatting quite hard to read.  Something along these
lines would be easier to parse visually:

/* Create a snapshot of a storage volume.  XML is optional, if non-NULL,
 * it would be a new top-level element <volsnapshot> which is similar to
 * the top-level <domainsnapshot> for virDomainSnapshotCreateXML, to
 * specify name and description. Flags is 0 for now.
 */
virStorageVolSnapshotPtr virDomainSnapshotCreateXML(
    virStorageVolPtr vol, const char *xml, unsigned int flags);

...coments...

> /* Probe if vol has snapshots.  1 if true, 0 if false, -1 on error.
> Flags is 0 for now.  */
> int virStorageVolHasCurrentSnapshot(virStorageVolPtr vol, unsigned int
> flags);
> [For qcow2 images, snapshots can be contained within the same file and
> managed with qemu-img -l, but for other formats, this may mean that
> libvirt has to start managing externally saved data associated with the
> storage pool that associates snapshots with filenames.  In fact, even
> for qcow2 it might be useful to support creation of new files backed by
> the previous snapshot rather than cramming multiple snapshots in one
> file, so we may have a use for flags to filter out the presence of
> single-file vs. multiple-file snapshot setups.]

What is the "current snapshot"?

Is this function necessary when you already have
virStorageVolSnapshotListNames()?

> /* Return the most recent snapshot of a volume, if one exists, or NULL
> on failure.  Flags is 0 for now.  */
> virStorageVolSnapshotPtr virStorageVolSnapshotCurrent(virStorageVolPtr
> vol, unsigned int flags);

The name should include "revert".  This looks like a shortcut function
for virStorageVolRevertToSnapshot().

> /* Delete the storage associated with a snapshot (although the opaque
> snapshot object must still be independently freed).  If flags is 0, any
> child snapshots based off of this one are rebased onto the parent; if
> flags is VIR_STORAGE_VOL_SNAPSHOT_DELETE_CHILDREN , then any child
> snapshots based off of this one are also deleted.  */

What is the "opaque snapshot object"?

> int virStorageVolSnapshotDelete(virStorageVolSnapshotPtr snapshot,
> unsigned int flags);
> [For qcow2, this would involve qemu-img snapshot -d.  For
> multiple-file snapshots, this would also involve qemu-img commit.]

Is virStorageVolDelete() modified to also delete the volume's snapshots?

Stefan


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