[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libvir] [PATCH] Don't throw away errors in virConnectOpen (at least not so often)



Error messages, I like them.  I don't like them to be thrown away.

A nice feature of virterror is that it'll throw away errors under the following conditions:
(1) You are in virConnectOpen, and
(2) You pass a non-NULL virConnectPtr to __virRaiseError.

libvirt has a lot of errors which meet those conditions - the attached patch fixes the ones I could find.

It also fixes qemuOpenConnection so that it doesn't try to open a Unix socket with random stack data.

It also adds error messages in some useful places where previously there was an error, but no message.

Rich.

--
Emerging Technologies, Red Hat  http://et.redhat.com/~rjones/
64 Baker Street, London, W1U 7DF     Mobile: +44 7866 314 421

Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.
Registered in England and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Charlie Peters (USA) and David
Owens (Ireland)
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/proxy_internal.c libvirt-open-errors/src/proxy_internal.c
--- libvirt-cvs/src/proxy_internal.c	2007-05-01 13:55:02.000000000 +0100
+++ libvirt-open-errors/src/proxy_internal.c	2007-05-01 14:29:04.000000000 +0100
@@ -544,7 +544,7 @@
 
     fd = virProxyOpenClientSocket(PROXY_SOCKET_PATH);
     if (fd < 0) {
-	    virProxyError(conn, VIR_ERR_NO_XEN, PROXY_SOCKET_PATH);
+	    virProxyError(NULL, VIR_ERR_NO_XEN, PROXY_SOCKET_PATH);
         return(-1);
     }
     priv->proxy = fd;
@@ -554,7 +554,7 @@
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, NULL, 1);
     if ((ret < 0) || (req.command != VIR_PROXY_NONE)) {
-	    virProxyError(conn, VIR_ERR_OPERATION_FAILED, __FUNCTION__);
+	    virProxyError(NULL, VIR_ERR_OPERATION_FAILED, __FUNCTION__);
         xenProxyClose(conn);
 	return(-1);
     }
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/qemu_internal.c libvirt-open-errors/src/qemu_internal.c
--- libvirt-cvs/src/qemu_internal.c	2007-05-01 13:55:02.000000000 +0100
+++ libvirt-open-errors/src/qemu_internal.c	2007-05-01 14:49:17.000000000 +0100
@@ -146,7 +146,7 @@
     int ret, pid, status;
 
     if (!proxyPath) {
-        fprintf(stderr, "failed to find qemu\n");
+        qemuError (NULL, NULL, VIR_ERR_INVALID_ARG, "no proxyPath");
         return(-1);
     }
 
@@ -231,6 +231,7 @@
  retry:
     fd = socket(PF_UNIX, SOCK_STREAM, 0);
     if (fd < 0) {
+        qemuError (NULL, NULL, VIR_ERR_SYSTEM_ERROR, "socket");
         return VIR_DRV_OPEN_ERROR;
     }
 
@@ -256,6 +257,10 @@
             usleep(5000 * trials * trials);
             goto retry;
         }
+        __virRaiseError (NULL, NULL, NULL,
+                         VIR_FROM_QEMU, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR,
+                         "connect", NULL, NULL, errno, 0,
+                         "connect: %s: %s", path, strerror (errno));
         return VIR_DRV_OPEN_ERROR;
     }
 
@@ -346,34 +351,43 @@
     int autostart = 0;
 
     if (uri->server != NULL) {
+        qemuError (NULL, NULL, VIR_ERR_INTERNAL_ERROR, __FUNCTION__);
         return VIR_DRV_OPEN_ERROR;
     }
 
