[libvirt] [libvirt-glib] [PATCH v5 1/3] libvirt-gobject-domain: Add _fetch_snapshots

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 | 86 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 37 ++++++++++++++ libvirt-gobject/libvirt-gobject.sym | 2 + 3 files changed, 125 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index c6e30e5..adb5179 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -38,6 +38,8 @@ struct _GVirDomainPrivate { virDomainPtr handle; gchar uuid[VIR_UUID_STRING_BUFLEN]; + GHashTable *snapshots; + GMutex *lock; }; G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT); @@ -121,6 +123,11 @@ static void gvir_domain_finalize(GObject *object) g_debug("Finalize GVirDomain=%p", domain); + if (priv->snapshots) { + g_hash_table_unref(priv->snapshots); + } + g_mutex_free(priv->lock); + virDomainFree(priv->handle); G_OBJECT_CLASS(gvir_domain_parent_class)->finalize(object); @@ -237,6 +244,7 @@ static void gvir_domain_init(GVirDomain *domain) g_debug("Init GVirDomain=%p", domain); domain->priv = GVIR_DOMAIN_GET_PRIVATE(domain); + domain->priv->lock = g_mutex_new(); } typedef struct virDomain GVirDomainHandle; @@ -1514,3 +1522,81 @@ 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 + * @cancellable: (allow-none)(transfer-none): cancellation object + * @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, + GCancellable *cancellable, + GError **error) +{ + GVirDomainPrivate *priv; + virDomainSnapshotPtr *snapshots = NULL; + GVirDomainSnapshot *snap; + GHashTable *snap_table; + int n_snaps = 0; + int i; + gboolean ret = TRUE; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail((error == NULL) || (*error == NULL), FALSE); + + priv = dom->priv; + + snap_table = g_hash_table_new_full(g_str_hash, + g_str_equal, + NULL, + g_object_unref); + + + n_snaps = virDomainListAllSnapshots(priv->handle, &snapshots, list_flags); + + if (g_cancellable_set_error_if_cancelled(cancellable, error)) { + ret = FALSE; + goto cleanup; + } + + if (n_snaps < 0) { + gvir_set_error(error, GVIR_DOMAIN_ERROR, 0, + "Unable to fetch snapshots of %s", + gvir_domain_get_name(dom)); + ret = FALSE; + goto cleanup; + } + + for (i = 0; i < n_snaps; i ++) { + if (g_cancellable_set_error_if_cancelled(cancellable, error)) { + ret = FALSE; + goto cleanup; + } + snap = GVIR_DOMAIN_SNAPSHOT(g_object_new(GVIR_TYPE_DOMAIN_SNAPSHOT, + "handle", snapshots[i], + NULL)); + g_hash_table_insert(snap_table, + (gpointer)gvir_domain_snapshot_get_name(snap), + snap); + } + + + g_mutex_lock(priv->lock); + if (priv->snapshots != NULL) + g_hash_table_unref(priv->snapshots); + priv->snapshots = snap_table; + snap_table = NULL; + g_mutex_unlock(priv->lock); + +cleanup: + free(snapshots); + if (snap_table != NULL) + g_hash_table_unref (snap_table); + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 38d3458..8c1a8e5 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,10 @@ gvir_domain_create_snapshot(GVirDomain *dom, guint flags, GError **err); +gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, + guint list_flags, + GCancellable *cancellable, + 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.1

... 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 adb5179..8f48c2e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1600,3 +1600,24 @@ cleanup: g_hash_table_unref (snap_table); return ret; } + +/** + * 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 8c1a8e5..22870c1 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -367,6 +367,10 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, guint list_flags, GCancellable *cancellable, 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.1

On Wed, Jul 09, 2014 at 06:23:24PM +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 adb5179..8f48c2e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1600,3 +1600,24 @@ cleanup: g_hash_table_unref (snap_table); return ret; } + +/** + * 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;
You need to take the snapshot list mutex while generating the list copy, otherwise you could race with a call to gvir_domain_fetch_snapshot running in another thread and destroying the objects you are copying. Christophe

--- libvirt-gobject/libvirt-gobject-domain.c | 61 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 10 ++++++ libvirt-gobject/libvirt-gobject.sym | 2 ++ 3 files changed, 73 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8f48c2e..7fd5043 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1621,3 +1621,64 @@ GList *gvir_domain_get_snapshots(GVirDomain *dom) return snapshots; } + + + +static void _fetch_snapshots_async_thread(GTask *task, + gpointer source_object, + gpointer task_data, + GCancellable *cancellable) { + GError *error = NULL; + gboolean status; + + status = gvir_domain_fetch_snapshots(source_object, + GPOINTER_TO_UINT(task_data), + cancellable, + &error); + if (status) + g_task_return_boolean(task, TRUE); + else + g_task_return_error(task, error); +} + + +/** + * + * @dom: The domain + * @list_flags: bitwise-OR of #GVirDomainSnapshotListFlags + * @cancellable: (allow-none)(transfer-none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + */ +void gvir_domain_fetch_snapshots_async(GVirDomain *dom, + guint list_flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) { + GTask *task; + + g_return_if_fail(GVIR_IS_DOMAIN(dom)); + g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + + task = g_task_new(dom, cancellable, callback, user_data); + g_task_set_task_data(task, GUINT_TO_POINTER(list_flags), NULL); + g_task_run_in_thread(task, _fetch_snapshots_async_thread); + g_object_unref(task); +} + + +/** + * gvir_domain_fetch_snapshots_finish: + * @dom: a #GVirDomain + * @res: (transfer none): async method result + * + * Returns: TRUE on success, FALSE otherwise. + */ +gboolean gvir_domain_fetch_snapshots_finish(GVirDomain *dom, + GAsyncResult *res, + GError **error) { + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail(g_task_is_valid(res, dom), FALSE); + + return g_task_propagate_boolean(G_TASK(res), error); +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 22870c1..9846375 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -370,6 +370,16 @@ gboolean gvir_domain_fetch_snapshots(GVirDomain *dom, GList *gvir_domain_get_snapshots(GVirDomain *dom); +void gvir_domain_fetch_snapshots_async(GVirDomain *dom, + guint list_flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); + +gboolean gvir_domain_fetch_snapshots_finish(GVirDomain *dom, + GAsyncResult *res, + GError **error); + G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 28e547a..6aa8b86 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -237,6 +237,8 @@ LIBVIRT_GOBJECT_0.1.5 { LIBVIRT_GOBJECT_0.1.9 { global: gvir_domain_fetch_snapshots; + gvir_domain_fetch_snapshots_async; + gvir_domain_fetch_snapshots_finish; gvir_domain_get_snapshots; gvir_domain_snapshot_delete; gvir_domain_snapshot_list_flags_get_type; -- 2.0.1

On Wed, Jul 09, 2014 at 06:23:25PM +0200, Timm Bäder wrote:
--- libvirt-gobject/libvirt-gobject-domain.c | 61 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 10 ++++++ libvirt-gobject/libvirt-gobject.sym | 2 ++ 3 files changed, 73 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8f48c2e..7fd5043 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1621,3 +1621,64 @@ GList *gvir_domain_get_snapshots(GVirDomain *dom)
return snapshots; } + + + +static void _fetch_snapshots_async_thread(GTask *task, + gpointer source_object, + gpointer task_data, + GCancellable *cancellable) { + GError *error = NULL; + gboolean status; + + status = gvir_domain_fetch_snapshots(source_object, + GPOINTER_TO_UINT(task_data), + cancellable, + &error); + if (status) + g_task_return_boolean(task, TRUE); + else + g_task_return_error(task, error);
This is using GTask without updating the configure.ac glib version check to 2.36. I assume when you use this, valgrind don't complain after you added the g_object_unref(task) in gvir_domain_fetch_snapshots_async? Looks good otherwise, Christophe

On Wed, Jul 09, 2014 at 06:23:23PM +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 | 86 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 37 ++++++++++++++ libvirt-gobject/libvirt-gobject.sym | 2 + 3 files changed, 125 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index c6e30e5..adb5179 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -38,6 +38,8 @@ struct _GVirDomainPrivate { virDomainPtr handle; gchar uuid[VIR_UUID_STRING_BUFLEN]; + GHashTable *snapshots; + GMutex *lock; };
G_DEFINE_TYPE(GVirDomain, gvir_domain, G_TYPE_OBJECT); @@ -121,6 +123,11 @@ static void gvir_domain_finalize(GObject *object)
g_debug("Finalize GVirDomain=%p", domain);
+ if (priv->snapshots) { + g_hash_table_unref(priv->snapshots); + } + g_mutex_free(priv->lock); + virDomainFree(priv->handle);
G_OBJECT_CLASS(gvir_domain_parent_class)->finalize(object); @@ -237,6 +244,7 @@ static void gvir_domain_init(GVirDomain *domain) g_debug("Init GVirDomain=%p", domain);
domain->priv = GVIR_DOMAIN_GET_PRIVATE(domain); + domain->priv->lock = g_mutex_new(); }
typedef struct virDomain GVirDomainHandle; @@ -1514,3 +1522,81 @@ 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 + * @cancellable: (allow-none)(transfer-none): cancellation object + * @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, + GCancellable *cancellable, + GError **error) +{ + GVirDomainPrivate *priv; + virDomainSnapshotPtr *snapshots = NULL; + GVirDomainSnapshot *snap; + GHashTable *snap_table; + int n_snaps = 0; + int i; + gboolean ret = TRUE;
Small nit, in such situations, I tend to set the return value to FALSE (ie the error value), and only explicitly set it to TRUE when the operation is successful, this way it's harder to mistakenly report success instead of failure to the caller. This is what gvir_connection_fetch_pools and gvir_connection_fetch_domains do. Christophe
participants (2)
-
Christophe Fergeau
-
Timm Bäder