Centralize error reporting for URI parsing/formatting problems

Move error reporting out of the callers, into virURIParse
and virURIFormat, to get consistency.

* include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_URI
* src/util/viruri.c, src/util/viruri.h: Add error reporting
* src/esx/esx_driver.c, src/libvirt.c, src/libxl/libxl_driver.c,
  src/lxc/lxc_driver.c, src/openvz/openvz_driver.c,
  src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
  src/remote/remote_driver.c, src/uml/uml_driver.c,
  src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xen/xen_driver.c,
  src/xen/xend_internal.c, tests/viruritest.c: Remove error
  reporting

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 38ac15e05cc87c4fcf794c5c2c4755364ce31cd1..c8ec8d40cb53ec45d40c4c8d7f2dd647a3521fa1 100644 (file)
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -85,6 +85,7 @@ typedef enum {
     VIR_FROM_LOCKING = 42,     /* Error from lock manager */
     VIR_FROM_HYPERV = 43,      /* Error from Hyper-V driver */
     VIR_FROM_CAPABILITIES = 44, /* Error from capabilities */
+    VIR_FROM_URI = 45,          /* Error from URI handling */
 } virErrorDomain;
 
 

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index dbf95f44840732c04e1b7f1b55bce5d3ede37f9b..7e41fa32cdaf696e61140f23f692502caec734de 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3977,12 +3977,8 @@ esxDomainMigratePerform(virDomainPtr domain,
     }
 
     /* Parse migration URI */
