On Thu, Apr 01, 2010 at 06:07:22PM -0400, Chris Lalancette wrote:
> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
> ---
[...]
> 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 ?
> +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 :-)
> + */
> +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.
[...]
> +/**
> + * virDomainCreateFromSnapshot
wrong function name :-)
> + * @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 ?
> + 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 :-)
> + * @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
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list