On Wed, May 02, 2018 at 11:48:24AM +0100, Maciej Wolny wrote:
> 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@xxxxxxxxxxxxxxx>
> > > ---
> > > 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?
My thought was that if rendernode is also applicable for SPICE, then
instead of duplicating the struct fields and duplicating parsing, we
should create a helper method for dealing with the <gl> element that
can be shared with SPICE & SDL. If rendernode is not allowed with
SPICE in QEMU, then your simpler approach is sufficient
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
