Daniel Veillard wrote:
On Tue, Jun 19, 2007 at 02:16:23PM +0100, Richard W.M. Jones wrote:(3) Split VIR_ERR_NO_SUPPORT into two categories. Currently this category mixes up cases where we fail to open a connection, and cases where there is no driver support for a particular operation (even with an open connection). In the first case, go through all the places which return this error and add proper diagnostic information to the error messages.-- Again, I am prepared to do this if people think it's a good idea.It would be logical to separate the two, I think we already at the low level do the difference but it doesn't show up at the error level.
The attached patch:(1) Makes VIR_ERR_NO_SUPPORT mean that a function is not supported by the hypervisor. It changes the message to be more specific and coincidentally corrects a bug which was causing the optional info field to be dropped.
(2) Makes VIR_ERR_NO_CONNECT mean that we could not connect to the hypervisor.
(3) Deprecates VIR_ERR_CALL_FAILED, because the meaning and use of this overlapped with VIR_ERR_NO_SUPPORT, and some calls failed with one and other calls failed with the other. Accordingly it changes all VIR_ERR_CALL_FAILED errors into VIR_ERR_NO_SUPPORT errors.
(4) Removes all error messages which occur before VIR_DRV_OPEN_DECLINED. If a driver is declines a URI, then that is normal behaviour and no reason to print an error. A later driver in the chain may accept the URI, and if none of the drivers can accept it then libvirt.c:do_open contains code to generate an error.
(5) If xen_unified.c was passed a naked string like "foo" as a URI then it would try to use it as a relative file reference. This was wrong and obscures a whole class of error message, so I've changed it so that xen_unified.c will only try to accept an absolute pathname (eg. ///var/lib/xen/xend-socket).
Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ 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. 03798903
Index: src/virterror.c =================================================================== RCS file: /data/cvs/libvirt/src/virterror.c,v retrieving revision 1.26 diff -u -r1.26 virterror.c --- src/virterror.c 7 Jun 2007 13:05:01 -0000 1.26 +++ src/virterror.c 20 Jun 2007 13:03:39 -0000 @@ -410,10 +410,10 @@ errmsg = _("out of memory"); break; case VIR_ERR_NO_SUPPORT: - if (info != NULL) - errmsg = _("no support for hypervisor"); + if (info == NULL) + errmsg = _("this function is not supported by the hypervisor"); else - errmsg = _("no support for hypervisor %s"); + errmsg = _("this function is not supported by the hypervisor: %s"); break; case VIR_ERR_NO_CONNECT: if (info == NULL) @@ -538,7 +538,7 @@ else errmsg = _("too many drivers registered in %s"); break; - case VIR_ERR_CALL_FAILED: + case VIR_ERR_CALL_FAILED: /* DEPRECATED, use VIR_ERR_NO_SUPPORT */ if (info == NULL) errmsg = _("library call failed, possibly not supported"); else Index: src/libvirt.c =================================================================== RCS file: /data/cvs/libvirt/src/libvirt.c,v retrieving revision 1.76 diff -u -r1.76 libvirt.c --- src/libvirt.c 20 Jun 2007 10:01:14 -0000 1.76 +++ src/libvirt.c 20 Jun 2007 13:03:41 -0000 @@ -314,7 +314,8 @@ } if (!ret->driver) { - virLibConnError (NULL, VIR_ERR_NO_SUPPORT, name); + /* If we reach here, then all drivers declined the connection. */ + virLibConnError (NULL, VIR_ERR_NO_CONNECT, name); goto failed; } @@ -889,7 +890,7 @@ if (conn->driver->domainSave) return conn->driver->domainSave (domain, to); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -942,7 +943,7 @@ if (conn->driver->domainRestore) return conn->driver->domainRestore (conn, from); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1001,7 +1002,7 @@ if (conn->driver->domainCoreDump) return conn->driver->domainCoreDump (domain, to, flags); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1037,7 +1038,7 @@ if (conn->driver->domainShutdown) return conn->driver->domainShutdown (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1071,7 +1072,7 @@ if (conn->driver->domainReboot) return conn->driver->domainReboot (domain, flags); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1217,7 +1218,7 @@ if (conn->driver->domainGetOSType) return conn->driver->domainGetOSType (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -1246,7 +1247,7 @@ if (conn->driver->domainGetMaxMemory) return conn->driver->domainGetMaxMemory (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return 0; } @@ -1288,7 +1289,7 @@ if (conn->driver->domainSetMaxMemory) return conn->driver->domainSetMaxMemory (domain, memory); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1331,7 +1332,7 @@ if (conn->driver->domainSetMemory) return conn->driver->domainSetMemory (domain, memory); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1367,7 +1368,7 @@ if (conn->driver->domainGetInfo) return conn->driver->domainGetInfo (domain, info); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1401,7 +1402,7 @@ if (conn->driver->domainDumpXML) return conn->driver->domainDumpXML (domain, flags); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -1429,7 +1430,7 @@ if (conn->driver->nodeGetInfo) return conn->driver->nodeGetInfo (conn, info); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1454,7 +1455,7 @@ if (conn->driver->getCapabilities) return conn->driver->getCapabilities (conn); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -1489,7 +1490,7 @@ return schedtype; } - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -1528,7 +1529,7 @@ if (conn->driver->domainGetSchedulerParameters) return conn->driver->domainGetSchedulerParameters (domain, params, nparams); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1564,7 +1565,7 @@ if (conn->driver->domainSetSchedulerParameters) return conn->driver->domainSetSchedulerParameters (domain, params, nparams); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1603,7 +1604,7 @@ if (conn->driver->domainDefineXML) return conn->driver->domainDefineXML (conn, xml); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -1632,7 +1633,7 @@ if (conn->driver->domainUndefine) return conn->driver->domainUndefine (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1655,7 +1656,7 @@ if (conn->driver->numOfDefinedDomains) return conn->driver->numOfDefinedDomains (conn); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1685,7 +1686,7 @@ if (conn->driver->listDefinedDomains) return conn->driver->listDefinedDomains (conn, names, maxnames); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1719,7 +1720,7 @@ if (conn->driver->domainCreate) return conn->driver->domainCreate (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1754,7 +1755,7 @@ if (conn->driver->domainGetAutostart) return conn->driver->domainGetAutostart (domain, autostart); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1784,7 +1785,7 @@ if (conn->driver->domainSetAutostart) return conn->driver->domainSetAutostart (domain, autostart); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1828,7 +1829,7 @@ if (conn->driver->domainSetVcpus) return conn->driver->domainSetVcpus (domain, nvcpus); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1879,7 +1880,7 @@ if (conn->driver->domainPinVcpu) return conn->driver->domainPinVcpu (domain, vcpu, cpumap, maplen); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1933,7 +1934,7 @@ return conn->driver->domainGetVcpus (domain, info, maxinfo, cpumaps, maplen); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1964,7 +1965,7 @@ if (conn->driver->domainGetMaxVcpus) return conn->driver->domainGetMaxVcpus (domain); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -1996,7 +1997,7 @@ if (conn->driver->domainAttachDevice) return conn->driver->domainAttachDevice (domain, xml); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2027,7 +2028,7 @@ if (conn->driver->domainDetachDevice) return conn->driver->domainDetachDevice (domain, xml); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2050,7 +2051,7 @@ if (conn->networkDriver && conn->networkDriver->numOfNetworks) return conn->networkDriver->numOfNetworks (conn); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2080,7 +2081,7 @@ if (conn->networkDriver && conn->networkDriver->listNetworks) return conn->networkDriver->listNetworks (conn, names, maxnames); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2103,7 +2104,7 @@ if (conn->networkDriver && conn->networkDriver->numOfDefinedNetworks) return conn->networkDriver->numOfDefinedNetworks (conn); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2135,7 +2136,7 @@ return conn->networkDriver->listDefinedNetworks (conn, names, maxnames); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2163,7 +2164,7 @@ if (conn->networkDriver && conn->networkDriver->networkLookupByName) return conn->networkDriver->networkLookupByName (conn, name); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2191,7 +2192,7 @@ if (conn->networkDriver && conn->networkDriver->networkLookupByUUID) return conn->networkDriver->networkLookupByUUID (conn, uuid); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2273,7 +2274,7 @@ if (conn->networkDriver && conn->networkDriver->networkCreateXML) return conn->networkDriver->networkCreateXML (conn, xmlDesc); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2305,7 +2306,7 @@ if (conn->networkDriver && conn->networkDriver->networkDefineXML) return conn->networkDriver->networkDefineXML (conn, xml); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2334,7 +2335,7 @@ if (conn->networkDriver && conn->networkDriver->networkUndefine) return conn->networkDriver->networkUndefine (network); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2368,7 +2369,7 @@ if (conn->networkDriver && conn->networkDriver->networkCreate) return conn->networkDriver->networkCreate (network); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2403,7 +2404,7 @@ if (conn->networkDriver && conn->networkDriver->networkDestroy) return conn->networkDriver->networkDestroy (network); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2539,7 +2540,7 @@ if (conn->networkDriver && conn->networkDriver->networkDumpXML) return conn->networkDriver->networkDumpXML (network, flags); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2568,7 +2569,7 @@ if (conn->networkDriver && conn->networkDriver->networkGetBridgeName) return conn->networkDriver->networkGetBridgeName (network); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } @@ -2603,7 +2604,7 @@ if (conn->networkDriver && conn->networkDriver->networkGetAutostart) return conn->networkDriver->networkGetAutostart (network, autostart); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } @@ -2633,7 +2634,7 @@ if (conn->networkDriver && conn->networkDriver->networkSetAutostart) return conn->networkDriver->networkSetAutostart (network, autostart); - virLibConnError (conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return -1; } Index: src/qemu_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_internal.c,v retrieving revision 1.27 diff -u -r1.27 qemu_internal.c --- src/qemu_internal.c 15 Jun 2007 15:24:20 -0000 1.27 +++ src/qemu_internal.c 20 Jun 2007 13:03:42 -0000 @@ -473,7 +473,6 @@ uri = xmlParseURI(name); if (uri == NULL) { - qemuError(NULL, NULL, VIR_ERR_NO_SUPPORT, name); return VIR_DRV_OPEN_DECLINED; } Index: src/test.c =================================================================== RCS file: /data/cvs/libvirt/src/test.c,v retrieving revision 1.33 diff -u -r1.33 test.c --- src/test.c 20 Jun 2007 10:01:14 -0000 1.33 +++ src/test.c 20 Jun 2007 13:03:43 -0000 @@ -720,7 +720,6 @@ uri = xmlParseURI(name); if (uri == NULL) { - testError(NULL, NULL, VIR_ERR_NO_SUPPORT, name); return VIR_DRV_OPEN_DECLINED; } Index: src/xen_unified.c =================================================================== RCS file: /data/cvs/libvirt/src/xen_unified.c,v retrieving revision 1.11 diff -u -r1.11 xen_unified.c --- src/xen_unified.c 20 Jun 2007 10:01:14 -0000 1.11 +++ src/xen_unified.c 20 Jun 2007 13:03:43 -0000 @@ -99,23 +99,33 @@ uri = xmlParseURI(name); if (uri == NULL) { - xenUnifiedError(NULL, VIR_ERR_NO_SUPPORT, name); return VIR_DRV_OPEN_DECLINED; } - /* Refuse any URI which doesn't start xen:///, / or http:// */ + /* Refuse any scheme which isn't "xen://" or "http://". */ if (uri->scheme && strcasecmp(uri->scheme, "xen") != 0 && - strcasecmp(uri->scheme, "http")) { + strcasecmp(uri->scheme, "http") != 0) { + xmlFreeURI(uri); + return VIR_DRV_OPEN_DECLINED; + } + + /* xmlParseURI will parse a naked string like "foo" as a URI with + * a NULL scheme. That's not useful for us because we want to only + * allow full pathnames (eg. ///var/lib/xen/xend-socket). Decline + * anything else. + */ + if (!uri->scheme && name[0] != '/') { xmlFreeURI(uri); return VIR_DRV_OPEN_DECLINED; } /* Refuse any xen:// URI with a server specified - allow remote to do it */ - if (uri->scheme && !strcasecmp(uri->scheme, "xen") && uri->server) { + if (uri->scheme && strcasecmp(uri->scheme, "xen") == 0 && uri->server) { xmlFreeURI(uri); return VIR_DRV_OPEN_DECLINED; } + xmlFreeURI(uri); /* Allocate per-connection private data. */ Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.121 diff -u -r1.121 xend_internal.c --- src/xend_internal.c 19 Jun 2007 19:08:09 -0000 1.121 +++ src/xend_internal.c 20 Jun 2007 13:03:45 -0000 @@ -1975,14 +1975,14 @@ */ uri = xmlParseURI(name); if (uri == NULL) { - virXendError(NULL, VIR_ERR_NO_SUPPORT, name); + virXendError(NULL, VIR_ERR_NO_CONNECT, name); goto failed; } if (uri->scheme == NULL) { /* It should be a file access */ if (uri->path == NULL) { - virXendError(NULL, VIR_ERR_NO_SUPPORT, name); + virXendError(NULL, VIR_ERR_NO_CONNECT, name); goto failed; } ret = xenDaemonOpen_unix(conn, uri->path); @@ -2000,7 +2000,7 @@ if (ret == -1) goto failed; } else { - virXendError(NULL, VIR_ERR_NO_SUPPORT, name); + virXendError(NULL, VIR_ERR_NO_CONNECT, name); goto failed; } } Index: include/libvirt/virterror.h =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/virterror.h,v retrieving revision 1.24 diff -u -r1.24 virterror.h --- include/libvirt/virterror.h 7 Jun 2007 13:05:01 -0000 1.24 +++ include/libvirt/virterror.h 20 Jun 2007 13:03:45 -0000 @@ -86,7 +86,7 @@ VIR_ERR_OK = 0, VIR_ERR_INTERNAL_ERROR, /* internal error */ VIR_ERR_NO_MEMORY, /* memory allocation failure */ - VIR_ERR_NO_SUPPORT, /* no support for this connection */ + VIR_ERR_NO_SUPPORT, /* no support for this function */ VIR_ERR_UNKNOWN_HOST,/* could not resolve hostname */ VIR_ERR_NO_CONNECT, /* can't connect to hypervisor */ VIR_ERR_INVALID_CONN,/* invalid connection object */ @@ -109,7 +109,7 @@ VIR_ERR_NO_DEVICE, /* missing domain devices information */ VIR_ERR_NO_XENSTORE,/* could not open Xen Store control */ VIR_ERR_DRIVER_FULL, /* too many drivers registered */ - VIR_ERR_CALL_FAILED, /* not supported by the drivers */ + VIR_ERR_CALL_FAILED, /* not supported by the drivers (DEPRECATED) */ VIR_ERR_XML_ERROR, /* an XML description is not well formed or broken */ VIR_ERR_DOM_EXIST,/* the domain already exist */ VIR_ERR_OPERATION_DENIED, /* operation forbidden on read-only connections */
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature