[libvirt] [libvirt-glib] [PATCH 0/2] Add API to fetch the snapshots of a GVirDomain

The following two patches add gvir_domain_fetch_snapshots which uses virDomainListAllSnapshots to fetch snapshots from the virDomain (using the given GVirDomainSnapshotListFlags) and gvir_domain_get_snapshots which returns a GList of containing all of the snapshots last fetched. Timm Bäder (2): libvirt-gobject-domain: Add _fetch_snapshots libvirt-gobject-domain: Add _get_snapshots libvirt-gobject/libvirt-gobject-domain.c | 80 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 40 ++++++++++++++++ libvirt-gobject/libvirt-gobject.sym | 3 ++ 3 files changed, 123 insertions(+) -- 2.0.0

This function can be used to fetch the snapshots of a domain (according to the given GVirDomainSnapshotListFlags) and save them in a domain-internal GHashTable. A function to access them from outside will be added in a later patch. --- libvirt-gobject/libvirt-gobject-domain.c | 59 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 36 +++++++++++++++++++ libvirt-gobject/libvirt-gobject.sym | 2 ++ 3 files changed, 97 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index c6e30e5..f6b5837 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -38,6 +38,7 @@ struct _GVirDomainPrivate { virDomainPtr handle; gchar uuid[VIR_UUID_STRING_BUFLEN]; + GHashTable *snapshots; }; G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT); @@ -121,6 +122,8 @@ static void gvir_domain_finalize(GObject *object) g_debug("Finalize GVirDomain=%p", domain); + g_hash_table_unref (priv->snapshots); + virDomainFree(priv->handle); G_OBJECT_CLASS(gvir_domain_parent_class)->finalize(object); @@ -1514,3 +1517,59 @@ gvir_domain_create_snapshot(GVirDomain *dom, g_free(custom_xml); return dom_snapshot; } + + + +/** + * gvir_domain_fetch_snapshots: + * @dom: The domain + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags + * @error: (allow-none): Place-holder for error or NULL + * + * Returns: TRUE on success, FALSE otherwise. + */ +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, + guint list_flags, + GError **error) +{ + GVirDomainPrivate *priv; + virDomainSnapshotPtr *snapshots = NULL; + GVirDomainSnapshot *snap; + int n_snaps = 0; + int i; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE); + + priv = dom->priv; + + if (priv->snapshots != NULL) { + g_hash_table_destroy (priv->snapshots); + } + + priv->snapshots = g_hash_table_new_full(g_str_hash, + g_str_equal, + g_free, + g_object_unref); + + + n_snaps = virDomainListAllSnapshots(priv->handle, &snapshots, list_flags); + + if (n_snaps < 0) { + gvir_set_error(error, GVIR_DOMAIN_ERROR, 0, + "Unable to fetch snapshots of %s", + gvir_domain_get_name (dom)); + return FALSE; + } + + for (i = 0; i < n_snaps; i ++) { + snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT, + "handle", snapshots[i], + NULL)); + g_hash_table_insert(priv->snapshots, + (gpointer)gvir_domain_snapshot_get_name(snap), + snap); + } + g_free(snapshots); + return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 38d3458..fb33e2b 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -183,6 +183,39 @@ typedef enum { GVIR_DOMAIN_REBOOT_GUEST_AGENT = VIR_DOMAIN_REBOOT_GUEST_AGENT, } GVirDomainRebootFlags; +/** + * GVirDomainSnapshotListFlags: + * @GVIR_DOMAIN_SNAPSHOT_LIST_ALL: List all snapshots + * @GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS: List all descendants, not just + * children, when listing a snapshot. + * For historical reasons, groups do not use contiguous bits. + * @GVIR_DOMAIN_SNAPSHOT_LIST_ROOTS: Filter by snapshots with no parents, when listing a domain + * @GVIR_DOMAIN_SNAPSHOT_LIST_METADATA: Filter by snapshots which have metadata + * @GVIR_DOMAIN_SNAPSHOT_LIST_LEAVES: Filter by snapshots with no children + * @GVIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES: Filter by snapshots that have children + * @GVIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA: Filter by snapshots with no metadata + * @GVIR_DOMAIN_SNAPSHOT_LIST_INACTIVE: Filter by snapshots taken while guest was shut off + * @GVIR_DOMAIN_SNAPSHOT_LIST_ACTIVE: Filter by snapshots taken while guest was active, and with memory state + * @GVIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY: Filter by snapshots taken while guest was active, but without memory state + * @GVIR_DOMAIN_SNAPSHOT_LIST_INTERNAL: Filter by snapshots stored internal to disk images + * @GVIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL: Filter by snapshots that use files external to disk images + */ +typedef enum { + GVIR_DOMAIN_SNAPSHOT_LIST_ALL = 0, + GVIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, + GVIR_DOMAIN_SNAPSHOT_LIST_ROOTS = VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, + GVIR_DOMAIN_SNAPSHOT_LIST_METADATA = VIR_DOMAIN_SNAPSHOT_LIST_METADATA, + GVIR_DOMAIN_SNAPSHOT_LIST_LEAVES = VIR_DOMAIN_SNAPSHOT_LIST_LEAVES, + GVIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES = VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, + GVIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA = VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, + GVIR_DOMAIN_SNAPSHOT_LIST_INACTIVE = VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE, + GVIR_DOMAIN_SNAPSHOT_LIST_ACTIVE = VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE, + GVIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY = VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY, + GVIR_DOMAIN_SNAPSHOT_LIST_INTERNAL = VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL, + GVIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL = VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL +} GVirDomainSnapshotListFlags; + + typedef struct _GVirDomainInfo GVirDomainInfo; struct _GVirDomainInfo { @@ -330,6 +363,9 @@ gvir_domain_create_snapshot(GVirDomain *dom, guint flags, GError **err); +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, + guint flags, + GError **error); G_END_DECLS #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index b781cc6..781310f 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -236,7 +236,9 @@ LIBVIRT_GOBJECT_0.1.5 { LIBVIRT_GOBJECT_0.1.9 { global: + gvir_domain_fetch_snapshots; gvir_domain_snapshot_delete; + gvir_domain_snapshot_list_flags_get_type; } LIBVIRT_GOBJECT_0.1.5; # .... define new API here using predicted next version number .... -- 2.0.0

