* Daniel Veillard <veillard@xxxxxxxxxx> [2006-03-03 04:15:34]:
> On Fri, Mar 03, 2006 at 01:27:09AM +0100, David Anderson wrote:
> > * Anthony Liguori <aliguori@xxxxxxxxxx> [2006-03-02 18:05:58]:
> >
> > > I would think changing virConnectOpen(const char *) to
> > > virConnectOpen(const char *host, int port) and have port default to
> > > something sane if say 0 is specified would be nice.
> >
> > Okay. And when we start supporting other virtualization methods, we
> > can rev that API to add a 'type' param.
>
> Hum, I would rather not change the API in the future. I really
> expected the parameter to Open to be the type selector too.
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.
> In a conference ATM, I will have a bit more time starting tomorrow
> to work on the issues raised.
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.