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

Re: [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 redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


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