[sorry, this probably breaks threading as I mistakenly deleted the original email] On Mon, Jun 23, 2014 at 05:29:10PM +0200, Timm Bäder wrote:
This function can be used to fetch the snapshots of a domain (according to the given GVirDomainSnapshotListFlags) and save them in a domain-internal GHashTable. A function to access them from outside will be added in a later patch. --- libvirt-gobject/libvirt-gobject-domain.c | 59 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 36 +++++++++++++++++++ libvirt-gobject/libvirt-gobject.sym | 2 ++ 3 files changed, 97 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index c6e30e5..f6b5837 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -38,6 +38,7 @@ struct _GVirDomainPrivate { virDomainPtr handle; gchar uuid[VIR_UUID_STRING_BUFLEN]; + GHashTable *snapshots; };
G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT); @@ -121,6 +122,8 @@ static void gvir_domain_finalize(GObject *object)
g_debug("Finalize GVirDomain=%p", domain);
+ g_hash_table_unref (priv->snapshots); +
You need to check if it's NULL before trying to unref it.
virDomainFree(priv->handle);
G_OBJECT_CLASS(gvir_domain_parent_class)->finalize(object); @@ -1514,3 +1517,59 @@ gvir_domain_create_snapshot(GVirDomain *dom, g_free(custom_xml); return dom_snapshot; } + + + +/** + * gvir_domain_fetch_snapshots: + * @dom: The domain + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags + * @error: (allow-none): Place-holder for error or NULL + * + * Returns: TRUE on success, FALSE otherwise. + */ +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, + guint list_flags, + GError **error) +{ + GVirDomainPrivate *priv; + virDomainSnapshotPtr *snapshots = NULL; + GVirDomainSnapshot *snap; + int n_snaps = 0; + int i; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE); + + priv = dom->priv; + + if (priv->snapshots != NULL) { + g_hash_table_destroy (priv->snapshots);
g_hash_table_unref() would be more consistent with what is done in _finalize.
+ } + + priv->snapshots = g_hash_table_new_full(g_str_hash, + g_str_equal, + g_free,
The g_free() here seems wrong (more details below)
+ g_object_unref); + + + n_snaps = virDomainListAllSnapshots(priv->handle, &snapshots, list_flags); + + if (n_snaps < 0) { + gvir_set_error(error, GVIR_DOMAIN_ERROR, 0, + "Unable to fetch snapshots of %s", + gvir_domain_get_name (dom)); + return FALSE; + } + + for (i = 0; i < n_snaps; i ++) { + snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT, + "handle", snapshots[i], + NULL)); + g_hash_table_insert(priv->snapshots, + (gpointer)gvir_domain_snapshot_get_name(snap), + snap);
gvir_domain_snapshot_get_name() does not return a copy of the string, but just a const char * you do not own, so the g_free you passed to g_hash_table_new_full to destroy the key is not correct.
+ } + g_free(snapshots);
snapshots was allocated by libvirt, so it should be freed using free(), not g_free(). Most of the time, g_free() ends up calling free(), but this can be changed (by the library user) with g_mem_set_vtable(). Patch looks good otherwise. As calling into libvirt can be costly (we could be talking to a remote libvirt), do you have plans to implement an async() variant of this as well? Christophe