-    if (!strcmp(uri->path, "/system")) {
+    if (strcmp(uri->path, "/system") == 0) {
         if (readonly) {
             if (snprintf(path, sizeof(path), "%s/run/libvirt/qemud-sock-ro", LOCAL_STATE_DIR) >= (int)sizeof(path)) {
+                qemuError (NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
                 return VIR_DRV_OPEN_ERROR;
             }
         } else {
             if (snprintf(path, sizeof(path), "%s/run/libvirt/qemud-sock", LOCAL_STATE_DIR) >= (int)sizeof(path)) {
+                qemuError (NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
                 return VIR_DRV_OPEN_ERROR;
             }
         }
-    } else if (!strcmp(uri->path, "/session")) {
+    } else if (strcmp(uri->path, "/session") == 0) {
         struct passwd *pw;
         int uid;
 
         if ((uid = geteuid()) < 0) {
+            qemuError (NULL, NULL, VIR_ERR_SYSTEM_ERROR, "geteuid");
             return VIR_DRV_OPEN_ERROR;
         }
 
-        if (!(pw = getpwuid(uid)))
+        if (!(pw = getpwuid(uid))) {
+            qemuError (NULL, NULL, VIR_ERR_SYSTEM_ERROR, "getpwuid");
             return VIR_DRV_OPEN_ERROR;
+        }
 
         if (snprintf(path, sizeof(path), "@%s/.libvirt/qemud-sock", pw->pw_dir) == sizeof(path)) {
             return VIR_DRV_OPEN_ERROR;
         }
         autostart = 1;
+    } else {
+        qemuError (NULL, NULL, VIR_ERR_INVALID_ARG, "path should be /system or /session - for example, qemu:///session");
+        return VIR_DRV_OPEN_ERROR;
     }
     return qemuOpenClientUNIX(conn, path, autostart);
 }
@@ -395,7 +409,7 @@
 
     uri = xmlParseURI(name);
     if (uri == NULL) {
-        qemuError(conn, NULL, VIR_ERR_NO_SUPPORT, name);
+        qemuError(NULL, NULL, VIR_ERR_NO_SUPPORT, name);
         return VIR_DRV_OPEN_DECLINED;
     }
 
@@ -409,7 +423,7 @@
     /* Create per-connection private data. */
     priv = conn->privateData = malloc (sizeof *priv);
     if (!priv) {
-        qemuError (conn, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__);
+        qemuError (NULL, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__);
         return VIR_DRV_OPEN_ERROR;
     }
 
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/test.c libvirt-open-errors/src/test.c
--- libvirt-cvs/src/test.c	2007-05-01 13:55:02.000000000 +0100
+++ libvirt-open-errors/src/test.c	2007-05-01 14:21:14.000000000 +0100
@@ -562,7 +562,7 @@
     if (ret == 0) {
         nodeInfo->nodes = l;
     } else if (ret == -2) {
-	testError(conn, NULL, VIR_ERR_XML_ERROR, _("node cpu numa nodes"));
+	testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu numa nodes"));
 	goto error;
     }
 
@@ -570,7 +570,7 @@
     if (ret == 0) {
         nodeInfo->sockets = l;
     } else if (ret == -2) {
-	testError(conn, NULL, VIR_ERR_XML_ERROR, _("node cpu sockets"));
+	testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu sockets"));
 	goto error;
     }
 
@@ -578,7 +578,7 @@
     if (ret == 0) {
         nodeInfo->cores = l;
     } else if (ret == -2) {
-	testError(conn, NULL, VIR_ERR_XML_ERROR, _("node cpu cores"));
+	testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu cores"));
 	goto error;
     }
 
@@ -586,7 +586,7 @@
     if (ret == 0) {
         nodeInfo->threads = l;
     } else if (ret == -2) {
-	testError(conn, NULL, VIR_ERR_XML_ERROR, _("node cpu threads"));
+	testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu threads"));
 	goto error;
     }
 
@@ -597,14 +597,14 @@
 	    nodeInfo->cpus = l;
 	}
     } else if (ret == -2) {
-	testError(conn, NULL, VIR_ERR_XML_ERROR, _("node active cpu"));
+	testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node active cpu"));
 	goto error;
     }
     ret = virXPathLong("string(/node/cpu/mhz[1])", ctxt, &l);
     if (ret == 0) {
         nodeInfo->mhz = l;
     } else if (ret == -2) {
-        testError(conn, NULL, VIR_ERR_XML_ERROR, _("node cpu mhz"));
+        testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu mhz"));
 	goto error;
     }
 
@@ -619,7 +619,7 @@
     if (ret == 0) {
         nodeInfo->memory = l;
     } else if (ret == -2) {
-        testError(conn, NULL, VIR_ERR_XML_ERROR, _("node memory"));
+        testError(NULL, NULL, VIR_ERR_XML_ERROR, _("node memory"));
 	goto error;
     }
 
