On Thu, Oct 02, 2008 at 06:09:21PM +0100, Daniel P. Berrange wrote:
QEMU has two modes of providing a graphical display, VNC and SDL. Now most of our tools just use VNC, but occasionally people want to use SDL for some crazy reason. We already support this in Xen driver, but the QEMU impl has been rather lacking. At the moment if you ask for a SDL display it'll only happen to work if you had the $DISPLAY environment variable set when you started libvirtd - you probably don't.
Hum, it would be really nicer if everything was driven only by command line options, but i can understand why the QEmu guys made it that way, at least historically
The generic XML parser allows for two attributes on the <graohics> element for setting the display and xauth filename, so this patch updates the QEMU driver to use this data if available. This means we have to start setting environment variables when invoking QEMU, so this patch is a little larger than would otherwise be expected.
Okay i think I understand the patch, i didn't find anything suspicious in the C code diff (except missing -p ?) The tests diff looks simple and systematic, and for the test program this looks fine too. +1
Now previously since we just use 'execv' the QEMU process would just inherit all libvirtd's environment variables. When we now use execve() no variables are inherited - we have to explicitly set all the ones we need. I'm not sure what we should consider the mimimum required?
I'm merely setting 'LC_ALL=C' to ensure it runs in C locale. Do we need to set $PATH for QEMU - maybe ? Anything else which is good practice to set ?
if there is an LD_PRELOAD, pass it maybe, that can be really useful at times especially when debugging. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/