Add rendernode selection support

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

Posted by marcandre.lureau@redhat.com 8 years, 11 months ago

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
   &lt;clipboard copypaste='no'/&gt;
   &lt;mouse mode='client'/&gt;
   &lt;filetransfer enable='no'/&gt;
-  &lt;gl enable='yes'/&gt;
+  &lt;gl enable='yes' rendernode='/dev/dri/by-path/pci-0000:00:02.0-render'/&gt;
 &lt;/graphics&gt;</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;
 
                 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, "<gl enable='%s'",
+                      virTristateBoolTypeToString(def->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, "<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;
+        }
     }
 
     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;
             virTristateBool filetransfer;
             virTristateBool gl;
+            char *rendernode;
         } spice;
     } data;
     /* nListens, listens, and *port are only useful if type is vnc,
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 399e31447..e851eec7a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -357,6 +357,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
               "query-cpu-model-expansion", /* 245 */
               "virtio-net.host_mtu",
+              "spice-rendernode",
     );
 
 
@@ -2950,6 +2951,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
     { "spice", "unix", QEMU_CAPS_SPICE_UNIX },
     { "drive", "throttling.bps-total-max-length", QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH },
     { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP },
+    { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 95bb67d44..0f998c473 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -393,6 +393,7 @@ typedef enum {
     /* 245 */
     QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION, /* qmp query-cpu-model-expansion */
     QEMU_CAPS_VIRTIO_NET_HOST_MTU, /* virtio-net-*.host_mtu */
+    QEMU_CAPS_SPICE_RENDERNODE, /* -spice rendernode */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c00a47a91..f4bcfd467 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7920,6 +7920,16 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
          * TristateSwitch helper */
         virBufferAsprintf(&opt, "gl=%s,",
                           virTristateSwitchTypeToString(graphics->data.spice.gl));
+
+        if (graphics->data.spice.rendernode) {
+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("This QEMU doesn't support spice OpenGL rendernode"));
+                goto error;
+            }
+
+            virBufferAsprintf(&opt, "rendernode=%s,", graphics->data.spice.rendernode);
+        }
     }
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
index f1ebb62e4..84439cddc 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
@@ -19,6 +19,6 @@ QEMU_AUDIO_DRV=spice \
 -drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
 id=drive-ide0-0-0,cache=none \
 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--spice port=0,gl=on \
+-spice port=0,gl=on,rendernode=/dev/dri/foo \
 -device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml
index b9c7c8af0..fd2580635 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml
@@ -26,7 +26,7 @@
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <graphics type='spice' autoport='no'>
-      <gl enable='yes'/>
+      <gl enable='yes' rendernode='/dev/dri/foo'/>
     </graphics>
     <video>
       <model type='virtio' heads='1'>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index faa99c64c..f55b04b05 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1725,6 +1725,7 @@ mymain(void)
             QEMU_CAPS_VIRTIO_GPU_VIRGL,
             QEMU_CAPS_SPICE,
             QEMU_CAPS_SPICE_GL,
+            QEMU_CAPS_SPICE_RENDERNODE,
             QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
     DO_TEST("video-virtio-gpu-secondary",
             QEMU_CAPS_DEVICE_VIRTIO_GPU,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
index 9fb533ad9..2ce838fab 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-video-virtio-gpu-spice-gl.xml
@@ -31,7 +31,7 @@
     <input type='keyboard' bus='ps2'/>
     <graphics type='spice'>
       <listen type='none'/>
-      <gl enable='yes'/>
+      <gl enable='yes' rendernode='/dev/dri/foo'/>
     </graphics>
     <video>
       <model type='virtio' heads='1' primary='yes'>
-- 
2.11.0.295.gd7dffce1c.dirty

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

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

Posted by Daniel P. Berrange 8 years, 11 months ago

On Wed, Feb 15, 2017 at 01:04:13AM +0400, 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(-)

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

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

Posted by Michal Privoznik 8 years, 11 months ago

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
>    &lt;clipboard copypaste='no'/&gt;
>    &lt;mouse mode='client'/&gt;
>    &lt;filetransfer enable='no'/&gt;
> -  &lt;gl enable='yes'/&gt;
> +  &lt;gl enable='yes' rendernode='/dev/dri/by-path/pci-0000:00:02.0-render'/&gt;
>  &lt;/graphics&gt;</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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/5] nodedev: fix extra space in dump
[libvirt] [PATCH 2/5] nodedev: add <devnode> paths
[libvirt] [PATCH 3/5] nodedev: parse <path>
[libvirt] [PATCH 4/5] nodedev: add drm capability
[libvirt] [PATCH 5/5] qemu: add rendernode argument