* Anthony Liguori <aliguori(a)us.ibm.com&gt; [2006-03-02 14:14:38]: Right. Is there a preferred formatting for the name parameter? 'http://dom0.whatever:8000' is a little annoying to parse, and doesn't specify that the connection is to a Xen daemon, which could be a problem for future support of other virtualization engines. How about 'xen:tcp:dom0.whatever.net:8000' ? Other than that, I'm factoring out the connection step into a single function that handles both readonly and write connections, and I'm wondering about the handling. In virConnectOpenReadOnly, all three methods (hypervisor, xenstore, xend) are tried in turn, and the only criterion for success is that one succeed. However, in virConnectOpen, all three must succeed. Would it be desirable to unite these three, and if not could you explain why there is a difference in handling of failure between the two? My question here is all to do with figuring out how to handle remote xend connections. If the connection is remote, then the hypervisor and xenstore connections will inevitably fail. And if that is acceptable in the case of remote connections, then surely the two opening functions could be factored into a single one that always tolerates partial failure? Actually, looking through the code, it seems to me that the xend interface can accomplish almost all of the current API by itself (and the few functions it doesn't provide could probably be implemented). Is the direct hypervisor/xenstore access kept in only for performance reasons? Erm, you can? Through the official API, and while the guest is running? I grepped for 'cpu' in the source code, and that returned only querying functions. Not yet (though it'd be a nice feature to have, for completedness :) ), but being able to set the general number of vcpus for a domain, without pinning them to any specific one would be cool. Ah, good. Then patches will be coming soon :) - Dave, still a little puzzled.

* Anthony Liguori <aliguori(a)us.ibm.com&gt; [2006-03-02 18:05:58]: Okay. And when we start supporting other virtualization methods, we can rev that API to add a 'type' param. Okay. Then, when xend has a way of locking a connection to read-only, we can change that. We can keep them different opens. I just wanted to unite the code in a single internal function, and make the public functions use that, to avoid code duplication: static virConnectPtr connect_open(const char *host, int port, int read_only) { /* Common init code */ } virConnectPtr virConnectOpen(const char *host, int port) { return(connect_open(host, port, 0)); } virConnectPtr virConnectOpenReadOnly(const char *host, int port) { return(connect_open(host, port, 1)); } Also, currently OpenReadOnly still tries to open the xen hypervisor and xend connections. Shouldn't it just open a xenstore connection, to enforce read-onlyness ? Ok, that's clearer now. Thanks. Oh, right. I'll send a patch for vcpu setting support then. Right. - Dave.

* Daniel Veillard <veillard(a)redhat.com&gt; [2006-03-03 04:15:34]: Hmm, okay. What I can do then is revert to using a single URI-type argument, write a simple parsing function for it to decompose the URI into its components, and support specific initialization for a xen:// type. Something like 'xen://dom0.domain.net:8000' , or 'xen://' for the default of localhost. Okay. Like I said, I'm more than willing to provide patches for what I propose, I just need a little guidance wrt. what you plan for the future evolutions of the lib :). While we're on the design questions, I'd like to put another change spec up for review. It is to do with the current segmentation of the library. Currently, there are three ways to access Xen-related data: the direct link to the hypervisor, the xenstore handle, or a xend connection. In each of the public API functions, there are tests to try one, then the other (if more than one can retrieve the requested data), etc. This layout does not scale well, especially if the plan is to introduce support for other virtualization engines. So, I'd like to propose implementing a vtable-based interface. We define a structure of all the callback functions a backend can/must provide. Then, the open functions, depending on the URI passed in, will initialize a certain backend and let it populate the vtable with its callbacks. The way, when we start supporting more than just Xen, it'll be a "simple" matter of writing a new backend, providing different functions to the callback vtable. And even before that, the vtable mechanism could be of use to the Xen backend: depending on the locality of the hypervisor (local/distant), the Xen backend could populate the vtable with different callbacks. As an example, take the virDomainSuspend() function. Currently, that function tries to suspend the domain first through xend, then via a direct hypervisor call. As far as I can tell, there is little sense trying both, as if one fails there is a good chance that the second will fail as well. Given that, the initialization of the Xen backend provider could do something along the lines of: int vir__xen_open(vir__vtable_t *vtable) { /* Getting various handles, testing for presence of * xend/hypervisor/xenstore... */ /* Do we have a valid handle to the Xen daemon? */ if (xend_available) { vtable->domain_suspend = vir__xen_daemon_domain_suspend; /* Other xen daemon implementations of functionality assigned here. */ } /* Do we have a valid handle to the hypervisor? * If we do, then some of the functions in the vtable will be * overriden to be handled through direct hypervisor * access. Otherwise, the xend implementations previously defined * remain. */ if (hypervisor_available) { vtable->domain_suspend = vir__xen_hyper_domain_suspend; /* Other hypervisor implementations of functionality assigned here. */ } /* The caller will call a validation function that checks that all * mandatory functions have an implementation defined. */ return 0; } Such a layout would require a little refactoring, but it is far from impossible, and would result in a much cleaner internal structure imho. The problem is if there is good reason to try several methods for accessing Xen data, other than trying to abstract away errors from the library user (not a good idea imo). I'm not aware of any such reason, but if there is one, the vtable mechanism can still be useful for future implementation of other virt mechs, by packing all the xen-specific functionality away in xen*.{c,h} file immediately, leaving a libvirt.c that is as generic as possible (only the open function would have to know about the various actual implementations, and even then only a couple of functions, like vir__xen_can_handle(URI) and vir__xen_open(URI, vtable)). Thoughts? Ah, and while we're on the subject, Anthony told me over IRC that the current codebase is the result of the fusion of two libraries; the previous libvirt, and his libxend. He told me this in reply to my questions about coding style. At the moment, it is very obvious which pieces of code come from which lib. He also said that, given time, both coding styles would eventually merge, but that which of the two current styles would prevail was currently undecided. On the grounds that changing the formatting and calling conventions of the code is easier now, before a lot of features are added, I'd like to propose a patch reformating all the code to conform to a single coding style. Any one of the two existing styles is fine by me, although I'll confess a fondness for the style found in the xend backend, as it is closer to the GNU coding style I use in other projects. The task of converting formatting is just an annoyance, and one I'm willing to undertake if it means we end up with a nicely uniformly coded library. There would also be the task of converting calling conventions and general function style: hypervisor/xenstore funcs return their handle which get inserted in the connection structure in virConnectOpen, whereas the structure is passed into the xend init function and is filled in there; error and sanity checking is done sometimes in the caller, sometimes in the callee; that kind of stuff. So, thoughts on this reformatting, and thoughts on the coding style to adopt if you're okay letting me do the reformatting? There, I think that's enough for one mail ;-) - Dave, going back to preparing his remote-xend patch.

On Fri, Mar 03, 2006 at 09:39:26AM -0600, Anthony Liguori wrote: I agree that code refactoring is needed, but I don't want to go too far before having a second backend (hopefully I will be able to hack QEmu soon enough to build one). I'm always a bit vary of too much abstraction when the code ain't really field tested :-) . But again I totally agree that the current code must be cleaned up, it's just that I would prefer to work toward the implementation phase to then be able to take the right decisions on terms of internal abstract APIs. W.r.t. plugins, this may be a bit too far away from my taste, it's not like there is as many virtualization/emulation frameworks as hardware species supported on Linux, I would rather not make backend APIs public and instead see the code in libvirt. If someone really has a need for extending with proprietary code, it's LGPL so they can fork it legally, but with all associated duties :-) . But anyway let's first try to see if we can drive 2 engines with current APIs and it still makes any sense ;-) Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Sat, Mar 04, 2006 at 02:54:11PM +0100, David Anderson wrote: I'm not fond of GNU style which you suggested. I use paphio:~ -> cat ~/bin/cb #!/bin/sh indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 -nut -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc I asked a couple of time, and since I'm likely to be the one maintaining the code in the long term, I would rather stick to what I'm most used to. For calling conventions what do you refer to ? Even if we don't do things right now, it's fine listing relatively precisely what you would like to get changed, the first patch could be against CVS' TODO to just list those, that would be an useful first step. IMHO indenting is peanuts, it's one ":1,$ !cb" per file. I didn't want to change Anthony's code at that level because I expect him to have patches floating around for the XML-RPC and I would rather reindent after applying his patches than painfully (him or I) go though the merge after a reindentation has occured. Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

* Daniel Veillard <veillard(a)redhat.com&gt; [2006-03-03 04:15:34]: Actually, regarding my previous mail, scratch that. I see that you're a big contributor to the libxml2 library, and that it has functions for parsing URIs. No need to reinvent the wheel in that case, especially if we already link to libxml2. It'll make parsing the virtualizer URI much, much simpler and cleaner! Also, on a completely offtopic note, I see that we must have missed each other at Solutions Linux 2k6. Pity I only got interested in Xen more recently :-). - Dave.