@@ -717,7 +717,7 @@
 
     uri = xmlParseURI(name);
     if (uri == NULL) {
-        testError(conn, NULL, VIR_ERR_NO_SUPPORT, name);
+        testError(NULL, NULL, VIR_ERR_NO_SUPPORT, name);
         return VIR_DRV_OPEN_DECLINED;
     }
 
@@ -730,7 +730,7 @@
     if (!uri->path
         || uri->path[0] == '\0'
         || (uri->path[0] == '/' && uri->path[1] == '\0')) {
-        testError (conn, NULL, VIR_ERR_INVALID_ARG,
+        testError (NULL, NULL, VIR_ERR_INVALID_ARG,
                    _("testOpen: supply a path or use test:///default"));
         return VIR_DRV_OPEN_ERROR;
     }
@@ -743,7 +743,7 @@
     /* Allocate per-connection private data. */
     priv = conn->privateData = malloc (sizeof (struct _testPrivate));
     if (!priv) {
-        testError(conn, NULL, VIR_ERR_NO_MEMORY, _("allocating private data"));
+        testError(NULL, NULL, VIR_ERR_NO_MEMORY, _("allocating private data"));
         return VIR_DRV_OPEN_ERROR;
     }
     priv->handle = -1;
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/xend_internal.c libvirt-open-errors/src/xend_internal.c
--- libvirt-cvs/src/xend_internal.c	2007-05-01 13:55:03.000000000 +0100
+++ libvirt-open-errors/src/xend_internal.c	2007-05-01 14:34:35.000000000 +0100
@@ -894,7 +894,7 @@
     pent = gethostbyname(host);
     if (pent == NULL) {
         if (inet_aton(host, &ip) == 0) {
-            virXendError(conn, VIR_ERR_UNKNOWN_HOST, host);
+            virXendError(NULL, VIR_ERR_UNKNOWN_HOST, host);
             errno = ESRCH;
             return (-1);
         }
@@ -1960,14 +1960,14 @@
          */
         uri = xmlParseURI(name);
         if (uri == NULL) {
-            virXendError(conn, VIR_ERR_NO_SUPPORT, name);
+            virXendError(NULL, VIR_ERR_NO_SUPPORT, name);
             goto failed;
         }
 
         if (uri->scheme == NULL) {
             /* It should be a file access */
             if (uri->path == NULL) {
-                virXendError(conn, VIR_ERR_NO_SUPPORT, name);
+                virXendError(NULL, VIR_ERR_NO_SUPPORT, name);
                 goto failed;
             }
             ret = xenDaemonOpen_unix(conn, uri->path);
@@ -1985,7 +1985,7 @@
             if (ret == -1)
                 goto failed;
         } else {
-            virXendError(conn, VIR_ERR_NO_SUPPORT, name);
+            virXendError(NULL, VIR_ERR_NO_SUPPORT, name);
             goto failed;
         }
     }
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/xen_unified.c libvirt-open-errors/src/xen_unified.c
--- libvirt-cvs/src/xen_unified.c	2007-05-01 13:55:02.000000000 +0100
+++ libvirt-open-errors/src/xen_unified.c	2007-05-01 14:52:55.000000000 +0100
@@ -95,7 +95,7 @@
     /* Allocate per-connection private data. */
     priv = malloc (sizeof *priv);
     if (!priv) {
-        xenUnifiedError (conn, VIR_ERR_NO_MEMORY, "allocating private data");
+        xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, "allocating private data");
         return VIR_DRV_OPEN_ERROR;
     }
     conn->privateData = priv;
diff -urN --exclude=CVS --exclude=.git --exclude='*.pem' --exclude=demoCA --exclude=.gitignore --exclude='*.orig' --exclude='*.bak' libvirt-cvs/src/xs_internal.c libvirt-open-errors/src/xs_internal.c
--- libvirt-cvs/src/xs_internal.c	2007-05-01 13:55:03.000000000 +0100
+++ libvirt-open-errors/src/xs_internal.c	2007-05-01 14:37:30.000000000 +0100
@@ -340,7 +340,7 @@
 #endif /* ! PROXY */
 
     if (priv->xshandle == NULL) {
-        virXenStoreError(conn, VIR_ERR_NO_XEN, 
+        virXenStoreError(NULL, VIR_ERR_NO_XEN, 
 	                     _("failed to connect to Xen Store"));
         return (-1);
     }

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]