[libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config

[John Ferlan]

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 at 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 at redhat.com>

John