[libvirt] [libvirt-glib] gobject: Also delete storage volume from its pool's list

[Zeeshan Ali (Khattak)]

On Wed, Nov 21, 2012 at 12:49 PM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> On Tue, Nov 20, 2012 at 08:51:23PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>
>>
>> Without this patch, storage pool still lists the volume even after it is
>> deleted.
>>
>> Related Boxes bug: https://bugzilla.gnome.org/show_bug.cgi?id=688724
>> ---
>>  libvirt-gobject/libvirt-gobject-storage-pool.c | 23 +++++++++++++++++++++++
>>  libvirt-gobject/libvirt-gobject-storage-vol.c  |  4 ++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> index 8f579a1..df7e77c 100644
>> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
>> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
>> @@ -29,6 +29,7 @@
>>  #include "libvirt-glib/libvirt-glib.h"
>>  #include "libvirt-gobject/libvirt-gobject.h"
>>  #include "libvirt-gobject-compat.h"
>> +#include "libvirt-gobject-storage-pool-private.h"
>
> libvirt-gobject-storage-pool-private.h is missing from this diff
>
>>
>>  #define GVIR_STORAGE_POOL_GET_PRIVATE(obj)                         \
>>          (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_POOL, GVirStoragePoolPrivate))
>> @@ -1129,3 +1130,25 @@ gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool,
>>
>>      return TRUE;
>>  }
>> +
>> +G_GNUC_INTERNAL gboolean gvir_storage_pool_delete_vol(GVirStoragePool *pool,
>> +                                                      GVirStorageVol *volume)
>> +{
>> +    GVirStoragePoolPrivate *priv;
>> +    gboolean ret = FALSE;
>> +
>> +    g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE);
>> +    g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume), FALSE);
>> +
>> +    priv = pool->priv;
>> +    g_mutex_lock(priv->lock);
>> +    if (priv->volumes != NULL) {
>> +        const gchar *name = gvir_storage_vol_get_name(volume);
>> +        ret = g_hash_table_remove(priv->volumes, name);
>> +    } else {
>> +        g_warn_if_reached();
>> +    }
>> +    g_mutex_unlock(priv->lock);
>> +
>> +    return ret;
>> +}
>> diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c
>> index c7ebb45..4352d30 100644
>> --- a/libvirt-gobject/libvirt-gobject-storage-vol.c
>> +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
>> @@ -29,6 +29,7 @@
>>  #include "libvirt-glib/libvirt-glib.h"
>>  #include "libvirt-gobject/libvirt-gobject.h"
>>  #include "libvirt-gobject-compat.h"
>> +#include "libvirt-gobject-storage-pool-private.h"
>>
>>  #define GVIR_STORAGE_VOL_GET_PRIVATE(obj)                         \
>>          (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_VOL, GVirStorageVolPrivate))
>> @@ -309,6 +310,9 @@ gboolean gvir_storage_vol_delete(GVirStorageVol *vol,
>>      g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE);
>>      g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
>>
>> +    if (!gvir_storage_pool_delete_vol(vol->priv->pool, vol))
>> +        return FALSE;
>> +
>
> I'm not sure this should be fatal. Even if we report an error when this
> occurs, we probably should try to call virStorageVolDelete nonetheless.
> You need to fill 'err' if you return FALSE here.

On second thought gvir_storage_pool_delete_vol() doesn't need to
return FALSE at all.



-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124