Re: [libvirt] [PATCH] qemu: add sdl opengl support

On 02/05/18 08:13, Daniel P. Berrangé wrote: > On Tue, May 01, 2018 at 08:22:45PM +0100, Maciej Wolny wrote: > > Add SDL graphics gl attribute, modify the domain XML schema, add a > > test, modify the documentation to include the new option. > > > > Signed-off-by: Maciej Wolny <maciej.wolny(a)codethink.co.uk&gt; > > --- > > docs/schemas/domaincommon.rng | 8 +++++ > > src/conf/domain_conf.c | 41 ++++++++++++++++++++++ > > src/conf/domain_conf.h | 1 + > > src/qemu/qemu_capabilities.c | 2 ++ > > src/qemu/qemu_capabilities.h | 1 + > > src/qemu/qemu_command.c | 19 ++++++++++ > > .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args | 28 +++++++++++++++ > > tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++++ > > tests/qemuxml2argvtest.c | 5 +++ > > 9 files changed, 143 insertions(+) > > create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args > > create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml > > > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > > index 3569b9212..a2ef93c09 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 b0257068d..7d65ca9df 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -13448,6 +13448,7 @@ static int > > virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, > > xmlNodePtr node) > > { > > + xmlNodePtr cur; > > char *fullscreen = virXMLPropString(node, "fullscreen"); > > int ret = -1; > > @@ -13468,6 +13469,34 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, > > def->data.sdl.xauth = virXMLPropString(node, "xauth"); > > def->data.sdl.display = virXMLPropString(node, "display"); > > + cur = node->children; > > + while (cur != NULL) { > > + if (cur->type == XML_ELEMENT_NODE) { > > + if (virXMLNodeNameEqual(cur, "gl")) { > > + char *enable = virXMLPropString(cur, "enable"); > > + int enableVal; > > + > > + 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; > > + } > > + } > > + cur = cur->next; > > + } > > + > > ret = 0; > > cleanup: > > VIR_FREE(fullscreen); > > @@ -25652,6 +25681,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf, > > if (def->data.sdl.fullscreen) > > virBufferAddLit(buf, " fullscreen='yes'"); > > + if (!children && def->data.sdl.gl) { > > + virBufferAddLit(buf, ">\n"); > > + virBufferAdjustIndent(buf, 2); > > + children = true; > > + } > > + > > + if (def->data.sdl.gl) { > > + virBufferAsprintf(buf, "<gl enable='%s'", > > + virTristateBoolTypeToString(def->data.sdl.gl)); > > + virBufferAddLit(buf, "/>\n"); > > + } > > SPICE GL support allows a "rendernode" property to be set - I'm wondering > if that is relevant to SDL or not ? > > > Regards, > Daniel > I purposefully didn't look into this because we didn't need that option for our use cases - would you still merge this patch without implementing this option?