On Tue, Nov 22, 2011 at 02:18:15PM +0000, Daniel P. Berrange wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

--- libvirt-gobject/libvirt-gobject-connection.c | 104 +++++++++++++--------- libvirt-gobject/libvirt-gobject-domain.c | 70 ++++++++------ libvirt-gobject/libvirt-gobject-interface.c | 7 +- libvirt-gobject/libvirt-gobject-network-filter.c | 7 +- libvirt-gobject/libvirt-gobject-network.c | 7 +- libvirt-gobject/libvirt-gobject-node-device.c | 7 +- libvirt-gobject/libvirt-gobject-secret.c | 7 +- libvirt-gobject/libvirt-gobject-storage-pool.c | 53 +++++++----- libvirt-gobject/libvirt-gobject-storage-vol.c | 7 +- 9 files changed, 158 insertions(+), 111 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index affb496..35be5e3 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -389,19 +389,21 @@ gboolean gvir_connection_open(GVirConnection *conn,

g_mutex_lock(priv->lock); if (priv->conn) { - *err = g_error_new(GVIR_CONNECTION_ERROR, - 0, - "Connection %s is already open", - priv->uri); + if (err) + *err = g_error_new(GVIR_CONNECTION_ERROR, + 0, + "Connection %s is already open", + priv->uri);

g_set_error(err, GVIR_CONNECTION_ERROR, 0, "Connection...); does exactly that (test for non-NULL + g_error_new). I'm fine with this patch (ie ACK from me), but it would be even better to introduce gvir_set_error(...) and to use that. This can be done later though. Christophe

g_mutex_unlock(priv->lock); return FALSE; }

if (!(priv->conn = virConnectOpen(priv->uri))) { - *err = gvir_error_new(GVIR_CONNECTION_ERROR, - 0, - "Unable to open %s", - priv->uri); + if (err) + *err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + "Unable to open %s", + priv->uri); g_mutex_unlock(priv->lock); return FALSE; } @@ -540,9 +542,10 @@ static gchar ** fetch_list(virConnectPtr vconn, gint i;

if ((n = count_func(vconn)) < 0) { - *err = gvir_error_new(GVIR_CONNECTION_ERROR, - 0, - "Unable to count %s", name); + if (err) + *err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + "Unable to count %s", name); goto error; }

@@ -552,9 +555,10 @@ static gchar ** fetch_list(virConnectPtr vconn,

lst = g_new(gchar *, n); if ((n = list_func(vconn, lst, n)) < 0) { - *err = gvir_error_new(GVIR_CONNECTION_ERROR, - 0, - "Unable to list %s %d", name, n); + if (err) + *err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + "Unable to list %s %d", name, n); goto error; } } @@ -587,12 +591,14 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, gboolean ret = FALSE; gint i; virConnectPtr vconn = NULL; + GError *lerr = NULL;

g_mutex_lock(priv->lock); if (!priv->conn) { - *err = g_error_new(GVIR_CONNECTION_ERROR, - 0, - "Connection is not open"); + if (err) + *err = g_error_new(GVIR_CONNECTION_ERROR, + 0, + "Connection is not open"); g_mutex_unlock(priv->lock); goto cleanup; } @@ -605,9 +611,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, goto cleanup;

if ((nactive = virConnectNumOfDomains(vconn)) < 0) { - *err = gvir_error_new(GVIR_CONNECTION_ERROR, - 0, - "Unable to count domains"); + if (err) + *err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + "Unable to count domains"); goto cleanup; } if (nactive) { @@ -616,9 +623,10 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,

active = g_new(gint, nactive); if ((nactive = virConnectListDomains(vconn, active, nactive)) < 0) { - *err = gvir_error_new(GVIR_CONNECTION_ERROR, - 0, - "Unable to list domains"); + if (err) + *err = gvir_error_new(GVIR_CONNECTION_ERROR, + 0, + "Unable to list domains"); goto cleanup; } } @@ -632,9 +640,12 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn, virConnectListDefinedDomains, cancellable, &ninactive, - err); - if (*err != NULL) + &lerr); + if (lerr) { + g_propagate_error(err, lerr); + lerr = NULL; goto cleanup; + }

doms = g_hash_table_new_full(g_str_hash, g_str_equal, @@ -712,12 +723,14 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, gboolean ret = FALSE; gint i; virConnectPtr vconn = NULL; + GError *lerr = NULL;

g_mutex_lock(priv->lock); if (!priv->conn) { - *err = g_error_new(GVIR_CONNECTION_ERROR, - 0, - "Connection is not open"); + if (err) + *err = g_error_new(GVIR_CONNECTION_ERROR, + 0, + "Connection is not open"); g_mutex_unlock(priv->lock); goto cleanup; } @@ -735,9 +748,12 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, virConnectListStoragePools, cancellable, &nactive, - err); - if (*err != NULL) + &lerr); + if (lerr) { + g_propagate_error(err, lerr); + lerr = NULL; goto cleanup; + }

