Re: [libvirt] [libvirt-glib PATCHv5 1/7] gobject: Simplify gvir_connection_list*() implementations

Make use of virConnectListAll* functions to avoid making 4 calls and hence avoid race conditions and complicated code. --- libvirt-gobject/libvirt-gobject-connection.c | 203 ++++----------------------- 1 file changed, 28 insertions(+), 175 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index cf073a5..e088427 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -680,48 +680,6 @@ void gvir_connection_close(GVirConnection *conn) g_signal_emit(conn, signals[VIR_CONNECTION_CLOSED], 0); } -typedef gint (* CountFunction) (virConnectPtr vconn); -typedef gint (* ListFunction) (virConnectPtr vconn, gchar **lst, gint max); - -static gchar ** fetch_list(virConnectPtr vconn, - const char *name, - CountFunction count_func, - ListFunction list_func, - GCancellable *cancellable, - gint *length, - GError **err) -{ - gchar **lst = NULL; - gint n = 0; - - if ((n = count_func(vconn)) < 0) { - gvir_set_error(err, GVIR_CONNECTION_ERROR, - 0, - _("Unable to count %s"), name); - goto error; - } - - if (n) { - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto error; - - lst = g_new0(gchar *, n); - if ((n = list_func(vconn, lst, n)) < 0) { - gvir_set_error(err, GVIR_CONNECTION_ERROR, - 0, - _("Unable to list %s %d"), name, n); - goto error; - } - } - - *length = n; - return lst; - -error: - g_free(lst); - return NULL; -} - /** * gvir_connection_fetch_domains: * @conn: a #GVirConnection @@ -733,14 +691,11 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, { GVirConnectionPrivate *priv; GHashTable *doms; - gchar **inactive = NULL; - gint ninactive = 0; - gint *active = NULL; - gint nactive = 0; + virDomainPtr *domains = NULL; + gint ndomains = 0; gboolean ret = FALSE; gint i; virConnectPtr vconn = NULL; - GError *lerr = NULL; g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE); g_return_val_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable), @@ -761,81 +716,28 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, virConnectRef(vconn); g_mutex_unlock(priv->lock); - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto cleanup; - - if ((nactive = virConnectNumOfDomains(vconn)) < 0) { - gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, - 0, - _("Unable to count domains")); + ndomains = virConnectListAllDomains(vconn, &domains, 0); + if (ndomains < 0) { + gvir_set_error(err, GVIR_CONNECTION_ERROR, + 0, + _("Failed to fetch list of domains")); goto cleanup; } - if (nactive) { - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto cleanup; - - active = g_new(gint, nactive); - if ((nactive = virConnectListDomains(vconn, active, nactive)) < 0) { - gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, - 0, - _("Unable to list domains")); - goto cleanup; - } - } if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; - inactive = fetch_list(vconn, - "Domains", - virConnectNumOfDefinedDomains, - virConnectListDefinedDomains, - cancellable, - &ninactive, - &lerr); - if (lerr) { - g_propagate_error(err, lerr); - lerr = NULL; - goto cleanup; - } - doms = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, g_object_unref); - for (i = 0 ; i < nactive ; i++) { - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto cleanup; - - virDomainPtr vdom = virDomainLookupByID(vconn, active[i]); + for (i = 0 ; i < ndomains; i++) { GVirDomain *dom; - if (!vdom) - continue; dom = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN, - "handle", vdom, + "handle", domains[i], NULL)); - virDomainFree(vdom); - - g_hash_table_insert(doms, - (gpointer)gvir_domain_get_uuid(dom), - dom); - } - - for (i = 0 ; i < ninactive ; i++) { - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto cleanup; - - virDomainPtr vdom = virDomainLookupByName(vconn, inactive[i]); - GVirDomain *dom; - if (!vdom) - continue; - - dom = GVIR_DOMAIN(g_object_new(GVIR_TYPE_DOMAIN, - "handle", vdom, - NULL)); - virDomainFree(vdom); g_hash_table_insert(doms, (gpointer)gvir_domain_get_uuid(dom), @@ -852,10 +754,11 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, ret = TRUE; cleanup: - g_free(active); - for (i = 0 ; i < ninactive ; i++) - g_free(inactive[i]); - g_free(inactive); + if (ndomains > 0) { + for (i = 0 ; i < ndomains; i++) + virDomainFree(domains[i]); + free(domains); + } return ret; } @@ -870,14 +773,11 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, { GVirConnectionPrivate *priv; GHashTable *pools; - gchar **inactive = NULL; - gint ninactive = 0; - gchar **active = NULL; - gint nactive = 0; + virStoragePoolPtr *vpools = NULL; + gint npools = 0; gboolean ret = FALSE; gint i; virConnectPtr vconn = NULL; - GError *lerr = NULL; g_return_val_if_fail(GVIR_IS_CONNECTION(conn), FALSE); g_return_val_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable), @@ -901,77 +801,31 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; - active = fetch_list(vconn, - "Storage Pools", - virConnectNumOfStoragePools, - virConnectListStoragePools, - cancellable, - &nactive, - &lerr); - if (lerr) { - g_propagate_error(err, lerr); - lerr = NULL; + npools = virConnectListAllStoragePools(vconn, &vpools, 0); + if (npools < 0) { + gvir_set_error(err, GVIR_CONNECTION_ERROR, + 0, + _("Failed to fetch list of pools")); goto cleanup; } if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; - inactive = fetch_list(vconn, - "Storage Pools", - virConnectNumOfDefinedStoragePools, - virConnectListDefinedStoragePools, - cancellable, - &ninactive, - &lerr); - if (lerr) { - g_propagate_error(err, lerr); - lerr = NULL; - goto cleanup; - } - pools = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, g_object_unref); - for (i = 0 ; i < nactive ; i++) { - if (g_cancellable_set_error_if_cancelled(cancellable, err)) - goto cleanup; - - virStoragePoolPtr vpool; + for (i = 0 ; i < npools; i++) { GVirStoragePool *pool; - vpool = virStoragePoolLookupByName(vconn, active[i]); - if (!vpool) - continue; - - pool = GVIR_STORAGE_POOL(g_object_new(GVIR_TYPE_STORAGE_POOL, - "handle", vpool, - NULL)); - virStoragePoolFree(vpool); - - g_hash_table_insert(pools, - (gpointer)gvir_storage_pool_get_uuid(pool), - pool); - } - - for (i = 0 ; i < ninactive ; i++) { if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; - virStoragePoolPtr vpool; - GVirStoragePool *pool; - - vpool = virStoragePoolLookupByName(vconn, inactive[i]); - if (!vpool) - continue; - pool = GVIR_STORAGE_POOL(g_object_new(GVIR_TYPE_STORAGE_POOL, - "handle", vpool, + "handle", vpools[i], NULL)); - virStoragePoolFree(vpool); - g_hash_table_insert(pools, (gpointer)gvir_storage_pool_get_uuid(pool), pool); @@ -987,12 +841,11 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, ret = TRUE; cleanup: - for (i = 0 ; i < nactive ; i++) - g_free(active[i]); - g_free(active); - for (i = 0 ; i < ninactive ; i++) - g_free(inactive[i]); - g_free(inactive); + if (npools > 0) { + for (i = 0 ; i < npools; i++) + virStoragePoolFree(vpools[i]); + free(vpools); + } return ret; } -- 2.4.3 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list