Re: [libvirt] [Xen-devel] [PATCH] libxl: initialize vfb defbools in libxlMakeVfb
On Wed, 2015-05-06 at 10:24 +0200, Olaf Hering wrote:
On Fri, May 01, Ian Campbell wrote:
> Olaf, please can you use gdb to capture the stack trace so we can fix
> this (and the other issue) properly in libxl instead of just hacking
> around it in libvirt (which might also be appropriate for compat with
> old libxl but shouldn't be done without also fixing libxl IMHO).
The code flow was essentially like this:
libxl_device_vfb_init(libxl);
switch(libvirt->type) {
case SDL:
libxl_defbool_set(libxl->sdl.enable, 1);
break;
case VNC:
libxl_defbool_set(libxl->vnc.enable, 1);
break;
}
if (libvirt->os.type == HVM) {
if (libxl_defbool_val(libxl->vnc.enable)) {
/* do VNC things */
} else if (libxl_defbool_val(libxl->sdl.enable)) {
/* do SDL things */
if (libxl_defbool_val(libxl->opengl.enable))
/* do openGL things */
}
}
The first crash was because I had SDL enabled, and the SDL case did not
initialize the defbool for VNC. Once it did the next crash was the
openGL part which was not initialized either.
I see nothing wrong with libxl in such usage.
I've explained this repeatedly now, it is a bug in libxl if the above
ends up crashing in libxl.
It is always a *libxl bug* for a libxl code path to lead a use of
libxl_defbool_val on a value which has not had the appropriate
setdefault called on it *by libxl*. It is not required for the user of
libxl to initialise any defbool (other than via the libxl_TYPE_init
function, which should set it to the explicit "default" value).
It is therefore a bug if libxl reaches this code without having ensured
that libxl_defbool_setdefault has been called *by libxl* on
libxl->opengl.enable (perhaps on if libxl->sdl.enable is true).
I see no code in libxl which matches what you have above, the only call
to libxl_device_vfb_init has no switch statement or if == HVM anywhere
near it.
Please provide the actual stack trace as requested so we can see which
code path in libxl is failing to properly initialise the defbools.
Ian.