if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; @@ -748,9 +764,12 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn, virConnectListDefinedStoragePools, cancellable, &ninactive, - err); - if (*err != NULL) + &lerr); + if (lerr) { + g_propagate_error(err, lerr); + lerr = NULL; goto cleanup; + }

pools = g_hash_table_new_full(g_str_hash, g_str_equal, @@ -1189,9 +1208,10 @@ GVirDomain *gvir_connection_create_domain(GVirConnection *conn, g_return_val_if_fail(xml != NULL, NULL);

if (!(handle = virDomainDefineXML(priv->conn, xml))) { - *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, - 0, - "Failed to create domain"); + if (err) + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, + 0, + "Failed to create domain"); return NULL; }

@@ -1234,9 +1254,10 @@ GVirDomain *gvir_connection_start_domain(GVirConnection *conn, g_return_val_if_fail(xml != NULL, NULL);

if (!(handle = virDomainCreateXML(priv->conn, xml, flags))) { - *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, - 0, - "Failed to create domain"); + if (err) + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, + 0, + "Failed to create domain"); return NULL; }

@@ -1278,9 +1299,10 @@ GVirStoragePool *gvir_connection_create_storage_pool g_return_val_if_fail(xml != NULL, NULL);

if (!(handle = virStoragePoolDefineXML(priv->conn, xml, flags))) { - *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, - flags, - "Failed to create storage pool"); + if (err) + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, + flags, + "Failed to create storage pool"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 4a7a534..36d618c 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -287,9 +287,10 @@ gint gvir_domain_get_id(GVirDomain *dom, gint ret;

if ((ret = virDomainGetID(priv->handle)) < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to get ID for domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to get ID for domain"); } return ret; } @@ -312,9 +313,10 @@ gboolean gvir_domain_start(GVirDomain *dom, else ret = virDomainCreate(priv->handle); if (ret < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to start domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to start domain"); return FALSE; }

