Re: [Libvir] [PATCH] default hypervisor selection

2008-02-26 Thread Daniel Veillard 
On Mon, Feb 25, 2008 at 09:32:48AM -0500, Daniel Veillard wrote:
 On Sun, Feb 24, 2008 at 10:12:05PM +, 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/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [Libvir] [PATCH] default hypervisor selection

2008-02-26 Thread Daniel P. Berrange 
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  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [Libvir] [PATCH] default hypervisor selection

2008-02-25 Thread Daniel Veillard 
On Sun, Feb 24, 2008 at 10:12:05PM +, 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/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [Libvir] [PATCH] default hypervisor selection

2008-02-24 Thread Daniel P. Berrange 
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 -  1.123
 +++ src/libvirt.c 22 Feb 2008 15:42:27 -
 @@ -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 -   1.15
 +++ src/openvz_driver.c   22 Feb 2008 15:42:27 -
 @@ -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 -   1.52
 +++ src/qemu_driver.c 22 Feb 2008 15:42:27 -
 @@ -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