-    parsedUri = virURIParse(uri);
-
-    if (parsedUri == NULL) {
-        virReportOOMError();
+    if (!(parsedUri = virURIParse(uri)))
         return -1;
-    }
 
     if (parsedUri->scheme == NULL || STRCASENEQ(parsedUri->scheme, "vpxmigr")) {
         ESX_ERROR(VIR_ERR_INVALID_ARG, "%s",

diff --git a/src/libvirt.c b/src/libvirt.c
index f674ecb46b200d4abecc628160dbbe6263adc668..7aee13ec78cf48b65805eb877b54a7a59c6f39c6 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1165,11 +1165,7 @@ do_open (const char *name,
             virConnectOpenResolveURIAlias(conf, name, &alias) < 0)
             goto failed;
 
-        ret->uri = virURIParse (alias ? alias : name);
-        if (!ret->uri) {
-            virLibConnError(VIR_ERR_INVALID_ARG,
-                            _("could not parse connection URI %s"),
-                            alias ? alias : name);
+        if (!(ret->uri = virURIParse (alias ? alias : name))) {
             VIR_FREE(alias);
             goto failed;
         }
@@ -1770,11 +1766,9 @@ virConnectGetURI (virConnectPtr conn)
         return NULL;
     }
 
-    name = virURIFormat(conn->uri);
-    if (!name) {
-        virReportOOMError();
+    if (!(name = virURIFormat(conn->uri)))
         goto error;
-    }
+
     return name;
 
 error:
@@ -5061,9 +5055,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
         return -1;
     }
 
-    tempuri = virURIParse(dconnuri);
-    if (!tempuri) {
-        virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+    if (!(tempuri = virURIParse(dconnuri))) {
         virDispatchError(domain->conn);
         return -1;
     }

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 177b21898162da0a1df9e558162a673f3d02caae..eccd198bcda8668d4992dbf362d8eea1aa6f7b16 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1044,11 +1044,8 @@ libxlOpen(virConnectPtr conn,
         if (libxl_driver == NULL)
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse("xen:///");
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse("xen:///")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         /* Only xen scheme */
         if (conn->uri->scheme == NULL || STRNEQ(conn->uri->scheme, "xen"))

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3af8084e7a4eb388d228cc00ac7a209e602a3136..29842a5c9c5bb372d2f3d42aee98c08ab2dab5b6 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -140,11 +140,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn,
         if (lxc_driver == NULL)
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse("lxc:///");
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse("lxc:///")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         if (conn->uri->scheme == NULL ||
             STRNEQ(conn->uri->scheme, "lxc"))

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 4e369eae35bd991bdc4ff8390453d6d36478be68..be306401ea0614f34949edc8b2cea0cf66ac4730 100644 (file)
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1336,11 +1336,8 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn,
         if (access("/proc/vz", W_OK) < 0)
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse("openvz:///system");
-        if (conn->uri == NULL) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse("openvz:///system")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         /* If scheme isn't 'openvz', then its for another driver */
         if (conn->uri->scheme == NULL ||

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b00257ba8aaf5dbaa7297cae63078b593389c1f5..57ade0e8018f4175cc4a49282b265eacf6194ec5 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -860,13 +860,10 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn,
         if (qemu_driver == NULL)
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse(qemu_driver->privileged ?
-                                "qemu:///system" :
-                                "qemu:///session");
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse(qemu_driver->privileged ?
+                                      "qemu:///system" :
+                                      "qemu:///session")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         /* If URI isn't 'qemu' its definitely not for us */
         if (conn->uri->scheme == NULL ||

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e380bc08c3257b2bcba0893d114a00835e577f88..802785f3412f6f88c6bd8da2d30a10f57637ea86 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1936,11 +1936,8 @@ static int doNativeMigrate(struct qemud_driver *driver,
     } else {
         uribits = virURIParse(uri);
     }
-    if (!uribits) {
-        qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                        _("cannot parse URI %s"), uri);
+    if (!uribits)
         return -1;
-    }
 
     if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD))
         spec.destType = MIGRATION_DEST_CONNECT_HOST;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 4ddebcb2679afdb544c02d3c35f844f7f8aabe02..c6c5809fccc0bb61ae91cc447b66341a7c245f20 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -485,7 +485,8 @@ doRemoteOpen (virConnectPtr conn,
                 (STREQ(conn->uri->scheme, "remote") ||
                  STRPREFIX(conn->uri->scheme, "remote+"))) {
                 /* Allow remote serve to probe */
-                name = strdup("");
+                if (!(name = strdup("")))
+                    goto out_of_memory;
             } else {
                 virURI tmpuri = {
                     .scheme = conn->uri->scheme,
@@ -515,6 +516,9 @@ doRemoteOpen (virConnectPtr conn,
                 /* Restore transport scheme */
                 if (transport_str)
                     transport_str[-1] = '+';
+
+                if (!name)
+                    goto failed;
             }
         }
 
@@ -522,12 +526,8 @@ doRemoteOpen (virConnectPtr conn,
         vars = NULL;
     } else {
         /* Probe URI server side */
-        name = strdup("");
-    }
-
-    if (!name) {
-        virReportOOMError();
-        goto failed;
+        if (!(name = strdup("")))
+            goto out_of_memory;
     }
 
     VIR_DEBUG("proceeding with name = %s", name);
@@ -720,10 +720,8 @@ doRemoteOpen (virConnectPtr conn,
         VIR_DEBUG("Auto-probed URI is %s", uriret.uri);
         conn->uri = virURIParse(uriret.uri);
         VIR_FREE(uriret.uri);
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!conn->uri)
             goto failed;
-        }
     }
 
     if (!(priv->domainEventState = virDomainEventStateNew()))

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 16818cdc18cf65898a2a2579abad465478b7db1c..d86652c1cd3780322b2264abb507c3328ce9552d 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1139,13 +1139,10 @@ static virDrvOpenStatus umlOpen(virConnectPtr conn,
         if (uml_driver == NULL)
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse(uml_driver->privileged ?
-                                "uml:///system" :
-                                "uml:///session");
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse(uml_driver->privileged ?
+                                      "uml:///system" :
+                                      "uml:///session")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         if (conn->uri->scheme == NULL ||
             STRNEQ (conn->uri->scheme, "uml"))

diff --git a/src/util/virterror.c b/src/util/virterror.c
index b4e496ac77585c3a45b958d24321203205fa291f..e1fe5228a14bc61f7cb257f3458d545d0615203e 100644 (file)
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -178,6 +178,9 @@ static const char *virErrorDomainName(virErrorDomain domain) {
         case VIR_FROM_CAPABILITIES:
             dom = "Capabilities ";
             break;
+        case VIR_FROM_URI:
+            dom = "URI ";
+            break;
     }
     return(dom);
 }

diff --git a/src/util/viruri.c b/src/util/viruri.c
index 1d2dca4b39048c39d9f916a6f1af89a4cd8390f2..bbd8742cb3d50c25757eacfed47ee362da0aaa4f 100644 (file)
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -12,6 +12,14 @@
 
 #include "memory.h"
 #include "util.h"
+#include "virterror_internal.h"
+
+#define VIR_FROM_THIS VIR_FROM_URI
+
+#define virURIReportError(code, ...)                                    \
+    virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,                 \
+                         __FUNCTION__, __LINE__, __VA_ARGS__)
+
 
 /**
  * virURIParse:
@@ -30,9 +38,15 @@ virURIParse(const char *uri)
 {
     virURIPtr ret = xmlParseURI(uri);
 
+    if (!ret) {
+        /* libxml2 does not tell us what failed. Grr :-( */
+        virURIReportError(VIR_ERR_INTERNAL_ERROR,
+                          "Unable to parse URI %s", uri);
+        return NULL;
+    }
+
     /* First check: does it even make sense to jump inside */
