On 02/14/2017 10:04 PM, marcandre.lureau@xxxxxxxxxx wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> Add a new attribute 'rendernode' to <gl> spice element.
>
> Give it to QEMU if qemu supports it (queued for 2.9).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 7 +++++-
> docs/schemas/domaincommon.rng | 5 ++++
> src/conf/domain_conf.c | 28 +++++++++++++++++++---
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_command.c | 10 ++++++++
> .../qemuxml2argv-video-virtio-gpu-spice-gl.args | 2 +-
> .../qemuxml2argv-video-virtio-gpu-spice-gl.xml | 2 +-
> tests/qemuxml2argvtest.c | 1 +
> .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml | 2 +-
> 11 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0a115f5dc..67f486747 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5619,7 +5619,7 @@ qemu-kvm -net nic,model=? /dev/null
> <clipboard copypaste='no'/>
> <mouse mode='client'/>
> <filetransfer enable='no'/>
> - <gl enable='yes'/>
> + <gl enable='yes' rendernode='/dev/dri/by-path/pci-0000:00:02.0-render'/>
> </graphics></pre>
> <p>
> Spice supports variable compression settings for audio, images and
> @@ -5665,6 +5665,11 @@ qemu-kvm -net nic,model=? /dev/null
> the <code>gl</code> element, by setting the <code>enable</code>
> property. (QEMU only, <span class="since">since 1.3.3</span>).
> </p>
> + <p>
> + By default, QEMU will pick the first available GPU DRM render node.
> + You may specify a DRM render node path to use instead. (QEMU only,
> + <span class="since">since 3.1</span>).
> + </p>
> </dd>
> <dt><code>rdp</code></dt>
> <dd>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index d715bff29..c5f101325 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3033,6 +3033,11 @@
> <attribute name="enable">
> <ref name="virYesNo"/>
> </attribute>
> + <optional>
> + <attribute name="rendernode">
> + <ref name="absFilePath"/>
> + </attribute>
> + </optional>
> <empty/>
> </element>
> </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1bc72a4e9..b577d0aba 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1300,6 +1300,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
> break;
>
> case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
> + VIR_FREE(def->data.spice.rendernode);
> VIR_FREE(def->data.spice.keymap);
> virDomainGraphicsAuthDefClear(&def->data.spice.auth);
> break;
> @@ -12141,6 +12142,7 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def,
> def->data.spice.filetransfer = enableVal;
> } else if (xmlStrEqual(cur->name, BAD_CAST "gl")) {
> char *enable = virXMLPropString(cur, "enable");
> + char *rendernode = virXMLPropString(cur, "rendernode");
> int enableVal;
This might be leaked if control reaches 'goto' lines in between this and
the following hunk.
>
> if (!enable) {
> @@ -12159,6 +12161,8 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def,
> VIR_FREE(enable);
>
> def->data.spice.gl = enableVal;
> + def->data.spice.rendernode = rendernode;
> +
> } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) {
> char *mode = virXMLPropString(cur, "mode");
> int modeVal;
> @@ -22839,6 +22843,23 @@ virDomainGraphicsListenDefFormatAddr(virBufferPtr buf,
> virBufferAsprintf(buf, " listen='%s'", glisten->address);
> }
>
> +static int
> +virDomainSpiceGLDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def)
> +{
> + if (def->data.spice.gl == VIR_TRISTATE_BOOL_ABSENT) {
> + return 0;
> + }
Firstly, no need for this function to return an int. We usually have
functions like this return nothing (void). Secondly, there's no need to
put curly braces around one line body.
> +
> + virBufferAsprintf(buf, "<gl enable='%s'",
> + virTristateBoolTypeToString(def->data.spice.gl));
> +
> + if (def->data.spice.rendernode)
> + virBufferEscapeString(buf, " rendernode='%s'", def->data.spice.rendernode);
The virBufferEscapeString() is no-op if the last arg is NULL.
> +
> + virBufferAddLit(buf, "/>\n");
> +
> + return 0;
> +}
>
> static int
> virDomainGraphicsDefFormat(virBufferPtr buf,
> @@ -23082,9 +23103,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
> if (def->data.spice.filetransfer)
> virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n",
> virTristateBoolTypeToString(def->data.spice.filetransfer));
> - if (def->data.spice.gl)
> - virBufferAsprintf(buf, "<gl enable='%s'/>\n",
> - virTristateBoolTypeToString(def->data.spice.gl));
> +
> + if (virDomainSpiceGLDefFormat(buf, def) < 0) {
> + return -1;
> + }
Again. This will never happen and also there's no need for {}.
> }
>
> if (children) {
I am fixing all of these small nits that I've raised and pushing all of
the patches. I have couple of follow up patches ready so expect them to
be send shortly.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
