8:26 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:36 a.m.
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 -=|
8:51 a.m.
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
8:57 a.m.
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 -=|
9:26 a.m.
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.
9:40 a.m.
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 -=|
10:55 a.m.
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.
10:37 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
7:23 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:59 a.m.
New subject: [Libvir] [PATCH] default hypervisor selection
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
4:12 p.m.
New subject: [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 -=|
8:32 a.m.
New subject: [Libvir] [PATCH] default hypervisor selection
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
2:34 a.m.
New subject: [Libvir] [PATCH] default hypervisor selection
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
6:45 a.m.
New subject: [Libvir] [PATCH] default hypervisor selection
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 -=|
8:37 a.m.
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(a)us.ibm.com
9:40 a.m.
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
1:13 p.m.
New subject: [Libvir] Re: default hypervisor selection
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
6114
days inactive
6119
days old
16 comments
7 participants
participants (7)
-
Anthony Liguori -
Dan Smith -
Daniel P. Berrange -
Daniel Veillard -
John Levon -
Mark McLoughlin -
Richard W.M. Jones