[libvirt] [PATCH] qemu: Reject SDL graphic if it's not supported by qemu
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently. * src/qemu/qemu_command.c --- src/qemu/qemu_command.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7dd8e03..cf4ae01 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3567,6 +3567,14 @@ qemuBuildCommandLine(virConnectPtr conn, } } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_SDL)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("sdl not supported by '%s'"), + def->emulator); + + goto error; + } + if (def->graphics[0]->data.sdl.xauth) virCommandAddEnvPair(cmd, "XAUTHORITY", def->graphics[0]->data.sdl.xauth); @@ -3586,9 +3594,7 @@ qemuBuildCommandLine(virConnectPtr conn, /* New QEMU has this flag to let us explicitly ask for * SDL graphics. This is better than relying on the * default, since the default changes :-( */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL) - virCommandAddArg(cmd, "-sdl"); - + virCommandAddArg(cmd, "-sdl"); } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { virBuffer opt = VIR_BUFFER_INITIALIZER; -- 1.7.3.2
On Tue, Jan 11, 2011 at 03:43:07PM +0800, Osier Yang wrote:
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently.
* src/qemu/qemu_command.c
NACK. Most previous versions of QEMU don't have any explicit -sdl flag. You got SDL automagically when *no* -vnc flag was given. So this change breaks many old QEMU versions.
--- src/qemu/qemu_command.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7dd8e03..cf4ae01 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3567,6 +3567,14 @@ qemuBuildCommandLine(virConnectPtr conn, } } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_SDL)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("sdl not supported by '%s'"), + def->emulator); + + goto error; + } + if (def->graphics[0]->data.sdl.xauth) virCommandAddEnvPair(cmd, "XAUTHORITY", def->graphics[0]->data.sdl.xauth); @@ -3586,9 +3594,7 @@ qemuBuildCommandLine(virConnectPtr conn, /* New QEMU has this flag to let us explicitly ask for * SDL graphics. This is better than relying on the * default, since the default changes :-( */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL) - virCommandAddArg(cmd, "-sdl"); - + virCommandAddArg(cmd, "-sdl"); } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { virBuffer opt = VIR_BUFFER_INITIALIZER;
Daniel
于 2011年01月11日 19:29, Daniel P. Berrange 写道:
On Tue, Jan 11, 2011 at 03:43:07PM +0800, Osier Yang wrote:
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently.
* src/qemu/qemu_command.c
NACK.
Most previous versions of QEMU don't have any explicit -sdl flag. You got SDL automagically when *no* -vnc flag was given. So this change breaks many old QEMU versions.
I didn't known the older versions of QEMU don't have explicit -sdl flag before, thanks for the knowledge, but the problem is that qemu-kvm of RHEL removed sdl support. * Fri Jan 15 2010 Eduardo Habkost <ehabkost@redhat.com> - qemu-kvm-0.12.1.2-2.5.el6 - Remove unneeded/unsupported features: [bz#555336] - make default options explicit - remove sdl support So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream. IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine. Any good idea? Thanks Regards Osier
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ... Been discussed with upstream qemu many times and no movement on it. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
On 12/01/2011, at 12:39 AM, Richard W.M. Jones wrote:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Is it feasible (or even useful) to create a library or utility called "qemu-capabilities", who's sole purpose is to interrogate a qemu installation and then provide us a list of its capabilities?
于 2011年01月11日 22:01, Justin Clift 写道:
On 12/01/2011, at 12:39 AM, Richard W.M. Jones wrote:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Is it feasible (or even useful) to create a library or utility called "qemu-capabilities", who's sole purpose is to interrogate a qemu installation and then provide us a list of its capabilities?
Just saw Dan has sent a patch to upstream qemu before, but seems it's not got pushed yet. http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00940.html Regards Osier
On Wed, Jan 12, 2011 at 01:01:25AM +1100, Justin Clift wrote:
On 12/01/2011, at 12:39 AM, Richard W.M. Jones wrote:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Is it feasible (or even useful) to create a library or utility called "qemu-capabilities", who's sole purpose is to interrogate a qemu installation and then provide us a list of its capabilities?
This doesn't really solve anything. libvirt already has such a "library", it is simply part of oura internal API. The problem is that QEMU itself needs to expose information that does not currently exist clearly anywhere. Daniel
于 2011年01月11日 21:39, Richard W.M. Jones 写道:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Rich.
Even qemu support capabilities, I guess it also need to be backported to older versions of qemu. Otherwise we still have the problem. Regards Osier
On Tue, Jan 11, 2011 at 10:03:42PM +0800, Osier Yang wrote:
于 2011年01月11日 21:39, Richard W.M. Jones 写道:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Rich.
Even qemu support capabilities, I guess it also need to be backported to older versions of qemu. Otherwise we still have the problem.
It's not so much a problem for RHEL, since there are only a handful of versions of qemu for RHEL and we could just build a big table of precise version -> features supported. It's a bunch of unnecessary effort, that's all. But this doesn't scale for every version of qemu, and won't even work if you consider qemu that people have compiled for themselves. So we need to fix this for _future_ versions and continue with the current hacks for older versions. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
于 2011年01月11日 22:25, Richard W.M. Jones 写道:
On Tue, Jan 11, 2011 at 10:03:42PM +0800, Osier Yang wrote:
于 2011年01月11日 21:39, Richard W.M. Jones 写道:
On Tue, Jan 11, 2011 at 09:21:26PM +0800, Osier Yang wrote:
So, it means the newer qemu-kvm (of RHEL) doesn't support sdl anymore. Didn't send the patch to Red Hat internal list, as the patch won't conflict with upstream.
IMHO it's not reasonable that we have "sdl" xml in domain config, but no according command line of qemu, and pretend everything is fine.
Any good idea? Thanks
qemu needs to support capabilities ...
Been discussed with upstream qemu many times and no movement on it.
Rich.
Even qemu support capabilities, I guess it also need to be backported to older versions of qemu. Otherwise we still have the problem.
It's not so much a problem for RHEL, since there are only a handful of versions of qemu for RHEL and we could just build a big table of precise version -> features supported. It's a bunch of unnecessary effort, that's all.
But this doesn't scale for every version of qemu, and won't even work if you consider qemu that people have compiled for themselves.
So we need to fix this for _future_ versions and continue with the current hacks for older versions.
Yeah, this make sense, seems we have no other choice, thanks for the idea. :-) Regards Osier
On 01/11/2011 04:29 AM, Daniel P. Berrange wrote:
On Tue, Jan 11, 2011 at 03:43:07PM +0800, Osier Yang wrote:
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently.
* src/qemu/qemu_command.c
NACK.
Most previous versions of QEMU don't have any explicit -sdl flag. You got SDL automagically when *no* -vnc flag was given. So this change breaks many old QEMU versions.
We should know (or be able to find out) the precise version of qemu where sdl was added as an explicit device. Once we do that, it's easy enough to modify libvirt's qemu/qemu_capabilities.h to add yet another flag bit, and qemu_capabilities.c to set that flag bit to true for old versions where SDL is automatic, and to new versions where SDL is explicitly available, while leaving it false on new versions (such as the RHEL build) where SDL is disabled. I agree that it doesn't scale well for other features with similar treatments, but the entire qemu_capabilities.c file is an example of the effect of poor scalability of upstream qemu. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年01月11日 23:33, Eric Blake 写道:
On 01/11/2011 04:29 AM, Daniel P. Berrange wrote:
On Tue, Jan 11, 2011 at 03:43:07PM +0800, Osier Yang wrote:
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently.
* src/qemu/qemu_command.c
NACK.
Most previous versions of QEMU don't have any explicit -sdl flag. You got SDL automagically when *no* -vnc flag was given. So this change breaks many old QEMU versions.
We should know (or be able to find out) the precise version of qemu where sdl was added as an explicit device. Once we do that, it's easy enough to modify libvirt's qemu/qemu_capabilities.h to add yet another flag bit, and qemu_capabilities.c to set that flag bit to true for old versions where SDL is automatic, and to new versions where SDL is explicitly available, while leaving it false on new versions (such as the RHEL build) where SDL is disabled.
I agree that it doesn't scale well for other features with similar treatments, but the entire qemu_capabilities.c file is an example of the effect of poor scalability of upstream qemu.
Seems this idea is same with Rich's. So, will send a v2 patch with the idea. Thanks Regards Osier
于 2011年01月11日 23:33, Eric Blake 写道:
On 01/11/2011 04:29 AM, Daniel P. Berrange wrote:
On Tue, Jan 11, 2011 at 03:43:07PM +0800, Osier Yang wrote:
If the emulator doesn't support SDL graphic, we should reject the use of SDL graphic xml with error messages, but not ignore it silently.
* src/qemu/qemu_command.c
NACK.
Most previous versions of QEMU don't have any explicit -sdl flag. You got SDL automagically when *no* -vnc flag was given. So this change breaks many old QEMU versions.
We should know (or be able to find out) the precise version of qemu where sdl was added as an explicit device. Once we do that, it's easy enough to modify libvirt's qemu/qemu_capabilities.h to add yet another flag bit, and qemu_capabilities.c to set that flag bit to true for old versions where SDL is automatic, and to new versions where SDL is explicitly available, while leaving it false on new versions (such as the RHEL build) where SDL is disabled.
I agree that it doesn't scale well for other features with similar treatments, but the entire qemu_capabilities.c file is an example of the effect of poor scalability of upstream qemu.
[root@Osier qemu]# git log 7d957bd8cbcbf56f7916d375e65042d767f544b5 commit 7d957bd8cbcbf56f7916d375e65042d767f544b5 Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Thu Jan 15 22:14:11 2009 +0000 DisplayState interface change (Stefano Stabellini) [root@Osier qemu]# git log VERSION <split> commit b4171e4b79d5be97ee0d1516aab321f32a3aca1a Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Wed Mar 4 22:47:59 2009 +0000 Add version information for 0.10.0 release. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6685 c046a42c-6fe2-441c-8c8c-71466251a162 diff --git a/VERSION b/VERSION index f514a2f..78bc1ab 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.1 \ No newline at end of file +0.10.0 commit bfe312121eb80226f0cb2d4b7c2b9b5fafecd93e Author: bellard <bellard@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Sun Jan 6 17:10:54 2008 +0000 version change git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@3892 c046a42c-6fe2-441c-8c8c-71466251a162 </split> so, it should be from 0.10.0 Regards Osier
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Justin Clift -
Osier Yang -
Richard W.M. Jones