... which returns a GList of GVirDomainSnapshots, i.e. without any tree structure or other relationship between the snapshots. --- libvirt-gobject/libvirt-gobject-domain.c | 21 +++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 26 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index f6b5837..2da99df 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1573,3 +1573,24 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, g_free(snapshots); return TRUE; } + +/** + * gvir_domain_get_snapshots: + * @dom: The domain + * Returns: (element-type LibvirtGObject.DomainSnapshot) (transfer full): A list of + * all the snapshots available for the given domain. The returned list + * should be freed with g_list_free(), after its elements have been unreffed + * with g_object_unref(). + */ +GList *gvir_domain_get_snapshots(GVirDomain *dom) +{ + GList *snapshots = NULL; + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), NULL); + + if (dom->priv->snapshots != NULL) { + snapshots = g_hash_table_get_values(dom->priv->snapshots); + g_list_foreach(snapshots, (GFunc)g_object_ref, NULL); + } + + return snapshots; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index fb33e2b..acea7c2 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -366,6 +366,10 @@ gvir_domain_create_snapshot(GVirDomain *dom, gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, guint flags, GError **error); + +GList *gvir_domain_get_snapshots(GVirDomain *dom); + + G_END_DECLS #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 781310f..28e547a 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -237,6 +237,7 @@ LIBVIRT_GOBJECT_0.1.5 { LIBVIRT_GOBJECT_0.1.9 { global: gvir_domain_fetch_snapshots; + gvir_domain_get_snapshots; gvir_domain_snapshot_delete; gvir_domain_snapshot_list_flags_get_type; } LIBVIRT_GOBJECT_0.1.5; -- 2.0.0

On Mon, Jun 23, 2014 at 05:29:11PM +0200, Timm Bäder wrote:
... which returns a GList of GVirDomainSnapshots, i.e. without any tree structure or other relationship between the snapshots. --- libvirt-gobject/libvirt-gobject-domain.c | 21 +++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 26 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index f6b5837..2da99df 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1573,3 +1573,24 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, g_free(snapshots); return TRUE; } + +/** + * gvir_domain_get_snapshots: + * @dom: The domain + * Returns: (element-type LibvirtGObject.DomainSnapshot) (transfer full): A list of
This line is a few characters too long, ACK to the patch otherwise. Christophe
participants (2)
-
Christophe Fergeau
-
Timm Bäder