[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