On 04/02/2010 12:17 PM, Daniel Veillard wrote:
On Thu, Apr 01, 2010 at 06:07:22PM -0400, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- [...] diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7cb483e..a8a97e3 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1861,6 +1861,67 @@ int virDomainGetJobInfo(virDomainPtr dom, virDomainJobInfoPtr info); int virDomainAbortJob(virDomainPtr dom);
+/** + * virDomainSnapshot: + * + * a virDomainSnapshot is a private structure representing a snapshot of + * a domain. + */ +typedef struct _virDomainSnapshot virDomainSnapshot; + +/** + * virDomainSnapshotPtr: + * + * a virDomainSnapshotPtr is pointer to a virDomainSnapshot private structure, + * and is the type used to reference a domain snapshot in the API. + */ +typedef virDomainSnapshot *virDomainSnapshotPtr; + +/* Take a snapshot of the current VM state */ +virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + +/* Dump the XML of a snapshot */ +char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, + unsigned int flags); + +/* Return the number of snapshots for this domain */ +int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags); + +/* Get the names of all snapshots for this domain */ +int virDomainSnapshotListNames(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); + +/* Check whether a domain has a snapshot which is currently used */ +int virDomainHasCurrentSnapshot(virDomainPtr domain, unsigned flags); + +/* Get a handle to the current snapshot */ +virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain, + unsigned int flags); + +/* Set this snapshot as the current one for a domain, to be + * used next time domain is started */ +int virDomainCreateFromSnapshot(virDomainSnapshotPtr snapshot, + unsigned int flags); + +/* Deactivate a snapshot */ +typedef enum { + VIR_DOMAIN_SNAPSHOT_DELETE_MERGE = (1 << 0), + VIR_DOMAIN_SNAPSHOT_DELETE_MERGE_FORCE = (1 << 1), + VIR_DOMAIN_SNAPSHOT_DELETE_DISCARD = (1 << 2), + VIR_DOMAIN_SNAPSHOT_DELETE_DISCARD_FORCE = (1 << 3), +} virDomainSnapshotDeleteFlags;
Having a graphical reprentation of what the 4 options do on a snapshot arborescence would be nice but that's for documentation, in the meantime I would comment the enums. Also it seems to me the current set of options are expected to be mutually exclusive so why use bitfields ?
Actually, this is the one area where the new set of patches are changing semantics. All of these flags are going to be removed except for just one, which is "DISCARD_CHILDREN". I'll add documentation for that.
+int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags); + +int virDomainSnapshotFree(virDomainSnapshotPtr snapshot);
/* A generic callback definition. Specific events usually have a customization * with extra parameters */ [...] diff --git a/src/libvirt.c b/src/libvirt.c index 5247fe7..edb3084 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -683,6 +683,31 @@ virLibNWFilterError(virNWFilterPtr pool, virErrorNumber error, }
/** + * virLibDomainSnapshotError: + * @snapshot: the snapshot if available + * @error: the error number + * @info: extra information string + * + * Handle an error at the secret level
Comment is obviously an old cut and paste :-)
Oops, yeah, fixed now.
+ */ +static void +virLibDomainSnapshotError(virDomainSnapshotPtr snapshot, virErrorNumber error, const char *info) +{ + virConnectPtr conn = NULL; + const char *errmsg; + + if (error == VIR_ERR_OK) + return; + + errmsg = virErrorMsg(error, info); + if (error != VIR_ERR_INVALID_DOMAIN_SNAPSHOT) + conn = snapshot->domain->conn; + + virRaiseError(conn, NULL, NULL, VIR_FROM_DOMAIN_SNAPSHOT, error, VIR_ERR_ERROR, + errmsg, info, NULL, 0, 0, errmsg, info); +} + +/** * virRegisterNetworkDriver: * @driver: pointer to a network driver block * @@ -12136,3 +12161,426 @@ error: virDispatchError(conn); return -1; [...] +/** + * virDomainSnapshotListNames: + * @domain: a domain object + * @names: array to collect the list of names of snapshots + * @nameslen: size of @names + * @flags: unused flag parameters; callers should pass 0 + * + * Collect the list of domain snapshots for the given domain, and store + * their names in @names. + * + * Returns the number of domain snapshots found or -1 in case of error. + */
Maybe need to indicate the strategy for freeing the names list.
Added now.
[...]
+/** + * virDomainCreateFromSnapshot
wrong function name :-)
Yeah, I noticed that after posting the series. Fixed now.
+ * @snapshot: a domain snapshot object + * @flags: flag parameters + * + * Delete the snapshot. + * + * Returns 0 if the snapshot was successfully deleted, -1 on error. + */ +int +virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virConnectPtr conn; + + DEBUG("snapshot=%p, flags=%u", snapshot, flags); + + /* FIXME: make sure only one of the flags is set */
hence why use a bitfield allocation of the enums at all ?
Yeah, this was silliness. It's all gone now since we only have one flag.
+ virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(NULL, VIR_ERR_INVALID_DOMAIN_SNAPSHOT, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = snapshot->domain->conn; + + if (conn->driver->domainSnapshotDelete) { + int ret = conn->driver->domainSnapshotDelete(snapshot, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return -1; +} + +/** + * virDomainFree:
function name error :-)
Fixed.
+ * @snapshot: a domain snapshot object + * + * Free the domain snapshot object. The snapshot itself is not modified. + * The data structure is freed and should not be used thereafter. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSnapshotFree(virDomainSnapshotPtr snapshot) +{ + DEBUG("snapshot=%p", snapshot); + + virResetLastError(); + + if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { + virLibDomainSnapshotError(NULL, VIR_ERR_INVALID_DOMAIN_SNAPSHOT, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (virUnrefDomainSnapshot(snapshot) < 0) { + virDispatchError(NULL); + return -1; + } + return 0; +}
Except for those few things this looks fine to me,
ACK once fixed
Great, thanks. -- Chris Lalancette