[Libvir] [PATCH] default hypervisor selection

[Daniel Veillard]

On Sun, Feb 24, 2008 at 10:12:05PM +0000, Daniel P. Berrange wrote:
> On Fri, Feb 22, 2008 at 10:59:43AM -0500, Daniel Veillard wrote:
> >   Okay, first patch enclosed, it seems to work for me:
> > +		    /*
> > +		     * if running a xen kernel, give it priority over
> > +		     * QEmu emultation
> > +		     */
> > +		    if (STREQ(latest, "xen:///")) 
> > +		        use = latest;
> 
> If we edit virInitialize() to make 'xenUnifiedRegister' run before the
> call to 'qemudRegister', then we won't need this check, since the Xen
> driver would get probed ahead of the QEMU driver.

  Honnestly I prefer to keep the test in than base the behaviour purely
on the order of the drivers, it exposes the intent clearly, and it's not
like it's a timely critical operation :-)

> > +		    else if ((use == NULL) && (!STREQ(latest, "test:///")))
> > +		        use = latest;
> 
> IMHO, remove the !STREQ(latest, "test:///") and get rid of the 'probe' impl
> for the test driver. The test driver will always succeed, but should
> only ever be used for test suites, never real world deployment, so
> we shouldn't probe it.

  Okay, i wondered if i should keep the test probe or not, and though it would
still be useful if we were to expose the list of drivers from a library API
as you suggested too. But yeah with the current code, let's get rid of it,

[...]
> > +/**
> > + * qemudProbe:
> > + *
> > + * Probe for the availability of the qemu driver, assume the
> > + * presence of QEmu emulation if the binaries are installed
> > + */
> > +static const char *qemudProbe(void)
> > +{
> > +    if ((virFileExists("/usr/bin/qemu")) ||
> > +        (virFileExists("/usr/bin/qemu-kvm"))) {
> > +        if (getuid() == 0) {
> > +	    return("qemu:///system");
> > +	} else {
> > +	    return("qemu:///session");
> > +	}
> > +    }
> > +    return(NULL);
> > +}
> 
> Should add a check for '/usr/bin/xenner' too, which is Gerd's
> Xen compatability layer for the QEMU driver :-)

  Okidoc :-)

[...]
> > +static const char *
> > +xenUnifiedProbe (void)
> > +{
> > +#ifdef __linux__
> > +    if (virFileExists("/proc/xen"))
> > +        return("xen:///");
> > +#endif
> > +#ifdef __sun__
> > +    FILE *fh;
> > +
> > +    if (fh = fopen(path, "r")) {
> > +	fclose(fh);
> > +        return("xen:///");
> > +    }
> > +#endif
> 
> AFAICT this won't compile on Solaris since 'path' isn't defined.

 Oops it's fopen("/dev/xen/domcaps", "r") as John suggested,
I will commit with those fixes later today unless someone has an objection,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/