On 02/14/2017 10:04 PM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
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@redhat.com> --- 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