[libvirt] [PATCH v2 0/5] Add QEMU SDL OpenGL support
This patch set adds support for accelerated graphics rendering with OpenGL when using the SDL backend with QEMU. This takes advantage of the `-sdl gl` option in QEMU. Maciej Wolny (5): qemu_command: Move SDL command line building into helper qemu_command: Remove outdated comment qemu: Add gl property to graphics of type sdl in domain config qemu: Add QEMU_CAPS_SDL_GL to qemu capabilities qemu: Add gl option to SDL graphics command line docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 8 +++ src/conf/domain_conf.c | 44 +++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 69 +++++++++++++++------- .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 +- .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 +++++++++ tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++ tests/qemuxml2argvtest.c | 5 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 14 files changed, 237 insertions(+), 23 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml -- 2.11.0
Create a function called `qemuBuildGraphicsSDLCommandLine` which is called from qemuBuildGraphicsCommandLine. Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c6bf6d583..d8a44b9e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7568,6 +7568,34 @@ qemuBuildMemoryDeviceCommandLine(virCommandPtr cmd, static int +qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, + virCommandPtr cmd, + virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, + virDomainGraphicsDefPtr graphics) +{ + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + virCommandAddArg(cmd, "-full-screen"); + + /* If using SDL for video, then we should just let it + * use QEMU's host audio drivers, possibly SDL too + * User can set these two before starting libvirtd + */ + virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); + virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); + + /* 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 :-( */ + virCommandAddArg(cmd, "-sdl"); + return 0; +} + + +static int qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, @@ -7946,26 +7974,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, { switch (graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (graphics->data.sdl.xauth) - virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.sdl.xauth); - if (graphics->data.sdl.display) - virCommandAddEnvPair(cmd, "DISPLAY", graphics->data.sdl.display); - if (graphics->data.sdl.fullscreen) - virCommandAddArg(cmd, "-full-screen"); - - /* If using SDL for video, then we should just let it - * use QEMU's host audio drivers, possibly SDL too - * User can set these two before starting libvirtd - */ - virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); - virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); - - /* 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 :-( */ - virCommandAddArg(cmd, "-sdl"); - - break; + return qemuBuildGraphicsSDLCommandLine(cfg, cmd, qemuCaps, graphics); case VIR_DOMAIN_GRAPHICS_TYPE_VNC: return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); -- 2.11.0
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Create a function called `qemuBuildGraphicsSDLCommandLine` which is called from qemuBuildGraphicsCommandLine.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John [yes, the new function could be a void now, but it's changing in a couple patches so leave things as is]
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d8a44b9e6..690be51e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7587,9 +7587,6 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virCommandAddEnvPassBlockSUID(cmd, "QEMU_AUDIO_DRV", NULL); virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); - /* 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 :-( */ virCommandAddArg(cmd, "-sdl"); return 0; } -- 2.11.0
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 3 --- 1 file changed, 3 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation. Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e2f..a7ef9269f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6162,6 +6162,12 @@ qemu-kvm -net nic,model=? /dev/null and an optional <code>fullscreen</code> attribute accepting values <code>yes</code> or <code>no</code>. </p> + + <p> + You can use a <code>gl</code> with the <code>enable="yes"</code> + property to enable OpenGL support in SDL. Likewise you can + explicitly disable OpenGL support with <code>enable="no"</code>. + </p> </dd> <dt><code>vnc</code></dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b2f..c4c500a3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3031,6 +3031,14 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <element name="gl"> + <attribute name="enable"> + <ref name="virYesNo"/> + </attribute> + <empty/> + </element> + </optional> </group> <group> <attribute name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26b2..85bfa800b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13448,11 +13448,18 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def, static int virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, - xmlNodePtr node) + xmlNodePtr node, + xmlXPathContextPtr ctxt) { + xmlNodePtr save = ctxt->node; + char *enable; + int enableVal; + xmlNodePtr glNode; char *fullscreen = virXMLPropString(node, "fullscreen"); int ret = -1; + ctxt->node = node; + if (fullscreen != NULL) { if (STREQ(fullscreen, "yes")) { def->data.sdl.fullscreen = true; @@ -13470,9 +13477,30 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, def->data.sdl.xauth = virXMLPropString(node, "xauth"); def->data.sdl.display = virXMLPropString(node, "display"); + glNode = virXPathNode("./gl", ctxt); + if (glNode) { + enable = virXMLPropString(glNode, "enable"); + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("sdl gl element missing enable")); + goto cleanup; + } + + enableVal = virTristateBoolTypeFromString(enable); + if (enableVal < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown enable value '%s'"), enable); + VIR_FREE(enable); + goto cleanup; + } + VIR_FREE(enable); + def->data.sdl.gl = enableVal; + } + ret = 0; cleanup: VIR_FREE(fullscreen); + ctxt->node = save; return ret; } @@ -13901,7 +13929,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, goto error; break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (virDomainGraphicsDefParseXMLSDL(def, node) < 0) + if (virDomainGraphicsDefParseXMLSDL(def, node, ctxt) < 0) goto error; break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -25654,6 +25682,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.sdl.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); + if (!children && def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + children = true; + } + + if (def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { + virBufferAsprintf(buf, "<gl enable='%s'", + virTristateBoolTypeToString(def->data.sdl.gl)); + virBufferAddLit(buf, "/>\n"); + } + break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228ba9..517278989 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1599,6 +1599,7 @@ struct _virDomainGraphicsDef { char *display; char *xauth; bool fullscreen; + virTristateBool gl; } sdl; struct { int port; diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml new file mode 100644 index 000000000..9dea73fbe --- /dev/null +++ b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='sdl'> + <gl enable='yes'/> + </graphics> + <video> + <model type='virtio' heads='1'> + <acceleration accel3d='yes'/> + </model> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml b/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml new file mode 100644 index 000000000..da03dd5da --- /dev/null +++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='sdl'> + <gl enable='yes'/> + </graphics> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 53a26a0c3..9c4f8054e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1096,6 +1096,7 @@ mymain(void) DO_TEST("video-virtio-gpu-device", NONE); DO_TEST("video-virtio-gpu-virgl", NONE); DO_TEST("video-virtio-gpu-spice-gl", NONE); + DO_TEST("video-virtio-gpu-sdl-gl", NONE); DO_TEST("virtio-input", NONE); DO_TEST("virtio-input-passthrough", NONE); -- 2.11.0
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
[...]
--- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13448,11 +13448,18 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
static int virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, - xmlNodePtr node) + xmlNodePtr node, + xmlXPathContextPtr ctxt) { + xmlNodePtr save = ctxt->node; + char *enable;
Initialize = NULL, then...
+ int enableVal; + xmlNodePtr glNode; char *fullscreen = virXMLPropString(node, "fullscreen"); int ret = -1;
+ ctxt->node = node; + if (fullscreen != NULL) { if (STREQ(fullscreen, "yes")) { def->data.sdl.fullscreen = true; @@ -13470,9 +13477,30 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, def->data.sdl.xauth = virXMLPropString(node, "xauth"); def->data.sdl.display = virXMLPropString(node, "display");
+ glNode = virXPathNode("./gl", ctxt); + if (glNode) { + enable = virXMLPropString(glNode, "enable"); + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("sdl gl element missing enable")); + goto cleanup; + } + + enableVal = virTristateBoolTypeFromString(enable); + if (enableVal < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown enable value '%s'"), enable); + VIR_FREE(enable);
Move the VIR_FREE(enable) into cleanup and remove the one 3 lines later.
+ goto cleanup; + } + VIR_FREE(enable); + def->data.sdl.gl = enableVal; + } + ret = 0; cleanup: VIR_FREE(fullscreen); + ctxt->node = save; return ret; }
@@ -13901,7 +13929,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, goto error; break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (virDomainGraphicsDefParseXMLSDL(def, node) < 0) + if (virDomainGraphicsDefParseXMLSDL(def, node, ctxt) < 0) goto error; break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -25654,6 +25682,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.sdl.fullscreen) virBufferAddLit(buf, " fullscreen='yes'");
+ if (!children && def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) {
Well I certainly hope it cannot be true at this point; otherwise, the compiler reorganized things on us ;-)... I'll leave it as is since it's a bit of preventive coding...
+ virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + children = true; + } + + if (def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { + virBufferAsprintf(buf, "<gl enable='%s'", + virTristateBoolTypeToString(def->data.sdl.gl)); + virBufferAddLit(buf, "/>\n"); + } + break;
I'll make the above adjustment to enable processing before pushing Reviewed-by: John Ferlan <jferlan@redhat.com> John
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:') For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming. -- milloni
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML. I hope that's clear. Have a nice day.
On 05/14/2018 07:24 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML.
I hope that's clear.
Have a nice day.
However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired? Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". Could/should generification of the xml2xml tests be considered a "bite sized task"? John
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote:
On 05/14/2018 07:24 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML.
I hope that's clear.
Have a nice day.
However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired?
Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>".
Could/should generification of the xml2xml tests be considered a "bite sized task"?
Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those.
On 14/05/18 13:40, Martin Kletzander wrote:
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote:
On 05/14/2018 07:24 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote:
Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 44 ++++++++++++++++++++- src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML.
I hope that's clear.
Have a nice day.
However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired?
Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>".
Could/should generification of the xml2xml tests be considered a "bite sized task"?
Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those.
So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested? -- milloni
On 05/14/2018 09:18 AM, Maciej Wolny wrote:
On 14/05/18 13:40, Martin Kletzander wrote:
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote:
On 05/14/2018 07:24 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote:
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: > Support OpenGL accelerated rendering when using SDL graphics in the > domain config. Add associated test and documentation. > > Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> > --- > docs/formatdomain.html.in | 6 +++ > docs/schemas/domaincommon.rng | 8 ++++ > src/conf/domain_conf.c | 44 > ++++++++++++++++++++- > src/conf/domain_conf.h | 1 +
docs, conf and schemas fit together nicely, they should be in one patch, but.
> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 > ++++++++++++++++++ > .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 > ++++++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 +
this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:')
For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML.
I hope that's clear.
Have a nice day.
However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired?
Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>".
Could/should generification of the xml2xml tests be considered a "bite sized task"?
Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those.
So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested?
My opinion is leave it as is - hard to require you to do something we're not requiring other patches at this point. I'm not a fan of duplication - that is the task would be to essentially copy everything into the generic xml2xml test at this point. John
On Mon, May 14, 2018 at 09:21:58AM -0400, John Ferlan wrote:
On 05/14/2018 09:18 AM, Maciej Wolny wrote:
On 14/05/18 13:40, Martin Kletzander wrote:
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote:
On 05/14/2018 07:24 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote:
On 11/05/18 09:42, Martin Kletzander wrote: > On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >> Support OpenGL accelerated rendering when using SDL graphics in the >> domain config. Add associated test and documentation. >> >> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >> --- >> docs/formatdomain.html.in | 6 +++ >> docs/schemas/domaincommon.rng | 8 ++++ >> src/conf/domain_conf.c | 44 >> ++++++++++++++++++++- >> src/conf/domain_conf.h | 1 + > > docs, conf and schemas fit together nicely, they should be in one > patch, but. > >> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >> ++++++++++++++++++ >> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >> ++++++++++++++++++++++ >> tests/qemuxml2xmltest.c | 1 + > > this has nothing to do with qemu (yet), also see Subject (I wouldn't say > 'qemu:' there, but rather something like 'docs, conf, schema:') > > For the XML tests above you can use genericxml2xmltest instead of the > QEMU-specific one.
The option only makes sense in QEMU afaik, hence the naming.
Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML.
I hope that's clear.
Have a nice day.
However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired?
Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>".
Could/should generification of the xml2xml tests be considered a "bite sized task"?
Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those.
So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested?
My opinion is leave it as is - hard to require you to do something we're not requiring other patches at this point. I'm not a fan of duplication - that is the task would be to essentially copy everything into the generic xml2xml test at this point.
Well, I got reminded several times about this, but it is poosible that that was off-list (IRC or privately). I'm fine with leaving it as is. Mainly since there are soo many of similar ones from the past. So sorry for blocking you with this tiny, unimportant request.
John
Support OpenGL acceleration capability when using SDL graphics. Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", ); @@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index abd6eff14..01d6be818 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */ QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */ + QEMU_CAPS_SDL_GL, /* -sdl gl */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 68ecb0c17..5614fc63c 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3575,6 +3575,15 @@ }, { "parameters": [ + { + "name": "gl", + "type": "boolean" + } + ], + "option": "sdl" + }, + { + "parameters": [ ], "option": "acpi" }, diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index f3834d5bc..3a968542b 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -153,9 +153,10 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> + <flag name='sdl-gl'/> <version>2004000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>75406</microcodeVersion> + <microcodeVersion>75544</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> -- 2.11.0
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, }; static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT); + /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index abd6eff14..01d6be818 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */ QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */ + QEMU_CAPS_SDL_GL, /* -sdl gl */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 68ecb0c17..5614fc63c 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3575,6 +3575,15 @@ }, { "parameters": [ + { + "name": "gl", + "type": "boolean" + } + ], + "option": "sdl" + }, + { + "parameters": [ ], "option": "acpi" },
I'll remove this hand edit...
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index f3834d5bc..3a968542b 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -153,9 +153,10 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> + <flag name='sdl-gl'/> <version>2004000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>75406</microcodeVersion> + <microcodeVersion>75544</microcodeVersion>
This hunk would be reverted
<package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/>
and run VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest to generate a bunch of caps_2*.*.xml files across all the arches. I've made the adjustments, but just need to make sure you're OK with that and that no one else has some sort of agita over the solution or some other way to try and get the answer without changing virQEMUCapsInitQMPMonitor. Reviewed-by: John Ferlan <jferlan@redhat.com> John
On Thu, May 10, 2018 at 04:17:52PM -0400, John Ferlan wrote:
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+ /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
NO! Why? We only result to setting capabilities by version if there is no other way to do that. If query-commandline-options (or whatever that name is) works for this capability, why would you even consider this change? NACK to the diff you added.
On 05/11/2018 04:26 AM, Martin Kletzander wrote:
On Thu, May 10, 2018 at 04:17:52PM -0400, John Ferlan wrote:
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+ /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
NO! Why? We only result to setting capabilities by version if there is no other way to do that. If query-commandline-options (or whatever that name is) works for this capability, why would you even consider this change?
NACK to the diff you added.
Are you sure? Did you read my responses from v1: https://www.redhat.com/archives/libvir-list/2018-May/msg00250.html https://www.redhat.com/archives/libvir-list/2018-May/msg00671.html I actually spent some time trying to figure out which magic incantation of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted. Looking at the QEMU source code - AFAICT the gl option is only "known" or handled within parse_options of vl.c. I found nothing in the .json files where I'd usually find some mechanism that would allow introspection for the "-sdl gl" option. There is something in qapi/ui.json that has 'gl' that looks like it could be used in some new command syntax, but that seems to imply QEMU 2.12 only. Please enlighten me/us with code that will work before just pushing the NACK button. Tks, John
On Fri, May 11, 2018 at 06:34:54AM -0400, John Ferlan wrote:
On 05/11/2018 04:26 AM, Martin Kletzander wrote:
On Thu, May 10, 2018 at 04:17:52PM -0400, John Ferlan wrote:
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+ /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
NO! Why? We only result to setting capabilities by version if there is no other way to do that. If query-commandline-options (or whatever that name is) works for this capability, why would you even consider this change?
NACK to the diff you added.
Are you sure? Did you read my responses from v1:
https://www.redhat.com/archives/libvir-list/2018-May/msg00250.html https://www.redhat.com/archives/libvir-list/2018-May/msg00671.html
I actually spent some time trying to figure out which magic incantation of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted.
Looking at the QEMU source code - AFAICT the gl option is only "known" or handled within parse_options of vl.c. I found nothing in the .json files where I'd usually find some mechanism that would allow introspection for the "-sdl gl" option. There is something in qapi/ui.json that has 'gl' that looks like it could be used in some new command syntax, but that seems to imply QEMU 2.12 only.
Please enlighten me/us with code that will work before just pushing the NACK button.
I'm so sorry for that. I totally missed the part that the reply was hand-edited. In that case, unfortunately, the only way is to use the version _again_. So my apologies once again, especially if that sounded rude (which it does to me now when I'm reading after myself). I am adding a capability currently and I'm dealing with part of code that I reworked few times due to a similar misunderstanding from a previous developer. So feel free to squash that in if Maciej is OK with it as well (which he should be, otherwise it won't work for anyone anyway). ACK.
Tks,
John
On 05/11/2018 07:47 AM, Martin Kletzander wrote:
On Fri, May 11, 2018 at 06:34:54AM -0400, John Ferlan wrote:
On 05/11/2018 04:26 AM, Martin Kletzander wrote:
On Thu, May 10, 2018 at 04:17:52PM -0400, John Ferlan wrote:
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+ /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
NO! Why? We only result to setting capabilities by version if there is no other way to do that. If query-commandline-options (or whatever that name is) works for this capability, why would you even consider this change?
NACK to the diff you added.
Are you sure? Did you read my responses from v1:
https://www.redhat.com/archives/libvir-list/2018-May/msg00250.html https://www.redhat.com/archives/libvir-list/2018-May/msg00671.html
I actually spent some time trying to figure out which magic incantation of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted.
Looking at the QEMU source code - AFAICT the gl option is only "known" or handled within parse_options of vl.c. I found nothing in the .json files where I'd usually find some mechanism that would allow introspection for the "-sdl gl" option. There is something in qapi/ui.json that has 'gl' that looks like it could be used in some new command syntax, but that seems to imply QEMU 2.12 only.
Please enlighten me/us with code that will work before just pushing the NACK button.
I'm so sorry for that. I totally missed the part that the reply was hand-edited. In that case, unfortunately, the only way is to use the version _again_. So my apologies once again, especially if that sounded rude (which it does to me now when I'm reading after myself). I am adding a capability currently and I'm dealing with part of code that I reworked few times due to a similar misunderstanding from a previous developer.
We're good - don't worry - but thanks for the note. I certainly get the don't change virQEMUCapsInitQMPMonitor sentiment - it was my last resort (and at the bottom of my initial reply). It's frustrating when spice gl has the capability but sdl gl doesn't. Almost makes one want to say - well don't check and if someone tries with pre qemu 2.4 and it fails, send the bug report to qemu to add the proper capability. John
So feel free to squash that in if Maciej is OK with it as well (which he should be, otherwise it won't work for anyone anyway). ACK.
Tks,
John
On 11/05/18 11:34, John Ferlan wrote:> I actually spent some time trying to figure out which magic incantation
of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted.
It was hand-edited. I'm still not sure how this work.. I ran the tool to generate a .replies file; but based on that, how do I get the replies for all historical version of QEMU, do I need to have each of the versions of QEMU installed on my system. -- milloni
On Fri, May 11, 2018 at 03:14:26PM +0100, Maciej Wolny wrote:
On 11/05/18 11:34, John Ferlan wrote:> I actually spent some time trying to figure out which magic incantation
of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted.
It was hand-edited. I'm still not sure how this work.. I ran the tool to generate a .replies file; but based on that, how do I get the replies for all historical version of QEMU, do I need to have each of the versions of QEMU installed on my system.
Unfortunately, yes. There was some work done so taht we could automate that, but nobody actually finished the work. Anyway, since QEMU does not expose that capability in the QMP communication (what libvirt uses to check for the support), we need to guess it based on the release version :(
On 10/05/18 21:17, John Ferlan wrote:
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL acceleration capability when using SDL graphics.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-)
As I rather lengthily noted in the v1 - I'm assuming you handed edited the .replies file. What that perhaps works and gets you the answer, the fact that none of other files were adjusted leads me to the hand editing belief.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a59617..23f917b66 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "sdl-gl", );
@@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, + { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
Rather than this, I'll apply the following diff:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23f917b66e..28079fa7ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, - { "sdl", "gl", QEMU_CAPS_SDL_GL }, };
static int @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2004000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
+ /* sdl -gl option is supported from v2.4.0 (qemu commit id 0b71a5d5) */ + if (qemuCaps->version >= 2004000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL); + /* Since 2.4.50 ARM virt machine supports gic-version option */ if (qemuCaps->version >= 2004050) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index abd6eff14..01d6be818 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */ QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */ + QEMU_CAPS_SDL_GL, /* -sdl gl */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 68ecb0c17..5614fc63c 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3575,6 +3575,15 @@ }, { "parameters": [ + { + "name": "gl", + "type": "boolean" + } + ], + "option": "sdl" + }, + { + "parameters": [ ], "option": "acpi" },
I'll remove this hand edit...
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index f3834d5bc..3a968542b 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -153,9 +153,10 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> + <flag name='sdl-gl'/> <version>2004000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>75406</microcodeVersion> + <microcodeVersion>75544</microcodeVersion>
This hunk would be reverted
<package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/>
and run VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest to generate a bunch of caps_2*.*.xml files across all the arches.
I've made the adjustments, but just need to make sure you're OK with that and that no one else has some sort of agita over the solution or some other way to try and get the answer without changing virQEMUCapsInitQMPMonitor.
I'm OK with that.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
Support OpenGL when using SDL backend via -sdl,gl=on. Add associated tests. Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 25 ++++++++++++++++++- .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 690be51e0..9e7a0f26d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7573,6 +7573,10 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, virDomainGraphicsDefPtr graphics) { + int ret = -1; + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *optContent; + if (graphics->data.sdl.xauth) virCommandAddEnvPair(cmd, "XAUTHORITY", graphics->data.sdl.xauth); if (graphics->data.sdl.display) @@ -7588,7 +7592,26 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virCommandAddEnvPassBlockSUID(cmd, "SDL_AUDIODRIVER", NULL); virCommandAddArg(cmd, "-sdl"); - return 0; + + if (graphics->data.sdl.gl == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL_GL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support SDL OpenGL")); + goto cleanup; + } + + virBufferAsprintf(&opt, "gl=%s", + virTristateSwitchTypeToString(graphics->data.sdl.gl)); + } + + optContent = virBufferCurrentContent(&opt); + if (optContent && STRNEQ(optContent, "")) + virCommandAddArgBuffer(cmd, &opt); + + ret = 0; + cleanup: + virBufferFreeAndReset(&opt); + return ret; } diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args new file mode 100644 index 000000000..4172320ed --- /dev/null +++ b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ +id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-sdl gl=on \ +-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddf567b62..5d8dd9d04 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1908,6 +1908,11 @@ mymain(void) QEMU_CAPS_SPICE_GL, QEMU_CAPS_SPICE_RENDERNODE, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("video-virtio-gpu-sdl-gl", + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_SDL_GL, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); DO_TEST("video-virtio-gpu-secondary", QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); -- 2.11.0
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
Support OpenGL when using SDL backend via -sdl,gl=on. Add associated tests.
I'll add this here: NB: Usage of DO_TEST_CAPS_LATEST in qemuxml2argv doesn't work in this case because -sdl gl is not introspectable.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- src/qemu/qemu_command.c | 25 ++++++++++++++++++- .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args
Reviewed-by: John Ferlan <jferlan@redhat.com> John Plz just let me know if you're OK with my patch4 adjustment. I'll push in a couple of days anyway to ensure no one else has concerns I missed or has a better idea of how to get that -sdl gl introspected.
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
This patch set adds support for accelerated graphics rendering with OpenGL when using the SDL backend with QEMU. This takes advantage of the `-sdl gl` option in QEMU.
Maciej Wolny (5): qemu_command: Move SDL command line building into helper qemu_command: Remove outdated comment qemu: Add gl property to graphics of type sdl in domain config qemu: Add QEMU_CAPS_SDL_GL to qemu capabilities qemu: Add gl option to SDL graphics command line
docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 8 +++ src/conf/domain_conf.c | 44 +++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 69 +++++++++++++++------- .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 +- .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 +++++++++ tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++ tests/qemuxml2argvtest.c | 5 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 14 files changed, 237 insertions(+), 23 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
Could you also post a 6/5 that would change docs/news.xml to describe the Improvement... Tks - John
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 80181415c..e4a10f667 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,15 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Add suport for OpenGL rendering with SDL + </summary> + <description> + Domains using SDL as a graphics backend will now be able to use + OpenGL accelerated rendering. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.11.0
$SUBJ: docs: Update news.xml with QEMU SDL OpenGL Improvement On 05/11/2018 10:47 AM, Maciej Wolny wrote:
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> --- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 80181415c..e4a10f667 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,15 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Add suport for OpenGL rendering with SDL + </summary> + <description> + Domains using SDL as a graphics backend will now be able to use + OpenGL accelerated rendering. + </description> + </change>
This should have been put into the 4.4 section not the comments... And each indention level can be 2 spaces... I will move/fix it... Reviewed-by: John Ferlan <jferlan@redhat.com> John
</section> <section title="Bug fixes"> </section>
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
This patch set adds support for accelerated graphics rendering with OpenGL when using the SDL backend with QEMU. This takes advantage of the `-sdl gl` option in QEMU.
Maciej Wolny (5): qemu_command: Move SDL command line building into helper qemu_command: Remove outdated comment qemu: Add gl property to graphics of type sdl in domain config qemu: Add QEMU_CAPS_SDL_GL to qemu capabilities qemu: Add gl option to SDL graphics command line
docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 8 +++ src/conf/domain_conf.c | 44 +++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 69 +++++++++++++++------- .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 3 +- .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 +++++++++ tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++ tests/qemuxml2argvtest.c | 5 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 14 files changed, 237 insertions(+), 23 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
This is now pushed. John
participants (3)
-
John Ferlan -
Maciej Wolny -
Martin Kletzander