[Libvir] default hypervisor selection
While going though the ovirt installation steps I got annoyed with the need to add "-c qemu:///system" each time we are trying to start a virsh command if using KVM. The dfault setup is basically to assume a Xen hypervisor if the URI is not specified, this is glued in the do_open internal opening routine associated to the opening of connections: -------------------------------------- static virConnectPtr do_open (const char *name, virConnectAuthPtr auth, int flags) { int i, res; virConnectPtr ret = NULL; xmlURIPtr uri; /* Convert NULL or "" to xen:/// for back compat */ if (!name || name[0] == '\0') name = "xen:///"; -------------------------------------- In one hand this is the default behaviour of the library, but in my opinion it's not very smart. We should be able to be smarter than that, for example: - if /proc/xen doesn't exist (on linux, or /dev/xen on Solaris) well we should not do that we are pretty sure we will get an error when trying to connect - if /proc/vz is present, well it's very likely that if the kernel has been compiled with OpenVZ support, it's likely to be used as the default virtualization - if there is a kvm module loaded well we should probably use qemu:///system if running as root or qemu:///session otherwise I guess on Solaris an easy heuristic would allow to pick the right hypervisor by default too. At some point we may have multiple hypervisor support simultaneously on a linux system thanks to pv_ops, but right now it doesn't make too much sense to force a default Xen connection even when we know it won't work. For a virsh specific solution there is the VIRSH_DEFAULT_CONNECT_URI environment variable, but it's not really user friendly and not very generic, What do people think ? I would be tempted to provide a patch to change do_open() behaviour on linux in the case name is NULL or "", and then check what hypervisor might be present and running, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, Feb 21, 2008 at 09:26:38AM -0500, Daniel Veillard wrote:
While going though the ovirt installation steps I got annoyed with the need to add "-c qemu:///system" each time we are trying to start a virsh command if using KVM. The dfault setup is basically to assume a Xen hypervisor if the URI is not specified, this is glued in the do_open internal opening routine associated to the opening of connections:
-------------------------------------- static virConnectPtr do_open (const char *name, virConnectAuthPtr auth, int flags) { int i, res; virConnectPtr ret = NULL; xmlURIPtr uri;
/* Convert NULL or "" to xen:/// for back compat */ if (!name || name[0] == '\0') name = "xen:///"; --------------------------------------
In one hand this is the default behaviour of the library, but in my opinion it's not very smart. We should be able to be smarter than that, for example:
- if /proc/xen doesn't exist (on linux, or /dev/xen on Solaris) well we should not do that we are pretty sure we will get an error when trying to connect - if /proc/vz is present, well it's very likely that if the kernel has been compiled with OpenVZ support, it's likely to be used as the default virtualization - if there is a kvm module loaded well we should probably use qemu:///system if running as root or qemu:///session otherwise
I guess on Solaris an easy heuristic would allow to pick the right hypervisor by default too. At some point we may have multiple hypervisor support simultaneously on a linux system thanks to pv_ops, but right now it doesn't make too much sense to force a default Xen connection even when we know it won't work. For a virsh specific solution there is the VIRSH_DEFAULT_CONNECT_URI environment variable, but it's not really user friendly and not very generic,
I've proposed that we make this generic : http://www.redhat.com/archives/libvir-list/2008-January/msg00234.html I never got around to fixing the patch to also address the virsh issue Soren raised, but I think we should do this env var.
What do people think ? I would be tempted to provide a patch to change do_open() behaviour on linux in the case name is NULL or "", and then check what hypervisor might be present and running,
I think it is worthwhile - the 'default to Xen' behavious is a major cause of pain for people initially using libvirt, particularly since KVM is becoming the defacto standard for Linux platforms. I'd see the following levels of customization: - If a non-NULL URI is passed in virConnectOpen, use that - Else if LIBVIRT_DEFAULT_URI is set use that URI - Else probe each registered driver in order until one succeeds For the latter I think we could add a 'probe' method to the internal driver API table. Then we can just call 'probe' on each driver in turn until we find one which is available on the system. At the same time it could be worth having a public API to 'detect drivers' which will call probe for each driver and return a list of all drivers which are available. This allows an app to easily ask libvirt what it supports - because long term we'll definitely get hosts supporting many drivers at once. We can also advertise this list of supported drivers using the Avahi mDNS broadcasts we do from the remote daemon. Regards, 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 -=|
I'm mostly with Dan Berrange here. Surely an environment variable is the right thing to do, and then later we can add some advanced probing if the environment variable alone doesn't satisfy users. The only problem I see is making the NULL case unpredictable or introducing unreproducible bugs. But I guess that's not very likely. Dan's patch (http://www.redhat.com/archives/libvir-list/2008-January/msg00234.html) is fine except it should include a DEBUG() statement so we have some chance to gather information from bug reports about what it was actually trying to connect to. At the moment, the first DEBUG() isn't done until after the URI is parsed, and URI parsing is quite likely to be a failure point. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Thu, Feb 21, 2008 at 02:51:20PM +0000, Richard W.M. Jones wrote:
I'm mostly with Dan Berrange here. Surely an environment variable is the right thing to do, and then later we can add some advanced probing if the environment variable alone doesn't satisfy users.
The only problem I see is making the NULL case unpredictable or introducing unreproducible bugs. But I guess that's not very likely.
I agree - possible, but not likely too bad - I think we'll easily have a net win thanks to a more pleasant default end user experiance.
Dan's patch (http://www.redhat.com/archives/libvir-list/2008-January/msg00234.html) is fine except it should include a DEBUG() statement so we have some chance to gather information from bug reports about what it was actually trying to connect to. At the moment, the first DEBUG() isn't done until after the URI is parsed, and URI parsing is quite likely to be a failure point.
Actually in the PolicyKit fix I committed yesterday, I moved the DEBUG statement into the virConnectOpen() method so we should see the URI printed before any processing is done. 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 -=|
On Thu, 2008-02-21 at 14:57 +0000, Daniel P. Berrange wrote:
On Thu, Feb 21, 2008 at 02:51:20PM +0000, Richard W.M. Jones wrote:
I'm mostly with Dan Berrange here. Surely an environment variable is the right thing to do, and then later we can add some advanced probing if the environment variable alone doesn't satisfy users.
The only problem I see is making the NULL case unpredictable or introducing unreproducible bugs. But I guess that's not very likely.
I agree - possible, but not likely too bad - I think we'll easily have a net win thanks to a more pleasant default end user experiance.
(Re-suggesting something that was shot down before just in case it makes more sense to people this time around :-) I've never liked the fact that the individual drivers are exposed in the API and to the user. IMHO, the default behaviour for open(NULL), virsh, virt-manager etc. should be to talk to *all* drivers and aggregate the results. When you define a VM, that's the only time you should care about Xen vs. KVM etc. After that, it should just be a question of referring to a VM by name. The only time you should really need to specify a URI is when connecting to a remote host, IMHO. Adding a heuristic to select sensible driver by default won't help someone running e.g. KVM VMs and linux containers on the same host, which I don't think is such a crazy notion. Yes, you'd be trying to merge overlapping namespaces, but some ideas on that front: - If you simply prevent someone defining a VM using the same name as an existing VM defined via another driver, that gets you 90% there - Never aggregate a user's private namespace with the system namespace - e.g. for a normal user, an open(NULL) connection would only show the user's own VMs and we'd have another URI (or e.g. a virsh --system switch) for dealing with system-level VMs - If someone connects directly to a driver, or goes behind libvirt's back, and creates a VM with conflicting names, then just return an error if someone tries to perform an operation using the conflicting name ... and force the user to again connect to the specific driver or go behind libvirt's back - Also, figure out some way to deprecate the VM "id" attribute which is the most obvious point of conflict ... name and uuid should be enough identifiers, surely Cheers, Mark.
On Thu, Feb 21, 2008 at 03:26:08PM +0000, Mark McLoughlin wrote:
On Thu, 2008-02-21 at 14:57 +0000, Daniel P. Berrange wrote:
On Thu, Feb 21, 2008 at 02:51:20PM +0000, Richard W.M. Jones wrote:
I'm mostly with Dan Berrange here. Surely an environment variable is the right thing to do, and then later we can add some advanced probing if the environment variable alone doesn't satisfy users.
The only problem I see is making the NULL case unpredictable or introducing unreproducible bugs. But I guess that's not very likely.
I agree - possible, but not likely too bad - I think we'll easily have a net win thanks to a more pleasant default end user experiance.
(Re-suggesting something that was shot down before just in case it makes more sense to people this time around :-)
I've never liked the fact that the individual drivers are exposed in the API and to the user. IMHO, the default behaviour for open(NULL), virsh, virt-manager etc. should be to talk to *all* drivers and aggregate the results.
When you define a VM, that's the only time you should care about Xen vs. KVM etc. After that, it should just be a question of referring to a VM by name.
The only time you should really need to specify a URI is when connecting to a remote host, IMHO.
Adding a heuristic to select sensible driver by default won't help someone running e.g. KVM VMs and linux containers on the same host, which I don't think is such a crazy notion.
Applications can explicitly connect to multiple drivers as desired.
Yes, you'd be trying to merge overlapping namespaces, but some ideas on that front:
- If you simply prevent someone defining a VM using the same name as an existing VM defined via another driver, that gets you 90% there
Which we're unable to prevent. We are fundamentally dealing with different namespaces with both 'name' and 'id' values - any attempt to merge them is doomed to failure. The only globally unique identifier we have is UUID.
- Never aggregate a user's private namespace with the system namespace - e.g. for a normal user, an open(NULL) connection would only show the user's own VMs and we'd have another URI (or e.g. a virsh --system switch) for dealing with system-level VMs
- If someone connects directly to a driver, or goes behind libvirt's back, and creates a VM with conflicting names, then just return an error if someone tries to perform an operation using the conflicting name ... and force the user to again connect to the specific driver or go behind libvirt's back
The name namespace is not under libvirt's control, and is fundamentally going to clash between virtualization backends. We merely have illusion of control wrt to QEMU because currently all QEMU guests are directly managed by libvirt - even this won't neccessarily be true long term if we implement the ability to manage externally started QEMU instances.
- Also, figure out some way to deprecate the VM "id" attribute which is the most obvious point of conflict ... name and uuid should be enough identifiers, surely
The ID number is very convenient because it is short & simple and unique over the lifetime of a VM. The name is also unique, but name can change during the lifetime of a VM. 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 -=|
On Thu, 2008-02-21 at 15:40 +0000, Daniel P. Berrange wrote:
Yes, you'd be trying to merge overlapping namespaces, but some ideas on that front:
- If you simply prevent someone defining a VM using the same name as an existing VM defined via another driver, that gets you 90% there
Which we're unable to prevent.
I was talking about the trivial case - prevent a user from creating a conflict using the default connection. That gets us the right thing in at least 90% of use cases IMHO.
We are fundamentally dealing with different namespaces with both 'name' and 'id' values - any attempt to merge them is doomed to failure.
Yes, they are fundamentally different namespaces. But the fact that they are fundamentally different namespaces is nothing but a hinderance to users, and fudging an aggregation of these fundamentally different namespaces should be both possible and beneficial to most users. Cheers, Mark.
On Thu, Feb 21, 2008 at 02:36:10PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 21, 2008 at 09:26:38AM -0500, Daniel Veillard wrote:
What do people think ? I would be tempted to provide a patch to change do_open() behaviour on linux in the case name is NULL or "", and then check what hypervisor might be present and running,
I think it is worthwhile - the 'default to Xen' behavious is a major cause of pain for people initially using libvirt, particularly since KVM is becoming the defacto standard for Linux platforms. I'd see the following levels of customization:
- If a non-NULL URI is passed in virConnectOpen, use that - Else if LIBVIRT_DEFAULT_URI is set use that URI - Else probe each registered driver in order until one succeeds
For the latter I think we could add a 'probe' method to the internal driver API table. Then we can just call 'probe' on each driver in turn until we find one which is available on the system.
Yup, that's even better than ad-hoc attempt at detecting, it really should go in the driver.
At the same time it could be worth having a public API to 'detect drivers' which will call probe for each driver and return a list of all drivers which are available. This allows an app to easily ask libvirt what it supports - because long term we'll definitely get hosts supporting many drivers at once. We can also advertise this list of supported drivers using the Avahi mDNS broadcasts we do from the remote daemon.
That can be done in a second step, less urgent IMHO but useful too, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, Feb 21, 2008 at 02:36:10PM +0000, Daniel P. Berrange wrote:
- Else probe each registered driver in order until one succeeds
For the latter I think we could add a 'probe' method to the internal driver API table. Then we can just call 'probe' on each driver in turn until we find one which is available on the system.
At the same time it could be worth having a public API to 'detect drivers' which will call probe for each driver and return a list of all drivers which are available. This allows an app to easily ask libvirt what it supports
I actually started to look at that and while it sounds good 'on paper' it's a bit messy in practice, for example: - test driver: if compiled in, you would think the probe should return true, but as a result if trying an operation whyle no hypervisor is available you would end up doing it on the test driver instead of returning a failure, annoying - qemu driver: well QEmu is actually a driver for a potentially large set, KVM, QEmu for the current architecture, QEmu for emulated architectures. Would the probe return true if it finds /usr/bin/qemu-mips ? Or the KVM package may be present but would not be usable because the current kernel doesn't have a kvm module Trying to be a bit systematic purely based on the driver model proves harder than expected, still worth trying but I prefer early feedback :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, Feb 22, 2008 at 08:23:43AM -0500, Daniel Veillard wrote:
On Thu, Feb 21, 2008 at 02:36:10PM +0000, Daniel P. Berrange wrote:
- Else probe each registered driver in order until one succeeds
For the latter I think we could add a 'probe' method to the internal driver API table. Then we can just call 'probe' on each driver in turn until we find one which is available on the system.
At the same time it could be worth having a public API to 'detect drivers' which will call probe for each driver and return a list of all drivers which are available. This allows an app to easily ask libvirt what it supports
I actually started to look at that and while it sounds good 'on paper' it's a bit messy in practice, for example: - test driver: if compiled in, you would think the probe should return true, but as a result if trying an operation whyle no hypervisor is available you would end up doing it on the test driver instead of returning a failure, annoying - qemu driver: well QEmu is actually a driver for a potentially large set, KVM, QEmu for the current architecture, QEmu for emulated architectures. Would the probe return true if it finds /usr/bin/qemu-mips ? Or the KVM package may be present but would not be usable because the current kernel doesn't have a kvm module
Trying to be a bit systematic purely based on the driver model proves harder than expected, still worth trying but I prefer early feedback :-)
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 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
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 -=|
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@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Feb 25, 2008 at 09:32:48AM -0500, Daniel Veillard wrote:
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,
Okay, commited, seems to work well for me as root and as normal user on RHEL-5.1 , but on Fedora-8 as non-root this doesn't work the debug shows DEBUG: libvirt.c: do_open (Probed qemu:///session) DEBUG: libvirt.c: do_open (Using qemu:///session as default URI, 1 hypervisor found) DEBUG: libvirt.c: do_open (name "qemu:///session" to URI components: [...] DEBUG: libvirt.c: do_open (trying driver 1 (QEMU) ...) DEBUG: libvirt.c: do_open (driver 1 QEMU returned DECLINED) The problem seems to be that in qemudOpen at that point qemu_driver is NULL, and we return VIR_DRV_OPEN_DECLINED immediately as a result. When we end up in the remote driver I see DEBUG: remote_internal.c: doRemoteOpen (proceeding with name = qemu:///session?) DEBUG: remote_internal.c: remoteAuthPolkit (Client initialize PolicyKit authentication) Attempting to gain the privilege for org.libvirt.unix.monitor. polkit-grant-helper: given auth type (8 -> yes) is bogus Failed to gain the privilege for org.libvirt.unix.monitor. libvir: Remote error : authentication failed but I didn't got any option to authenticate at that point. virsh -c qemu:///session --readonly list also fails in a similar way, but virsh -c qemu:///system --readonly list succeeds. There is something strange in the processing of qemu:///session in my Fedora 8 (libvirt-0.4.0-4.fc8 , daemon running) I will try to understand the problem, it would be good to have this fixed before pushing the next release, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Tue, Feb 26, 2008 at 03:34:56AM -0500, Daniel Veillard wrote:
Okay, commited, seems to work well for me as root and as normal user on RHEL-5.1 , but on Fedora-8 as non-root this doesn't work the debug shows
DEBUG: libvirt.c: do_open (Probed qemu:///session) DEBUG: libvirt.c: do_open (Using qemu:///session as default URI, 1 hypervisor found) DEBUG: libvirt.c: do_open (name "qemu:///session" to URI components: [...] DEBUG: libvirt.c: do_open (trying driver 1 (QEMU) ...) DEBUG: libvirt.c: do_open (driver 1 QEMU returned DECLINED)
The problem seems to be that in qemudOpen at that point qemu_driver is NULL, and we return VIR_DRV_OPEN_DECLINED immediately as a result.
This is intentional - the QEMU driver should only ever be invoked from within the context of the daemon. Hence qemu_driver will always be NULL when accessed from the standalone library - it'll be initialized when the daemon invokes the private __virSTateInitialize method.
When we end up in the remote driver I see
DEBUG: remote_internal.c: doRemoteOpen (proceeding with name = qemu:///session?) DEBUG: remote_internal.c: remoteAuthPolkit (Client initialize PolicyKit authentication) Attempting to gain the privilege for org.libvirt.unix.monitor. polkit-grant-helper: given auth type (8 -> yes) is bogus Failed to gain the privilege for org.libvirt.unix.monitor. libvir: Remote error : authentication failed
but I didn't got any option to authenticate at that point.
This looks like a bug - I think the session daemon is mistakenly asking for auth, when none is neccessary because it runs as same UID. 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 -=|
DV> What do people think ? I would be tempted to provide a patch to DV> change do_open() behaviour on linux in the case name is NULL or DV> "", and then check what hypervisor might be present and running, This has been bugging me recently as well, so I'm definitely in favor of a change. Is there any reason not to also allow setting the default URI in an environment variable to solve the dual-hypervisor problem? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Thu, Feb 21, 2008 at 09:26:38AM -0500, Daniel Veillard wrote:
I guess on Solaris an easy heuristic would allow to pick the right hypervisor by default too.
/dev/xen isn't quite right as it can exist even when not booted into dom0. # cat /dev/xen/domcaps control_d This should do it, though. regards john
Daniel Veillard wrote:
- if /proc/xen doesn't exist (on linux, or /dev/xen on Solaris) well we should not do that we are pretty sure we will get an error when trying to connect - if /proc/vz is present, well it's very likely that if the kernel has been compiled with OpenVZ support, it's likely to be used as the default virtualization - if there is a kvm module loaded well we should probably use qemu:///system if running as root or qemu:///session otherwise
While I definitely like the direction this thread is going in, I'd just warn that this sort of probing can be equally as harmful as the current behavior. While Xen and KVM are mutually exclusive, the same is not true with OpenVZ/Linux Containers. While it may be unlikely that both are actively being used, I find it very likely that in future distributions, both features will be present. So you may probe that this is a Linux Containers capable host, but you may in fact be intending on using KVM for virtualization (and vice versa). Regards, Anthony Liguori
I guess on Solaris an easy heuristic would allow to pick the right hypervisor by default too. At some point we may have multiple hypervisor support simultaneously on a linux system thanks to pv_ops, but right now it doesn't make too much sense to force a default Xen connection even when we know it won't work. For a virsh specific solution there is the VIRSH_DEFAULT_CONNECT_URI environment variable, but it's not really user friendly and not very generic,
What do people think ? I would be tempted to provide a patch to change do_open() behaviour on linux in the case name is NULL or "", and then check what hypervisor might be present and running,
Daniel
participants (7)
-
Anthony Liguori -
Dan Smith -
Daniel P. Berrange -
Daniel Veillard -
John Levon -
Mark McLoughlin -
Richard W.M. Jones