[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] default hypervisor selection



On Fri, Feb 22, 2008 at 10:59:43AM -0500, Daniel Veillard wrote:
>   Okay, first patch enclosed, it seems to work for me:
>     - grow the internal driver adding a 
>       const char *probe(void)
>       entry point, it returns the URI to use to access the driver
>       I did a bit of reformating of driver.h to maintain alignment of fields
>     - define the entry point for OpenVZ, test, Xen (probe is arch specific
>       as suggested for Solaris), and QEmu. In the last case I just checked
>       /usr/bin/qemu and /usr/bin/qemu-kvm presence, and if non root
>       return the session URI instead of the system one.
>     - in do_open check first for LIBVIRT_DEFAULT_URI, if still empty,
>       then go though the probes, ignore the test driver to avoid surprises
>       and if Xen is found give it priority compared to others to maintain
>       compatibility with previous behaviour
>     - added a virFileExists to util.[ch] keeping code clean

> Index: src/libvirt.c
> ===================================================================
> RCS file: /data/cvs/libxen/src/libvirt.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 libvirt.c
> --- src/libvirt.c	20 Feb 2008 16:54:36 -0000	1.123
> +++ src/libvirt.c	22 Feb 2008 15:42:27 -0000
> @@ -632,9 +632,47 @@ do_open (const char *name,
>      virConnectPtr ret = NULL;
>      xmlURIPtr uri;
>  
> -    /* Convert NULL or "" to xen:/// for back compat */
> -    if (!name || name[0] == '\0')
> -        name = "xen:///";
> +    /*
> +     *  If no URI is passed, then check for an environment string if not
> +     *  available probe the compiled in drivers to find a default hypervisor
> +     *  if detectable.
> +     */
> +    if (!name || name[0] == '\0') {
> +        char *defname = getenv("LIBVIRT_DEFAULT_URI");
> +        if (defname && *defname) {
> +	    DEBUG("Using LIBVIRT_DEFAULT_URI %s", defname);
> +            name = defname;
> +        } else {
> +	    const char *use = NULL;
> +	    const char *latest;
> +	    int probes = 0;
> +	    for (i = 0; i < virNetworkDriverTabCount; i++) {
> +	        if ((virDriverTab[i]->probe != NULL) &&
> +		    ((latest = virDriverTab[i]->probe()) != NULL)) {
> +		    probes++;
> +
> +		    DEBUG("Probed %s", latest);
> +		    /*
> +		     * 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.

> +		    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.

> +		}
> +	    }
> +	    if (use == NULL) {
> +		name = "xen:///";
> +	        DEBUG("Could not probe any hypervisor defaulting to %s",
> +		      name);
> +	    } else {
> +		name = use;
> +	        DEBUG("Using %s as default URI, %d hypervisor found",
> +		      use, probes);
> +	    }
> +	}
> +    }
>  
>      /* Convert xen -> xen:/// for back compat */
>      if (!strcasecmp(name, "xen"))
> Index: src/openvz_driver.c
> ===================================================================
> RCS file: /data/cvs/libxen/src/openvz_driver.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 openvz_driver.c
> --- src/openvz_driver.c	5 Feb 2008 19:27:37 -0000	1.15
> +++ src/openvz_driver.c	22 Feb 2008 15:42:27 -0000
> @@ -546,6 +546,15 @@ bail_out5:
>      return ret;
>  }
>  
> +static const char *openvzProbe(void)
> +{
> +#ifdef __linux__
> +    if ((getuid() == 0) && (virFileExists("/proc/vz")))
> +        return("openvz:///");
> +#endif
> +    return(NULL);
> +}
> +
>  static virDrvOpenStatus openvzOpen(virConnectPtr conn,
>                                   xmlURIPtr uri,
>                                   virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> @@ -694,6 +703,7 @@ static virDriver openvzDriver = {
>      VIR_DRV_OPENVZ,
>      "OPENVZ",
>      LIBVIR_VERSION_NUMBER,
> +    openvzProbe, /* probe */
>      openvzOpen, /* open */
>      openvzClose, /* close */
>      NULL, /* supports_feature */
> Index: src/qemu_driver.c
> ===================================================================
> RCS file: /data/cvs/libxen/src/qemu_driver.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 qemu_driver.c
> --- src/qemu_driver.c	7 Feb 2008 16:50:17 -0000	1.52
> +++ src/qemu_driver.c	22 Feb 2008 15:42:27 -0000
> @@ -1386,6 +1386,24 @@ static int qemudMonitorCommand(struct qe
>      return -1;
>  }
>  
> +/**
> + * 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 :-)

>  
>  static virDrvOpenStatus qemudOpen(virConnectPtr conn,
>                                    xmlURIPtr uri,
> @@ -2857,6 +2875,7 @@ static virDriver qemuDriver = {
>      VIR_DRV_QEMU,
>      "QEMU",
>      LIBVIR_VERSION_NUMBER,
> +    qemudProbe, /* probe */
>      qemudOpen, /* open */
>      qemudClose, /* close */
>      NULL, /* supports_feature */


> Index: src/test.c
> ===================================================================
> RCS file: /data/cvs/libxen/src/test.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 test.c
> --- src/test.c	20 Feb 2008 15:53:34 -0000	1.65
> +++ src/test.c	22 Feb 2008 15:42:27 -0000
> @@ -180,6 +180,17 @@ testError(virConnectPtr con,
>                      errmsg, info, NULL, 0, 0, errmsg, info, 0);
>  }
>  
> +/**
> + * testProbe:
> + *
> + * Probe for the availability of the test driver
> + */
> +static const char *
> +testProbe(void)
> +{
> +    return("test:///");
> +}
> +

IMHO, don't add this - we shouldn't ever try to use the test driver
except in test suites.

>  static int testRestartStringToFlag(const char *str) {
>      if (!strcmp(str, "restart")) {
>          return VIR_DOMAIN_RESTART;
> @@ -1938,6 +1949,7 @@ static virDriver testDriver = {
>      VIR_DRV_TEST,
>      "Test",
>      LIBVIR_VERSION_NUMBER,
> +    testProbe, /* probe */
>      testOpen, /* open */
>      testClose, /* close */
>      NULL, /* supports_feature */
> Index: src/xen_unified.c
> ===================================================================
> RCS file: /data/cvs/libxen/src/xen_unified.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 xen_unified.c
> --- src/xen_unified.c	7 Feb 2008 09:37:10 -0000	1.36
> +++ src/xen_unified.c	22 Feb 2008 15:42:27 -0000
> @@ -39,6 +39,7 @@
>  #include "xs_internal.h"
>  #include "xm_internal.h"
>  #include "xml.h"
> +#include "util.h"
>  
>  #define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt,__VA_ARGS__)
>  
> @@ -217,6 +218,24 @@ done:
>   * in the low level drivers directly.
>   */
>  
> +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.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]