Re: [libvirt] [PATCH 5/5] qemu: add rendernode argument

[Michal Privoznik]

On 02/14/2017 10:04 PM, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Add a new attribute 'rendernode' to  spice element.
> 
> Give it to QEMU if qemu supports it (queued for 2.9).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  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-:00:02.0-render'/
>  /graphics
>  
>Spice supports variable compression settings for audio, images 
> and
> @@ -5665,6 +5665,11 @@ qemu-kvm -net nic,model=? /dev/null
>the gl element, by setting the enable
>property. (QEMU only, since 1.3.3).
>  
> +
> +  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,
> +  since 3.1).
> +
>
>rdp
>
> 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 @@
>  
>
>  
> +
> +  
> +
> +  
> +
>  
>
>  
> 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(>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, " +  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,
> 

Re: [libvirt] [PATCH 5/5] qemu: add rendernode argument

[Daniel P. Berrange]

On Wed, Feb 15, 2017 at 01:04:13AM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Add a new attribute 'rendernode' to  spice element.
> 
> Give it to QEMU if qemu supports it (queued for 2.9).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  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(-)

ACK

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 5/5] qemu: add rendernode argument

[marcandre . lureau]

From: Marc-André Lureau 

Add a new attribute 'rendernode' to  spice element.

Give it to QEMU if qemu supports it (queued for 2.9).

Signed-off-by: Marc-André Lureau 
---
 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-:00:02.0-render'/
 /graphics
 
   Spice supports variable compression settings for audio, images 
and
@@ -5665,6 +5665,11 @@ qemu-kvm -net nic,model=? /dev/null
   the gl element, by setting the enable
   property. (QEMU only, since 1.3.3).
 
+
+  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,
+  since 3.1).
+
   
   rdp
   
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 @@
 
   
 
+
+  
+
+  
+
 
   
 
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(>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;
 
 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;
+}
+
+virBufferAsprintf(buf, "data.spice.gl));
+
+if (def->data.spice.rendernode)
+virBufferEscapeString(buf, " rendernode='%s'", 
def->data.spice.rendernode);
+
+virBufferAddLit(buf, "/>\n");
+
+return 0;
+}
 
 static int
 virDomainGraphicsDefFormat(virBufferPtr buf,
@@ -23082,9 +23103,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 if (def->data.spice.filetransfer)
 virBufferAsprintf(buf, "\n",
   
virTristateBoolTypeToString(def->data.spice.filetransfer));
-if (def->data.spice.gl)
-virBufferAsprintf(buf, "\n",
-  virTristateBoolTypeToString(def->data.spice.gl));
+
+if (virDomainSpiceGLDefFormat(buf, def) < 0) {
+return -1;
+}
 }
 
 if (children) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index dd79206f6..7e1afa475 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1544,6 +1544,7 @@ struct _virDomainGraphicsDef {
 virTristateBool copypaste;