[libvirt] [libvirt-glib 1/4] gobject: don't try to use pool's volumes before a successful refresh
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL. --- libvirt-gobject/libvirt-gobject-storage-pool.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f50037..a4316e9 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -523,6 +523,8 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) if (priv->volumes != NULL) { volumes = g_hash_table_get_values(priv->volumes); g_list_foreach(volumes, gvir_storage_vol_ref, NULL); + } else { + g_warn_if_reached(); } g_mutex_unlock(priv->lock); @@ -548,9 +550,14 @@ GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, priv = pool->priv; g_mutex_lock(priv->lock); - volume = g_hash_table_lookup(priv->volumes, name); - if (volume) - g_object_ref(volume); + if (priv->volumes != NULL) { + volume = g_hash_table_lookup(priv->volumes, name); + if (volume) + g_object_ref(volume); + } else { + g_warn_if_reached(); + volume = NULL; + } g_mutex_unlock(priv->lock); return volume; @@ -604,7 +611,14 @@ GVirStorageVol *gvir_storage_pool_create_volume } g_mutex_lock(priv->lock); - g_hash_table_insert(priv->volumes, g_strdup(name), volume); + if (priv->volumes != NULL) { + g_hash_table_insert(priv->volumes, g_strdup(name), volume); + } else { + g_warn_if_reached(); + g_object_unref(G_OBJECT(volume)); + g_mutex_unlock(priv->lock); + return NULL; + } g_mutex_unlock(priv->lock); return g_object_ref(volume); -- 1.7.12.1
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 90 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 10 +++ libvirt-gobject/libvirt-gobject.sym | 6 ++ 3 files changed, 106 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index a4316e9..c5a98f2 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -837,3 +837,93 @@ gboolean gvir_storage_pool_start_finish(GVirStoragePool *pool, return TRUE; } + +/** + * gvir_storage_pool_stop: + * @pool: the storage pool to stop + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_stop (GVirStoragePool *pool, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (virStoragePoolDestroy(pool->priv->handle)) { + gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to stop storage pool"); + return FALSE; + } + + return TRUE; +} + +static void +gvir_storage_pool_stop_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ + GVirStoragePool *pool = GVIR_STORAGE_POOL(object); + GError *err = NULL; + + if (!gvir_storage_pool_stop(pool, &err)) { + g_simple_async_result_set_from_error(res, err); + g_error_free(err); + } +} + +/** + * gvir_storage_pool_stop_async: + * @pool: the storage pool to stop + * @cancellable: (allow-none)(transfer none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + */ +void gvir_storage_pool_stop_async (GVirStoragePool *pool, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GSimpleAsyncResult *res; + + g_return_if_fail(GVIR_IS_STORAGE_POOL(pool)); + g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + + res = g_simple_async_result_new(G_OBJECT(pool), + callback, + user_data, + gvir_storage_pool_stop_async); + g_simple_async_result_run_in_thread(res, + gvir_storage_pool_stop_helper, + G_PRIORITY_DEFAULT, + cancellable); + g_object_unref(res); +} + +/** + * gvir_storage_pool_stop_finish: + * @pool: the storage pool to stop + * @result: (transfer none): async method result + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_stop_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(pool), + gvir_storage_pool_stop_async), + FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), + err)) + return FALSE; + + return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.h b/libvirt-gobject/libvirt-gobject-storage-pool.h index 4589844..12c2d96 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.h +++ b/libvirt-gobject/libvirt-gobject-storage-pool.h @@ -133,6 +133,16 @@ gboolean gvir_storage_pool_start_finish(GVirStoragePool *pool, GAsyncResult *result, GError **err); +gboolean gvir_storage_pool_stop (GVirStoragePool *pool, + GError **err); +void gvir_storage_pool_stop_async (GVirStoragePool *pool, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean gvir_storage_pool_stop_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err); + G_END_DECLS #endif /* __LIBVIRT_GOBJECT_STORAGE_POOL_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 3a40a8a..9a4baee 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -204,4 +204,10 @@ LIBVIRT_GOBJECT_0.1.3 { gvir_domain_wakeup_finish; } LIBVIRT_GOBJECT_0.1.2; +LIBVIRT_GOBJECT_0.1.4 { + gvir_storage_pool_stop; + gvir_storage_pool_stop_async; + gvir_storage_pool_stop_finish; +} LIBVIRT_GOBJECT_0.1.3; + # .... define new API here using predicted next version number .... -- 1.7.12.1
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 90 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 10 +++ libvirt-gobject/libvirt-gobject.sym | 6 ++ 3 files changed, 106 insertions(+)
Looks good, ACK! -- Regards, Zeeshan Ali (Khattak) FSF member#5124
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 90 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 11 ++++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 104 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index c5a98f2..ac9b20f 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -734,6 +734,96 @@ gboolean gvir_storage_pool_build_finish(GVirStoragePool *pool, } /** + * gvir_storage_pool_undefine: + * @pool: the storage pool to undefine + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_undefine (GVirStoragePool *pool, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (virStoragePoolUndefine(pool->priv->handle)) { + gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to undefine storage pool"); + return FALSE; + } + + return TRUE; +} + +static void +gvir_storage_pool_undefine_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ + GVirStoragePool *pool = GVIR_STORAGE_POOL(object); + GError *err = NULL; + + if (!gvir_storage_pool_undefine(pool, &err)) { + g_simple_async_result_set_from_error(res, err); + g_error_free(err); + } +} + +/** + * gvir_storage_pool_undefine_async: + * @pool: the storage pool to undefine + * @cancellable: (allow-none)(transfer none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + */ +void gvir_storage_pool_undefine_async (GVirStoragePool *pool, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GSimpleAsyncResult *res; + + g_return_if_fail(GVIR_IS_STORAGE_POOL(pool)); + g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + + res = g_simple_async_result_new(G_OBJECT(pool), + callback, + user_data, + gvir_storage_pool_undefine_async); + g_simple_async_result_run_in_thread(res, + gvir_storage_pool_undefine_helper, + G_PRIORITY_DEFAULT, + cancellable); + g_object_unref(res); +} + +/** + * gvir_storage_pool_undefine_finish: + * @pool: the storage pool to undefine + * @result: (transfer none): async method result + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_undefine_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(pool), + gvir_storage_pool_undefine_async), + FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), + err)) + return FALSE; + + return TRUE; +} + +/** * gvir_storage_pool_start: * @pool: the storage pool to start * @flags: the flags diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.h b/libvirt-gobject/libvirt-gobject-storage-pool.h index 12c2d96..5fc9eeb 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.h +++ b/libvirt-gobject/libvirt-gobject-storage-pool.h @@ -121,6 +121,16 @@ gboolean gvir_storage_pool_build_finish(GVirStoragePool *pool, GAsyncResult *result, GError **err); +gboolean gvir_storage_pool_undefine (GVirStoragePool *pool, + GError **err); +void gvir_storage_pool_undefine_async (GVirStoragePool *pool, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean gvir_storage_pool_undefine_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err); + gboolean gvir_storage_pool_start (GVirStoragePool *pool, guint flags, GError **err); @@ -143,6 +153,7 @@ gboolean gvir_storage_pool_stop_finish(GVirStoragePool *pool, GAsyncResult *result, GError **err); + G_END_DECLS #endif /* __LIBVIRT_GOBJECT_STORAGE_POOL_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 9a4baee..3d394dc 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -208,6 +208,9 @@ LIBVIRT_GOBJECT_0.1.4 { gvir_storage_pool_stop; gvir_storage_pool_stop_async; gvir_storage_pool_stop_finish; + gvir_storage_pool_undefine; + gvir_storage_pool_undefine_async; + gvir_storage_pool_undefine_finish; } LIBVIRT_GOBJECT_0.1.3; # .... define new API here using predicted next version number .... -- 1.7.12.1
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 90 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 11 ++++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 104 insertions(+)
ACK. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 105 +++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 11 +++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 119 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index ac9b20f..feba3eb 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -1017,3 +1017,108 @@ gboolean gvir_storage_pool_stop_finish(GVirStoragePool *pool, return TRUE; } + +/** + * gvir_storage_pool_delete: + * @pool: the storage pool to delete + * @flags: the flags + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_delete (GVirStoragePool *pool, + guint flags, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (virStoragePoolDelete(pool->priv->handle, flags)) { + gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to delete storage pool"); + return FALSE; + } + + return TRUE; +} + +static void +gvir_storage_pool_delete_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ + GVirStoragePool *pool = GVIR_STORAGE_POOL(object); + StoragePoolBuildData *data; + GError *err = NULL; + + data = (StoragePoolBuildData *) g_object_get_data(G_OBJECT(res), + "StoragePoolBuildData"); + + if (!gvir_storage_pool_delete(pool, data->flags, &err)) { + g_simple_async_result_set_from_error(res, err); + g_error_free(err); + } + + g_slice_free (StoragePoolBuildData, data); +} + +/** + * gvir_storage_pool_delete_async: + * @pool: the storage pool to delete + * @flags: the flags + * @cancellable: (allow-none)(transfer none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + */ +void gvir_storage_pool_delete_async (GVirStoragePool *pool, + guint flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GSimpleAsyncResult *res; + StoragePoolBuildData *data; + + g_return_if_fail(GVIR_IS_STORAGE_POOL(pool)); + g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable)); + + data = g_slice_new0(StoragePoolBuildData); + data->flags = flags; + + res = g_simple_async_result_new(G_OBJECT(pool), + callback, + user_data, + gvir_storage_pool_delete_async); + g_object_set_data(G_OBJECT(res), "StoragePoolBuildData", data); + g_simple_async_result_run_in_thread(res, + gvir_storage_pool_delete_helper, + G_PRIORITY_DEFAULT, + cancellable); + g_object_unref(res); +} + +/** + * gvir_storage_pool_delete_finish: + * @pool: the storage pool to delete + * @result: (transfer none): async method result + * @err: return location for any #GError + * + * Return value: #True on success, #False otherwise. + */ +gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); + g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(pool), + gvir_storage_pool_delete_async), + FALSE); + g_return_val_if_fail(err == NULL || *err == NULL, FALSE); + + if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), + err)) + return FALSE; + + return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.h b/libvirt-gobject/libvirt-gobject-storage-pool.h index 5fc9eeb..1a1aae5 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.h +++ b/libvirt-gobject/libvirt-gobject-storage-pool.h @@ -153,6 +153,17 @@ gboolean gvir_storage_pool_stop_finish(GVirStoragePool *pool, GAsyncResult *result, GError **err); +gboolean gvir_storage_pool_delete (GVirStoragePool *pool, + guint flags, + GError **err); +void gvir_storage_pool_delete_async (GVirStoragePool *pool, + guint flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool, + GAsyncResult *result, + GError **err); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 3d394dc..a06d35f 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -205,6 +205,9 @@ LIBVIRT_GOBJECT_0.1.3 { } LIBVIRT_GOBJECT_0.1.2; LIBVIRT_GOBJECT_0.1.4 { + gvir_storage_pool_delete; + gvir_storage_pool_delete_async; + gvir_storage_pool_delete_finish; gvir_storage_pool_stop; gvir_storage_pool_stop_async; gvir_storage_pool_stop_finish; -- 1.7.12.1
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 105 +++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-storage-pool.h | 11 +++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 119 insertions(+)
ACK -- Regards, Zeeshan Ali (Khattak) FSF member#5124
Ping for these 4 patches? On Tue, Nov 06, 2012 at 01:45:12PM +0100, Christophe Fergeau wrote:
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL. --- libvirt-gobject/libvirt-gobject-storage-pool.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL.
Looks good. ACK! -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Tue, Nov 13, 2012 at 06:45:56PM +0100, Zeeshan Ali (Khattak) wrote:
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL.
Looks good. ACK!
Just this patch or the whole series? Christophe
On Wed, Nov 14, 2012 at 10:31 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Nov 13, 2012 at 06:45:56PM +0100, Zeeshan Ali (Khattak) wrote:
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL.
Looks good. ACK!
Just this patch or the whole series?
I've been reviewing/ACK'ing each patch separately so this one is only for this patch. :) Let me know if I missed a patch. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Wed, Nov 14, 2012 at 02:05:11PM +0100, Zeeshan Ali (Khattak) wrote:
On Wed, Nov 14, 2012 at 10:31 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Nov 13, 2012 at 06:45:56PM +0100, Zeeshan Ali (Khattak) wrote:
On Tue, Nov 6, 2012 at 1:45 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
gvir_storage_pool_refresh must be called and must be successful before trying to use gvir_storage_pool_get_volume, gvir_storage_pool_get_volumes and gvir_storage_pool_create_volume. As the storage pool refresh can fail for reasons external to libvirt/libvirt-gobject, the library user should check _refresh errors. This commit outputs runtime warnings when these functions are called and GVirObjectStoragePool::priv::volumes is NULL.
Looks good. ACK!
Just this patch or the whole series?
I've been reviewing/ACK'ing each patch separately so this one is only for this patch. :) Let me know if I missed a patch.
You missed the 3 other patches in this series then if I'm not mistaken ;) Christophe
participants (2)
-
Christophe Fergeau -
Zeeshan Ali (Khattak)