On Tue, Jun 19, 2007 at 02:16:23PM +0100, Richard W.M. Jones wrote: > (1) Document the URI formats fully. definitely > -- This is a feature which has been requested a few times and I will > do it anyway if Daniel Veillard will do the magic to set up a > "http://libvirt.org/uri.html" page. Okay I will do that within an hour :-)

- if our default behaviour was a bit less pathological virsh: error: failed to connect to the hypervisor paphio:~/libvirt -> virsh help libvir: error : operation failed: xenProxyOpen virsh: error: failed to connect to the hypervisor paphio:~/libvirt -> there is certainly things to be done at the virsh level too to improve user experience which don't require playing at the API level. Also to note, relying just on the driver may not work, suppose the user is not running a Xen kernel (like I pasted before) falling back to a Proxy mode won't work, but that won't tell him the real cause of the problem.

On Tue, Jun 19, 2007 at 02:16:23PM +0100, Richard W.M. Jones wrote: > (1) Document the URI formats fully. definitely > -- This is a feature which has been requested a few times and I will > do it anyway if Daniel Veillard will do the magic to set up a > "http://libvirt.org/uri.html" page. Okay I will do that within an hour :-)

On Tue, Jun 19, 2007 at 05:15:40PM +0100, Richard W.M. Jones wrote: > The attached one-liner adds a xen:/// URI, intended as the normal method > to connect to the Xen hypervisor on the local machine. This is just a > placeholder until I can get around to rewriting the Xen name parsing > code in xen_unified.c. This patch makes local (xen:///) and remote > (xen://hostname/) Xen URI-style calls possible and hopefully doesn't > prevent logical extensions to the Xen URI syntax from being added in future. > > Also, I couldn't get file path URIs to work as they seem to be intended, > but I haven't looked very closely yet: > > $ virsh -c ///var/lib/xend/xend-socket list > libvir: error : no support for hypervisor > virsh: error: failed to connect to the hypervisor This is because those URIs are declined by the xen_unified.c open method before they get anywhere near xend_internal.c I'm attaching a patch which addresses this, making xen_unified.c convert any NULL, 'xen', 'Xen' uri into xen:/// before passing it onto the other Xen drivers. This should make Rich's initial patch redundant. It also explicitly allows through any URI starting with / or http:// as back compat for Xen. Finally, it moves the remote driver to be the last one registered, and ensures the Xen & test drivers explicitly decline any URI with a hostname specified, so that they get passed onwards to the remote driver. I need this because when I move the QEMU driver across then I have an interesting scenario. Initially 'qemu:///session' has to be handled by the remote driver, but once inside the remote daemon that very same URI has to be handled by the QEMU driver. The QEMU driver can detect when its run inside the daemon, so by having the remote driver last I can handle this scenario quite easily.

Daniel P. Berrange wrote: >On Tue, Jun 19, 2007 at 05:15:40PM +0100, Richard W.M. Jones wrote: >>The attached one-liner adds a xen:/// URI, intended as the normal method >>to connect to the Xen hypervisor on the local machine. This is just a >>placeholder until I can get around to rewriting the Xen name parsing >>code in xen_unified.c. This patch makes local (xen:///) and remote >>(xen://hostname/) Xen URI-style calls possible and hopefully doesn't >>prevent logical extensions to the Xen URI syntax from being added in >>future. >> >>Also, I couldn't get file path URIs to work as they seem to be intended, >>but I haven't looked very closely yet: >> >> $ virsh -c ///var/lib/xend/xend-socket list >> libvir: error : no support for hypervisor >> virsh: error: failed to connect to the hypervisor > >This is because those URIs are declined by the xen_unified.c open method >before they get anywhere near xend_internal.c I've verified that your patch fixes this. >I'm attaching a patch which addresses this, making xen_unified.c convert >any NULL, 'xen', 'Xen' uri into xen:/// before passing it onto the other >Xen drivers. This should make Rich's initial patch redundant. It also >explicitly allows through any URI starting with / or http:// as back >compat for Xen. It turns out my patch in xend_internal.c is still needed because otherwise it refuses the new-style xen:/// URI and the connection to xend fails. What's really needed is to fix this so different parts of the Xen driver aren't all trying to make uncoordinated attempts to parse the URI ... later. >Finally, it moves the remote driver to be the last one registered, and >ensures the Xen & test drivers explicitly decline any URI with a hostname >specified, so that they get passed onwards to the remote driver. > >I need this because when I move the QEMU driver across then I have an >interesting scenario. Initially 'qemu:///session' has to be handled >by the remote driver, but once inside the remote daemon that very same >URI has to be handled by the QEMU driver. The QEMU driver can detect >when its run inside the daemon, so by having the remote driver last >I can handle this scenario quite easily. Remember that a remote URI is characterised by one of _two_ things: (1) there is a server in the URI or (2) the URI contains a transport. So for example: xen+ssh:/// is a remote URI.

Anyhow, your patch is still correct (I checked all the drivers and they now will only proceed if the scheme is one of a set of fixed strings). But by rearranging the list of drivers, we must remember in future. What I don't really understand is why this is necessary -- it worked fine before. The remote driver removes the transport and server name from the URI before passing it through to the remote end. It looks like you're planning to modify the remote driver to handle qemu:/// URIs ..?

Daniel Veillard wrote: > - if our default behaviour was a bit less pathological > > virsh: error: failed to connect to the hypervisor > paphio:~/libvirt -> virsh help > libvir: error : operation failed: xenProxyOpen > virsh: error: failed to connect to the hypervisor This patch fixes the pathological failure of 'virsh help command'. (1) We allow ctl->conn to be NULL, meaning "no connection". (2) All cmd* functions check vshConnectionUsability where required (I checked this) except the ones which don't need to such as cmdHelp. (3) vshInit has changed so that not being able to connect to a hypervisor is a non-fatal condition. Thus after vshInit, ctl->conn may be NULL. (4) I reordered the list of commands alphabetically, except that "help" is listed first. $ virsh help save libvir: error : operation failed: xenProxyOpen error: failed to connect to the hypervisor NAME save - save a domain state to a file SYNOPSIS save <domain> <file> DESCRIPTION Save a running domain. OPTIONS <domain> domain name, id or uuid <file> where to save the data