-    if (ret != NULL &&
-        ret->server != NULL &&
+    if (ret->server != NULL &&
         ret->server[0] == '[') {
         size_t length = strlen(ret->server);
 
@@ -70,8 +84,7 @@ virURIFormat(virURIPtr uri)
     char *ret;
 
     /* First check: does it make sense to do anything */
-    if (uri != NULL &&
-        uri->server != NULL &&
+    if (uri->server != NULL &&
         strchr(uri->server, ':') != NULL) {
 
         backupserver = uri->server;
@@ -82,7 +95,12 @@ virURIFormat(virURIPtr uri)
     }
 
     ret = (char *) xmlSaveUri(uri);
+    if (!ret) {
+        virReportOOMError();
+        goto cleanup;
+    }
 
+cleanup:
     /* Put the fixed version back */
     if (tmpserver) {
         uri->server = backupserver;

diff --git a/src/util/viruri.h b/src/util/viruri.h
index 80369beab2c4cf0e6ab1d30b3547600632b42498..5773ddadbb9a1bc1583890d6f151f9c3a3e740ec 100644 (file)
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -16,8 +16,10 @@
 typedef xmlURI    virURI;
 typedef xmlURIPtr virURIPtr;
 
-virURIPtr virURIParse(const char *uri);
-char *virURIFormat(virURIPtr uri);
+virURIPtr virURIParse(const char *uri)
+    ATTRIBUTE_NONNULL(1);
+char *virURIFormat(virURIPtr uri)
+    ATTRIBUTE_NONNULL(1);
 
 void virURIFree(virURIPtr uri);
 

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 853a599f162a53477d2100aa36f60e6507bd926a..7769b0cbe17fe14f762d86aa69cfd0e915398abe 100644 (file)
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -980,13 +980,9 @@ static virDrvOpenStatus vboxOpen(virConnectPtr conn,
 
     virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
 
-    if (conn->uri == NULL) {
-        conn->uri = virURIParse(uid ? "vbox:///session" : "vbox:///system");
-        if (conn->uri == NULL) {
-            virReportOOMError();
-            return VIR_DRV_OPEN_ERROR;
-        }
-    }
+    if (conn->uri == NULL &&
+        !(conn->uri = virURIParse(uid ? "vbox:///session" : "vbox:///system")))
+        return VIR_DRV_OPEN_ERROR;
 
     if (conn->uri->scheme == NULL ||
         STRNEQ (conn->uri->scheme, "vbox"))

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 3179688f75f7beccdb4725269a25dbba5f481040..4324bb8387951a681fe45afdcd222049321f0cf7 100644 (file)
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2617,12 +2617,8 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port,
         (*def)->target.port = port;
         (*def)->source.type = VIR_DOMAIN_CHR_TYPE_TCP;
 
-        parsedUri = virURIParse(fileName);
-
-        if (parsedUri == NULL) {
-            virReportOOMError();
+        if (!(parsedUri = virURIParse(fileName)))
             goto cleanup;
-        }
 
         if (parsedUri->port == 0) {
             VMX_ERROR(VIR_ERR_INTERNAL_ERROR,

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 0238ed740d6e478e026efe62b9d47633d6ec894d..4011913314c28e8dd038b21ea53f68efad10b905 100644 (file)
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -270,11 +270,8 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
         if (!xenUnifiedProbe())
             return VIR_DRV_OPEN_DECLINED;
 
-        conn->uri = virURIParse("xen:///");
-        if (!conn->uri) {
-            virReportOOMError();
+        if (!(conn->uri = virURIParse("xen:///")))
             return VIR_DRV_OPEN_ERROR;
-        }
     } else {
         if (conn->uri->scheme) {
             /* Decline any scheme which isn't "xen://" or "http://". */

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index df6b1bb31bc6e1a9a1a3fee7d1968eb21620e3f6..55bb5db6aee798e53fd0a35459b9e04ff1f9daea 100644 (file)
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3224,12 +3224,10 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
      * "hostname", "hostname:port" or "xenmigr://hostname[:port]/".
      */
     if (strstr (uri, "//")) {   /* Full URI. */
-        virURIPtr uriptr = virURIParse (uri);
-        if (!uriptr) {
-            virXendError(VIR_ERR_INVALID_ARG,
-                          "%s", _("xenDaemonDomainMigrate: invalid URI"));
+        virURIPtr uriptr;
+        if (!(uriptr = virURIParse (uri)))
             return -1;
-        }
+
         if (uriptr->scheme && STRCASENEQ (uriptr->scheme, "xenmigr")) {
             virXendError(VIR_ERR_INVALID_ARG,
                           "%s", _("xenDaemonDomainMigrate: only xenmigr://"

diff --git a/tests/viruritest.c b/tests/viruritest.c
index 91da6e6ef59bc5d8437ed0d0ad48b2d60ffd28e1..13eebe3d5a675a9e6956622e12e762ac82e817a1 100644 (file)
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -50,15 +50,11 @@ static int testURIParse(const void *args)
     const struct URIParseData *data = args;
     char *uristr;
 
-    if (!(uri = virURIParse(data->uri))) {
-        virReportOOMError();
+    if (!(uri = virURIParse(data->uri)))
         goto cleanup;
-    }
 
-    if (!(uristr = virURIFormat(uri))) {
-        virReportOOMError();
+    if (!(uristr = virURIFormat(uri)))
         goto cleanup;
-    }
 
     if (!STREQ(uristr, data->uri)) {
         VIR_DEBUG("URI did not roundtrip, expect '%s', actual '%s'",