There are currently far too many cases where a correct URI returns an generic 'failed to connect to hypervisor' error message. This gives the user no idea why they could not connect. Of particular importance is that they cannot distinguish a correct URI + plus a driver problem, vs incorrect URI. Consider the following examples virsh -c qemu:///bogus error: could not connect to qemu:///bogus error: failed to connect to the hypervisor virsh -c qemu:// error: could not connect to qemu:// error: failed to connect to the hypervisor virsh -c /var/lib/xen/xend-socket error: could not connect to /var/lib/xen/xend-socket error: failed to connect to the hypervisor virsh -c /bogus error: could not connect to /bogus error: failed to connect to the hypervisor virsh -c bogus error: could not connect to bogus error: failed to connect to the hypervisor virsh -c uml:///bogus error: could not connect to uml:///bogus error: failed to connect to the hypervisor virsh -c uml:// error: could not connect to uml:// error: failed to connect to the hypervisor virsh -c openvz:///system error: could not connect to openvz:///system error: failed to connect to the hypervisor virsh -c openvz:///bogus error: could not connect to openvz:///bogus error: failed to connect to the hypervisor virsh -c openvz:/// error: could not connect to openvz:/// error: failed to connect to the hypervisor virsh -c bogus:/// error: could not connect to bogus:/// error: failed to connect to the hypervisor virsh -c qemu+tls:///system error: unable to connect to 'localhost': Connection refused error: failed to connect to the hypervisor virsh -c qemu://localhost/system error: could not connect to qemu://localhost/system error: failed to connect to the hypervisor virsh -c bogus+unix:/// error: could not connect to bogus:/// error: failed to connect to the hypervisor The core problem here is that far too many places are using the return code VIR_DRV_OPEN_DECLINED instead of VIR_DRV_OPEN_ERROR. The former provides no way to give any error info to the user. With this patch I have taken the view that a driver must *only* ever use the return code VIR_DRV_OPEN_DECLINED when: - Auto-probe of a driver URI, and this driver is not active - Explicit URI with 'server' specified - URI scheme does not match the driver The remote driver is special in that it *must* accept all URIs given to it, no matter what, unless of course it is running inside the daemon already. The result is that if you give a correct URI, but there is a real problem opening the driver you are now guarenteed to get a useful error message back. Consider the same examples again virsh -c qemu:///bogus error: internal error unexpected QEMU URI path '/bogus', try qemu:///system error: failed to connect to the hypervisor virsh -c qemu:// error: internal error unexpected QEMU URI path '', try qemu:///system error: failed to connect to the hypervisor virsh -c xen:/// error: unable to connect to 'localhost:8000': Connection refused error: failed to connect to the hypervisor virsh -c xen:///bogus error: internal error unexpected Xen URI path '/bogus', try xen:/// error: failed to connect to the hypervisor virsh -c /var/lib/xen/xend-socket error: internal error failed to connect to xend error: failed to connect to the hypervisor virsh -c /bogus error: internal error failed to connect to xend error: failed to connect to the hypervisor virsh -c bogus error: internal error unexpected Xen URI path 'bogus', try ///var/lib/xen/xend-socket error: failed to connect to the hypervisor virsh -c uml:///bogus error: internal error unexpected UML URI path '/bogus', try uml:///system error: failed to connect to the hypervisor virsh -c uml:// error: internal error unexpected UML URI path '', try uml:///system error: failed to connect to the hypervisor virsh -c lxc:// error: internal error unexpected LXC URI path '', try lxc:/// error: failed to connect to the hypervisor virsh -c lxc:///bogus error: internal error unexpected LXC URI path '/bogus', try lxc:/// error: failed to connect to the hypervisor virsh -c openvz:///system error: internal error OpenVZ control file /proc/vz does not exist error: failed to connect to the hypervisor virsh -c openvz:///bogus error: internal error unexpected OpenVZ URI path '/bogus', try openvz:///system error: failed to connect to the hypervisor virsh -c openvz:/// error: internal error unexpected OpenVZ URI path '/', try openvz:///system error: failed to connect to the hypervisor virsh -c vbox:///system error: internal error unknown driver path '/system' specified (try vbox:///session) error: failed to connect to the hypervisor virsh -c vbox:///bogus error: internal error unknown driver path '/bogus' specified (try vbox:///session) error: failed to connect to the hypervisor virsh -c vbox:// error: internal error no VirtualBox driver path specified (try vbox:///session) error: failed to connect to the hypervisor virsh -c bogus:/// error: no hypervisor driver available for bogus:/// error: failed to connect to the hypervisor virsh -c qemu+tls:///system error: unable to connect to libvirtd at 'localhost': Connection refused error: failed to connect to the hypervisor virsh -c qemu://localhost/system error: unable to connect to libvirtd at 'localhost': Connection refused error: failed to connect to the hypervisor virsh -c bogus+unix:/// error: no hypervisor driver available for bogus:/// error: failed to connect to the hypervisor In every case there is now a useful error message. lxc_driver.c | 48 ++++++++++---------- openvz_driver.c | 57 ++++++++++++++++------- qemu_driver.c | 83 +++++++++++++++++----------------- remote_internal.c | 129 ++++++++++++++++++++++++++++-------------------------- test.c | 3 - uml_driver.c | 62 ++++++++++++++++--------- virterror.c | 4 - xen_unified.c | 55 +++++++++++++++-------- xend_internal.c | 7 ++ 9 files changed, 259 insertions(+), 189 deletions(-) Daniel diff -r 3c35f3b77286 src/lxc_driver.c --- a/src/lxc_driver.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/lxc_driver.c Wed Jun 03 17:31:17 2009 +0100 @@ -68,46 +68,48 @@ static void lxcDriverUnlock(lxc_driver_t } -static int lxcProbe(void) -{ - if (getuid() != 0 || - lxcContainerAvailable(0) < 0) - return 0; - - return 1; -} - static virDrvOpenStatus lxcOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (lxc_driver == NULL) - goto declineConnection; - /* Verify uri was specified */ if (conn->uri == NULL) { - if (!lxcProbe()) - goto declineConnection; + if (lxc_driver == NULL) + return VIR_DRV_OPEN_DECLINED; conn->uri = xmlParseURI("lxc:///"); if (!conn->uri) { virReportOOMError(conn); return VIR_DRV_OPEN_ERROR; } - } else if (conn->uri->scheme == NULL || - STRNEQ(conn->uri->scheme, "lxc")) { - goto declineConnection; - } else if (!lxcProbe()) { - goto declineConnection; + } else { + if (conn->uri->scheme == NULL || + STRNEQ(conn->uri->scheme, "lxc")) + return VIR_DRV_OPEN_DECLINED; + + /* Leave for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + /* If path isn't '/' then they typoed, tell them correct path */ + if (STRNEQ(conn->uri->path, "/")) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected LXC URI path '%s', try lxc:///"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + + /* URI was good, but driver isn't active */ + if (lxc_driver == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("lxc state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } } - conn->privateData = lxc_driver; return VIR_DRV_OPEN_SUCCESS; - -declineConnection: - return VIR_DRV_OPEN_DECLINED; } static int lxcClose(virConnectPtr conn) diff -r 3c35f3b77286 src/openvz_driver.c --- a/src/openvz_driver.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/openvz_driver.c Wed Jun 03 17:31:17 2009 +0100 @@ -1070,36 +1070,59 @@ cleanup: return ret; } -static int openvzProbe(void) -{ - if (geteuid() == 0 && - virFileExists("/proc/vz")) - return 1; - - return 0; -} - static virDrvOpenStatus openvzOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver; - if (!openvzProbe()) - return VIR_DRV_OPEN_DECLINED; if (conn->uri == NULL) { + if (!virFileExists("/proc/vz")) + return VIR_DRV_OPEN_DECLINED; + + if (access("/proc/vz", W_OK) < 0) + return VIR_DRV_OPEN_DECLINED; + conn->uri = xmlParseURI("openvz:///system"); if (conn->uri == NULL) { - openvzError(conn, VIR_ERR_NO_DOMAIN, NULL); + virReportOOMError(conn); return VIR_DRV_OPEN_ERROR; } - } else if (conn->uri->scheme == NULL || - conn->uri->path == NULL || - STRNEQ (conn->uri->scheme, "openvz") || - STRNEQ (conn->uri->path, "/system")) { - return VIR_DRV_OPEN_DECLINED; + } else { + /* If scheme isn't 'openvz', then its for another driver */ + if (conn->uri->scheme == NULL || + STRNEQ (conn->uri->scheme, "openvz")) + return VIR_DRV_OPEN_DECLINED; + + /* If server name is given, its for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + /* If path isn't /system, then they typoed, so tell them correct path */ + if (conn->uri->path == NULL || + STRNEQ (conn->uri->path, "/system")) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected OpenVZ URI path '%s', try openvz:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + + if (!virFileExists("/proc/vz")) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("OpenVZ control file /proc/vz does not exist")); + return VIR_DRV_OPEN_ERROR; + } + + if (access("/proc/vz", W_OK) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("OpenVZ control file /proc/vz is not accessible")); + return VIR_DRV_OPEN_ERROR; + } } + /* We now know the URI is definitely for this driver, so beyond + * here, don't return DECLINED, always use ERROR */ + if (VIR_ALLOC(driver) < 0) { virReportOOMError(conn); return VIR_DRV_OPEN_ERROR; diff -r 3c35f3b77286 src/qemu_driver.c --- a/src/qemu_driver.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/qemu_driver.c Wed Jun 03 17:31:17 2009 +0100 @@ -1743,62 +1743,61 @@ qemudMonitorCommand(const virDomainObjPt } -/** - * qemudProbe: - * - * Probe for the availability of the qemu driver, assume the - * presence of QEmu emulation if the binaries are installed - */ -static int qemudProbe(void) -{ - if ((virFileExists("/usr/bin/qemu")) || - (virFileExists("/usr/bin/qemu-kvm")) || - (virFileExists("/usr/bin/kvm")) || - (virFileExists("/usr/bin/xenner"))) - return 1; - - return 0; -} static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { uid_t uid = getuid(); - if (qemu_driver == NULL) - goto decline; - - if (!qemudProbe()) - goto decline; - if (conn->uri == NULL) { - conn->uri = xmlParseURI(uid ? "qemu:///session" : "qemu:///system"); + if (qemu_driver == NULL) + return VIR_DRV_OPEN_DECLINED; + + conn->uri = xmlParseURI(uid == 0 ? + "qemu:///system" : + "qemu:///session"); if (!conn->uri) { virReportOOMError(conn); return VIR_DRV_OPEN_ERROR; } - } else if (conn->uri->scheme == NULL || - conn->uri->path == NULL) - goto decline; - - if (STRNEQ (conn->uri->scheme, "qemu")) - goto decline; - - if (uid != 0) { - if (STRNEQ (conn->uri->path, "/session")) - goto decline; - } else { /* root */ - if (STRNEQ (conn->uri->path, "/system") && - STRNEQ (conn->uri->path, "/session")) - goto decline; - } - + } else { + /* If URI isn't 'qemu' its definitely not for us */ + if (conn->uri->scheme == NULL || + STRNEQ(conn->uri->scheme, "qemu")) + return VIR_DRV_OPEN_DECLINED; + + /* Allow remote driver to deal with URIs with hostname server */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + if (!uid) { + if (STRNEQ (conn->uri->path, "/system") && + STRNEQ (conn->uri->path, "/session")) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected QEMU URI path '%s', try qemu:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ (conn->uri->path, "/session")) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected QEMU URI path '%s', try qemu:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + + /* URI was good, but driver isn't active */ + if (qemu_driver == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + + } conn->privateData = qemu_driver; return VIR_DRV_OPEN_SUCCESS; - - decline: - return VIR_DRV_OPEN_DECLINED; } static int qemudClose(virConnectPtr conn) { diff -r 3c35f3b77286 src/remote_internal.c --- a/src/remote_internal.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/remote_internal.c Wed Jun 03 17:31:17 2009 +0100 @@ -305,21 +305,28 @@ remoteForkDaemon(virConnectPtr conn) enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), - VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1), - VIR_DRV_OPEN_REMOTE_USER = (1 << 2), - VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3), -}; - -/* What transport? */ -enum { - trans_tls, - trans_unix, - trans_ssh, - trans_ext, - trans_tcp, -} transport; - - + VIR_DRV_OPEN_REMOTE_USER = (1 << 1), /* Use the per-user socket path */ + VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2), /* Autostart a per-user daemon */ +}; + + +/* + * URIs that this driver needs to handle: + * + * The easy answer: + * - Everything that no one else has yet claimed, but nothing if + * we're inside the libvirtd daemon + * + * The hard answer: + * - Plain paths (///var/lib/xen/xend-socket) -> UNIX domain socket + * - xxx://servername/ -> TLS connection + * - xxx+tls://servername/ -> TLS connection + * - xxx+tls:/// -> TLS connection to localhost + * - xxx+tcp://servername/ -> TCP connection + * - xxx+tcp:/// -> TCP connection to localhost + * - xxx+unix:/// -> UNIX domain socket + * - xxx:/// -> UNIX domain socket + */ static int doRemoteOpen (virConnectPtr conn, struct private_data *priv, @@ -328,37 +335,51 @@ doRemoteOpen (virConnectPtr conn, { int wakeupFD[2] = { -1, -1 }; char *transport_str = NULL; + enum { + trans_tls, + trans_unix, + trans_ssh, + trans_ext, + trans_tcp, + } transport; + + /* We handle *ALL* URIs here. The caller has rejected any + * URIs we don't care about */ if (conn->uri) { - if (!conn->uri->scheme) - return VIR_DRV_OPEN_DECLINED; - - transport_str = get_transport_from_scheme (conn->uri->scheme); - - if (!transport_str || STRCASEEQ (transport_str, "tls")) - transport = trans_tls; - else if (STRCASEEQ (transport_str, "unix")) + if (!conn->uri->scheme) { + /* This is the ///var/lib/xen/xend-socket local path style */ transport = trans_unix; - else if (STRCASEEQ (transport_str, "ssh")) - transport = trans_ssh; - else if (STRCASEEQ (transport_str, "ext")) - transport = trans_ext; - else if (STRCASEEQ (transport_str, "tcp")) - transport = trans_tcp; - else { - error (conn, VIR_ERR_INVALID_ARG, - _("remote_open: transport in URL not recognised " - "(should be tls|unix|ssh|ext|tcp)")); - return VIR_DRV_OPEN_ERROR; - } - } - - if (!transport_str) { - if ((!conn->uri || !conn->uri->server) && - (flags & VIR_DRV_OPEN_REMOTE_UNIX)) - transport = trans_unix; - else - return VIR_DRV_OPEN_DECLINED; /* Decline - not a remote URL. */ + } else { + transport_str = get_transport_from_scheme (conn->uri->scheme); + + if (!transport_str) { + if (conn->uri->server) + transport = trans_tls; + else + transport = trans_unix; + } else { + if (STRCASEEQ (transport_str, "tls")) + transport = trans_tls; + else if (STRCASEEQ (transport_str, "unix")) + transport = trans_unix; + else if (STRCASEEQ (transport_str, "ssh")) + transport = trans_ssh; + else if (STRCASEEQ (transport_str, "ext")) + transport = trans_ext; + else if (STRCASEEQ (transport_str, "tcp")) + transport = trans_tcp; + else { + error (conn, VIR_ERR_INVALID_ARG, + _("remote_open: transport in URL not recognised " + "(should be tls|unix|ssh|ext|tcp)")); + return VIR_DRV_OPEN_ERROR; + } + } + } + } else { + /* No URI, then must be probing so use UNIX socket */ + transport = trans_unix; } /* Local variables which we will initialise. These can @@ -455,8 +476,9 @@ doRemoteOpen (virConnectPtr conn, /* Construct the original name. */ if (!name) { - if (STREQ(conn->uri->scheme, "remote") || - STRPREFIX(conn->uri->scheme, "remote+")) { + if (conn->uri->scheme && + (STREQ(conn->uri->scheme, "remote") || + STRPREFIX(conn->uri->scheme, "remote+"))) { /* Allow remote serve to probe */ name = strdup(""); } else { @@ -580,7 +602,7 @@ doRemoteOpen (virConnectPtr conn, freeaddrinfo (res); virReportSystemError(conn, saved_errno, - _("unable to connect to '%s'"), + _("unable to connect to libvirtd at '%s'"), priv->hostname); goto failed; @@ -925,7 +947,6 @@ remoteOpenSecondaryDriver(virConnectPtr if (flags & VIR_CONNECT_RO) rflags |= VIR_DRV_OPEN_REMOTE_RO; - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; ret = doRemoteOpen(conn, *priv, auth, rflags); if (ret != VIR_DRV_OPEN_SUCCESS) { @@ -958,19 +979,6 @@ remoteOpen (virConnectPtr conn, /* * If no servername is given, and no +XXX - * transport is listed, then force to a - * local UNIX socket connection - */ - if (conn->uri && - !conn->uri->server && - conn->uri->scheme && - !strchr(conn->uri->scheme, '+')) { - DEBUG0("Auto-remote UNIX socket"); - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; - } - - /* - * If no servername is given, and no +XXX * transport is listed, or transport is unix, * and path is /session, and uid is unprivileged * then auto-spawn a daemon. @@ -996,7 +1004,6 @@ remoteOpen (virConnectPtr conn, */ if (!conn->uri) { DEBUG0("Auto-probe remote URI"); - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; #ifndef __sun if (getuid() > 0) { DEBUG0("Auto-spawn user daemon instance"); diff -r 3c35f3b77286 src/test.c --- a/src/test.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/test.c Wed Jun 03 17:31:17 2009 +0100 @@ -639,9 +639,6 @@ static virDrvOpenStatus testOpen(virConn if (conn->uri->server) return VIR_DRV_OPEN_DECLINED; - if (conn->uri->server) - return VIR_DRV_OPEN_DECLINED; - /* From this point on, the connection is for us. */ if (!conn->uri->path || conn->uri->path[0] == '\0' diff -r 3c35f3b77286 src/uml_driver.c --- a/src/uml_driver.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/uml_driver.c Wed Jun 03 17:31:17 2009 +0100 @@ -913,28 +913,49 @@ static virDrvOpenStatus umlOpen(virConne int flags ATTRIBUTE_UNUSED) { uid_t uid = getuid(); - if (uml_driver == NULL) - goto decline; + if (conn->uri == NULL) { + if (uml_driver == NULL) + return VIR_DRV_OPEN_DECLINED; - if (conn->uri != NULL) { - if (conn->uri->scheme == NULL || conn->uri->path == NULL) - goto decline; - - if (STRNEQ (conn->uri->scheme, "uml")) - goto decline; - - if (uid != 0) { - if (STRNEQ (conn->uri->path, "/session")) - goto decline; - } else { /* root */ - if (STRNEQ (conn->uri->path, "/system") && - STRNEQ (conn->uri->path, "/session")) - goto decline; + conn->uri = xmlParseURI(uid == 0 ? + "uml:///system" : + "uml:///session"); + if (!conn->uri) { + virReportOOMError(conn); + return VIR_DRV_OPEN_ERROR; } } else { - conn->uri = xmlParseURI(uid ? "uml:///session" : "uml:///system"); - if (!conn->uri) { - virReportOOMError(conn); + if (conn->uri->scheme == NULL || + STRNEQ (conn->uri->scheme, "uml")) + return VIR_DRV_OPEN_DECLINED; + + /* Allow remote driver to deal with URIs with hostname server */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + + /* Check path and tell them correct path if they made a mistake */ + if (uid == 0) { + if (STRNEQ (conn->uri->path, "/system") && + STRNEQ (conn->uri->path, "/session")) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected UML URI path '%s', try uml:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } else { + if (STRNEQ (conn->uri->path, "/session")) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected UML URI path '%s', try uml:///session"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + } + + /* URI was good, but driver isn't active */ + if (uml_driver == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("uml state driver is not active")); return VIR_DRV_OPEN_ERROR; } } @@ -942,9 +963,6 @@ static virDrvOpenStatus umlOpen(virConne conn->privateData = uml_driver; return VIR_DRV_OPEN_SUCCESS; - - decline: - return VIR_DRV_OPEN_DECLINED; } static int umlClose(virConnectPtr conn) { diff -r 3c35f3b77286 src/virterror.c --- a/src/virterror.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/virterror.c Wed Jun 03 17:31:17 2009 +0100 @@ -737,9 +737,9 @@ virErrorMsg(virErrorNumber error, const break; case VIR_ERR_NO_CONNECT: if (info == NULL) - errmsg = _("could not connect to hypervisor"); + errmsg = _("no hypervisor driver available"); else - errmsg = _("could not connect to %s"); + errmsg = _("no hypervisor driver available for %s"); break; case VIR_ERR_INVALID_CONN: if (info == NULL) diff -r 3c35f3b77286 src/xen_unified.c --- a/src/xen_unified.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/xen_unified.c Wed Jun 03 17:31:17 2009 +0100 @@ -241,25 +241,46 @@ xenUnifiedOpen (virConnectPtr conn, virC virReportOOMError (NULL); return VIR_DRV_OPEN_ERROR; } + } else { + if (conn->uri->scheme) { + /* Decline any scheme which isn't "xen://" or "http://". */ + if (STRCASENEQ(conn->uri->scheme, "xen") && + STRCASENEQ(conn->uri->scheme, "http")) + return VIR_DRV_OPEN_DECLINED; + + + /* Return an error if the path isn't '' or '/' */ + if (conn->uri->path && + STRNEQ(conn->uri->path, "") && + STRNEQ(conn->uri->path, "/")) { + xenUnifiedError(NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected Xen URI path '%s', try xen:///"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + + /* Decline any xen:// URI with a server specified, allowing remote + * driver to handle, but keep any http:/// URIs */ + if (STRCASEEQ(conn->uri->scheme, "xen") && + conn->uri->server) + return VIR_DRV_OPEN_DECLINED; + } else { + /* Special case URI for Xen driver only: + * + * Treat a plain path as a Xen UNIX socket path, and give + * error unless path is absolute + */ + if (!conn->uri->path || conn->uri->path[0] != '/') { + xenUnifiedError(NULL, VIR_ERR_INTERNAL_ERROR, + _("unexpected Xen URI path '%s', try ///var/lib/xen/xend-socket"), + NULLSTR(conn->uri->path)); + return VIR_DRV_OPEN_ERROR; + } + } } - /* Refuse any scheme which isn't "xen://" or "http://". */ - if (conn->uri->scheme && - STRCASENEQ(conn->uri->scheme, "xen") && - STRCASENEQ(conn->uri->scheme, "http")) - 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 (!conn->uri->scheme && (!conn->uri->path || conn->uri->path[0] != '/')) - return VIR_DRV_OPEN_DECLINED; - - /* Refuse any xen:// URI with a server specified - allow remote to do it */ - if (conn->uri->scheme && STRCASEEQ(conn->uri->scheme, "xen") && conn->uri->server) - return VIR_DRV_OPEN_DECLINED; + /* We now know the URI is definitely for this driver, so beyond + * here, don't return DECLINED, always use ERROR */ /* Allocate per-connection private data. */ if (VIR_ALLOC(priv) < 0) { diff -r 3c35f3b77286 src/xend_internal.c --- a/src/xend_internal.c Wed Jun 03 15:37:52 2009 +0100 +++ b/src/xend_internal.c Wed Jun 03 17:31:17 2009 +0100 @@ -2927,10 +2927,13 @@ xenDaemonOpen(virConnectPtr conn, xend_detect_config_version(conn) == -1) goto failed; } else if (STRCASEEQ (conn->uri->scheme, "http")) { - if (virAsprintf(&port, "%d", conn->uri->port) == -1) + if (conn->uri->port && + virAsprintf(&port, "%d", conn->uri->port) == -1) goto failed; - if (xenDaemonOpen_tcp(conn, conn->uri->server, port) < 0 || + if (xenDaemonOpen_tcp(conn, + conn->uri->server ? conn->uri->server : "localhost", + port ? port : "8000") < 0 || xend_detect_config_version(conn) == -1) goto failed; } else { -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|