@@ -333,9 +335,10 @@ gboolean gvir_domain_resume(GVirDomain *dom, GVirDomainPrivate *priv = dom->priv;

if (virDomainResume(priv->handle) < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to resume domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to resume domain"); return FALSE; }

@@ -359,9 +362,10 @@ gboolean gvir_domain_stop(GVirDomain *dom, else ret = virDomainDestroy(priv->handle); if (ret < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to stop domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to stop domain"); return FALSE; }

@@ -385,9 +389,10 @@ gboolean gvir_domain_delete(GVirDomain *dom, else ret = virDomainUndefine(priv->handle); if (ret < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to delete domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to delete domain"); return FALSE; }

@@ -406,9 +411,10 @@ gboolean gvir_domain_shutdown(GVirDomain *dom, GVirDomainPrivate *priv = dom->priv;

if (virDomainShutdown(priv->handle) < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to shutdown domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to shutdown domain"); return FALSE; }

@@ -427,9 +433,10 @@ gboolean gvir_domain_reboot(GVirDomain *dom, GVirDomainPrivate *priv = dom->priv;

if (virDomainReboot(priv->handle, flags) < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to reboot domain"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to reboot domain"); return FALSE; }

@@ -450,9 +457,10 @@ GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom, gchar *xml;

if (!(xml = virDomainGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to get domain XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to get domain XML config"); return NULL; }

@@ -546,9 +554,10 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom, GVirDomainInfo *ret;

if (virDomainGetInfo(priv->handle, &info) < 0) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to get domain info"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to get domain info"); return NULL; }

@@ -590,9 +599,10 @@ gchar *gvir_domain_screenshot(GVirDomain *dom, st, monitor_id, flags))) { - *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, - 0, - "Unable to take a screenshot"); + if (err) + *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Unable to take a screenshot"); goto end; } end: diff --git a/libvirt-gobject/libvirt-gobject-interface.c b/libvirt-gobject/libvirt-gobject-interface.c index e47395c..7af83ee 100644 --- a/libvirt-gobject/libvirt-gobject-interface.c +++ b/libvirt-gobject/libvirt-gobject-interface.c @@ -193,9 +193,10 @@ GVirConfigInterface *gvir_interface_get_config(GVirInterface *iface, gchar *xml;

if (!(xml = virInterfaceGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_INTERFACE_ERROR, - 0, - "Unable to get interface XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_INTERFACE_ERROR, + 0, + "Unable to get interface XML config"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c b/libvirt-gobject/libvirt-gobject-network-filter.c index bdb0e3a..7107e3b 100644 --- a/libvirt-gobject/libvirt-gobject-network-filter.c +++ b/libvirt-gobject/libvirt-gobject-network-filter.c @@ -219,9 +219,10 @@ GVirConfigNetworkFilter *gvir_network_filter_get_config gchar *xml;

if (!(xml = virNWFilterGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_NETWORK_FILTER_ERROR, - 0, - "Unable to get network_filter XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_NETWORK_FILTER_ERROR, + 0, + "Unable to get network_filter XML config"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-network.c b/libvirt-gobject/libvirt-gobject-network.c index c486561..b2cb2ec 100644 --- a/libvirt-gobject/libvirt-gobject-network.c +++ b/libvirt-gobject/libvirt-gobject-network.c @@ -215,9 +215,10 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork *network, gchar *xml;

if (!(xml = virNetworkGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_NETWORK_ERROR, - 0, - "Unable to get network XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_NETWORK_ERROR, + 0, + "Unable to get network XML config"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-node-device.c b/libvirt-gobject/libvirt-gobject-node-device.c index 43564b6..eb19513 100644 --- a/libvirt-gobject/libvirt-gobject-node-device.c +++ b/libvirt-gobject/libvirt-gobject-node-device.c @@ -194,9 +194,10 @@ GVirConfigNodeDevice *gvir_node_device_get_config(GVirNodeDevice *device, gchar *xml;

if (!(xml = virNodeDeviceGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_NODE_DEVICE_ERROR, - 0, - "Unable to get node_device XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_NODE_DEVICE_ERROR, + 0, + "Unable to get node_device XML config"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-secret.c b/libvirt-gobject/libvirt-gobject-secret.c index 418e5aa..0365e8d 100644 --- a/libvirt-gobject/libvirt-gobject-secret.c +++ b/libvirt-gobject/libvirt-gobject-secret.c @@ -205,9 +205,10 @@ GVirConfigSecret *gvir_secret_get_config(GVirSecret *secret, gchar *xml;

if (!(xml = virSecretGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_SECRET_ERROR, - 0, - "Unable to get secret XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_SECRET_ERROR, + 0, + "Unable to get secret XML config"); return NULL; }

diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 92be539..d398532 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -230,9 +230,10 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, gchar *xml;

if (!(xml = virStoragePoolGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, - 0, - "Unable to get storage_pool XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, + 0, + "Unable to get storage_pool XML config"); return NULL; }

@@ -303,25 +304,30 @@ gboolean gvir_storage_pool_refresh(GVirStoragePool *pool, gboolean ret = FALSE; gint i; virStoragePoolPtr vpool = NULL; + GError *lerr = NULL;

vpool = priv->handle;

if (virStoragePoolRefresh(vpool, 0) < 0) { - *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, - 0, - "Unable to refresh storage pool"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, + 0, + "Unable to refresh storage pool"); goto cleanup; }

volumes = fetch_list(vpool, - "Storage Volumes", - virStoragePoolNumOfVolumes, - virStoragePoolListVolumes, - cancellable, - &nvolumes, - err); - if (*err != NULL) + "Storage Volumes", + virStoragePoolNumOfVolumes, + virStoragePoolListVolumes, + cancellable, + &nvolumes, + &lerr); + if (lerr) { + g_propagate_error(err, lerr); + lerr = NULL; goto cleanup; + }

if (g_cancellable_set_error_if_cancelled(cancellable, err)) goto cleanup; @@ -495,9 +501,10 @@ GVirStorageVol *gvir_storage_pool_create_volume g_return_val_if_fail(xml != NULL, NULL);

if (!(handle = virStorageVolCreateXML(priv->handle, xml, 0))) { - *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, - 0, - "Failed to create volume"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to create volume"); return NULL; }

@@ -529,9 +536,10 @@ gboolean gvir_storage_pool_build (GVirStoragePool *pool, GError **err) { if (virStoragePoolBuild(pool->priv->handle, flags)) { - *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, - 0, - "Failed to build storage pool"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to build storage pool"); return FALSE; }

@@ -633,9 +641,10 @@ gboolean gvir_storage_pool_start (GVirStoragePool *pool, GError **err) { if (virStoragePoolCreate(pool->priv->handle, flags)) { - *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, - 0, - "Failed to start storage pool"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR, + 0, + "Failed to start storage pool"); return FALSE; }

diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index 17aac36..8fea736 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -205,9 +205,10 @@ GVirConfigStorageVol *gvir_storage_vol_get_config(GVirStorageVol *vol, gchar *xml;

if (!(xml = virStorageVolGetXMLDesc(priv->handle, flags))) { - *err = gvir_error_new_literal(GVIR_STORAGE_VOL_ERROR, - 0, - "Unable to get storage_vol XML config"); + if (err) + *err = gvir_error_new_literal(GVIR_STORAGE_VOL_ERROR, + 0, + "Unable to get storage_vol XML config"); return NULL; }

-- 1.7.6.4

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list