[libvirt] [PATCH v1 07/11] conf: Allow usage of the <gl> element with VNC graphics

[John Ferlan]

On 06/27/2018 09:34 AM, Erik Skultety wrote:
> VNC doesn't support OpenGL natively, but can run with non-native
> egl-headless support, so enable that.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  docs/formatdomain.html.in                          |  6 ++++
>  docs/schemas/domaincommon.rng                      |  7 ++++
>  src/conf/domain_conf.c                             |  8 +++++
>  src/qemu/qemu_command.c                            |  7 ++++
>  tests/qemuxml2argvdata/graphics-vnc-gl-invalid.xml | 37 ++++++++++++++++++++++
>  tests/qemuxml2argvdata/graphics-vnc-gl.args        | 28 ++++++++++++++++
>  tests/qemuxml2argvdata/graphics-vnc-gl.xml         | 37 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  1 +
>  8 files changed, 131 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/graphics-vnc-gl-invalid.xml
>  create mode 100644 tests/qemuxml2argvdata/graphics-vnc-gl.args
>  create mode 100644 tests/qemuxml2argvdata/graphics-vnc-gl.xml
> 

hmmm...  I recall some discussion about egl-headless in the review of
the RFC:

https://www.redhat.com/archives/libvir-list/2018-June/msg00461.html

So, what's the point? Well I was going to ask why no capability for
egl-headless, but the above link (and a couple followups) discusses the
issue.

Anyway, I think either a commit log message or a comment in the code
when egl-headless is added may be appropriate.  Another option is
following what commit 3278a7bb does and adding the 2.10 check for the
capability that is added.

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0d68596991..aa0d6b26df 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6350,6 +6350,12 @@ qemu-kvm -net nic,model=? /dev/null
>                auto-allocation and <code>autoport</code> having no effect due to
>                security reasons) <span class="since">Since 1.0.6</span>.
>              </p>
> +            <p>
> +              <span class="since">Since 4.6.0</span> it's possible to use the
> +              <code>gl</code> element with <code>enable='yes'</code> to enable
> +              OpenGL support using QEMU's egl-headless display, since VNC
> +              doesn't support OpenGL natively like SPICE does.
> +            </p>
>            </dd>
>            <dt><code>spice</code> <span class="since">Since 0.8.6</span></dt>
>            <dd>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f46145cf9b..20649c5f6f 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3135,6 +3135,13 @@
>              </attribute>
>            </optional>
>            <ref name="listenElements"/>
> +          <optional>
> +            <element name="gl">
> +              <attribute name="enable">
> +                <ref name="virYesNo"/>
> +              </attribute>
> +            </element>
> +          </optional>
>          </group>
>          <group>
>            <attribute name="type">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6bfa3ca130..2ccd9e124f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13619,8 +13619,11 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
>      char *websocket = virXMLPropString(node, "websocket");
>      char *sharePolicy = virXMLPropString(node, "sharePolicy");
>      char *autoport = virXMLPropString(node, "autoport");
> +    xmlNodePtr save = ctxt->node;
>      int ret = -1;
>  
> +    ctxt->node = node;
> +

Not used here - but virDomainGraphicsListensParseXML does the same
thing... I think you can remove it here or just pass node to
virDomainGraphicsListensParseXML - your call.

>      if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
>          goto cleanup;
>  
> @@ -13681,12 +13684,17 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
>                                           def->type) < 0)
>          goto cleanup;
>  
> +    if (virDomainGraphicsGLDefParseXML(def,
> +                                       virXPathNode("./gl[1]", ctxt)) < 0)

Fits on previous line

> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      VIR_FREE(port);
>      VIR_FREE(autoport);
>      VIR_FREE(websocket);
>      VIR_FREE(sharePolicy);
> +    ctxt->node = save;
>      return ret;
>  }
>  

With a couple of cleanups and addressing the egl-headless conundrum, I
think we're good...

John