On 02/05/18 11:54, Daniel P. Berrangé wrote:
> 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
>
Grepping through the source code of QEMU, it seems that the rendernode option
is only used for SPICE (for egl_rendernode_init in ui/spice-core.c).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
