[libvirt] [PATCH v3 03/11] qemu: process: spice: Pick the first available DRM render node

[Ján Tomko]

On Thu, Nov 29, 2018 at 03:20:13PM +0100, Erik Skultety wrote:
>Up until now, we formatted 'rendernode=' onto QEMU cmdline only if the
>user specified it in the XML, otherwise we let QEMU do it for us. This
>causes permission issues because by default the /dev/dri/renderDX
>permissions are as follows:
>
>crw-rw----. 1 root video
>
>There's literally no reason why it shouldn't be libvirt picking the DRM
>render node instead of QEMU, that way (and because we're using
>namespaces by default), we can safely relabel the device within the
>namespace.
>
>Signed-off-by: Erik Skultety <eskultet at redhat.com>
>---
> src/qemu/qemu_process.c                       | 22 +++++++++++++++-
> src/util/virutil.h                            |  2 +-
> .../graphics-spice-gl-no-rendernode.args      | 25 +++++++++++++++++++
> .../graphics-spice-gl-no-rendernode.xml       | 24 ++++++++++++++++++
> tests/qemuxml2argvmock.c                      |  9 +++++++
> 5 files changed, 80 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-no-rendernode.args
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-no-rendernode.xml
>
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 12d1fca0d4..feebdc7fdc 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -4784,9 +4784,25 @@ qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver,
> }
>
>
>+static int
>+qemuProcessGraphicsSetupRenderNode(virDomainGraphicsDefPtr graphics,
>+                                   virQEMUCapsPtr qemuCaps)
>+{
>+    /* Don't bother picking a DRM node if QEMU doesn't support it. */
>+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE))
>+        return 0;
>+
>+    if (!(graphics->data.spice.rendernode = virHostGetDRMRenderNode()))
>+        return -1;
>+
>+    return 0;
>+}
>+
>+
> static int
> qemuProcessSetupGraphics(virQEMUDriverPtr driver,
>                          virDomainObjPtr vm,
>+                         virQEMUCapsPtr qemuCaps,
>                          unsigned int flags)
> {
>     virDomainGraphicsDefPtr graphics;
>@@ -4797,6 +4813,10 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver,
>     for (i = 0; i < vm->def->ngraphics; i++) {
>         graphics = vm->def->graphics[i];
>
>+        if (virDomainGraphicsNeedsRenderNode(graphics) &&

Just like the call below is called even for graphics with no listen,
you can call qemuProcessGraphicsSetupRenderNode unconditionally and move
the virDomainGraphicsNeedsRenderNode condition inside.

>+            qemuProcessGraphicsSetupRenderNode(graphics, qemuCaps) < 0)
>+            goto cleanup;
>+
>         if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0)
>             goto cleanup;
>     }
>@@ -5953,7 +5973,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
>         goto cleanup;
>
>     VIR_DEBUG("Setting graphics devices");
>-    if (qemuProcessSetupGraphics(driver, vm, flags) < 0)
>+    if (qemuProcessSetupGraphics(driver, vm, priv->qemuCaps, flags) < 0)
>         goto cleanup;
>
>     VIR_DEBUG("Create domain masterKey");
>diff --git a/src/util/virutil.h b/src/util/virutil.h
>index 89bd21b148..588d779d10 100644
>--- a/src/util/virutil.h
>+++ b/src/util/virutil.h
>@@ -222,7 +222,7 @@ unsigned long long virMemoryMaxValue(bool ulong) ATTRIBUTE_NOINLINE;
>
> bool virHostHasIOMMU(void);
>
>-char *virHostGetDRMRenderNode(void);
>+char *virHostGetDRMRenderNode(void) ATTRIBUTE_NOINLINE;
>
> /**
>  * VIR_ASSIGN_IS_OVERFLOW:
>diff --git a/tests/qemuxml2argvdata/graphics-spice-gl-no-rendernode.args b/tests/qemuxml2argvdata/graphics-spice-gl-no-rendernode.args
>new file mode 100644
>index 0000000000..1b08811692
>--- /dev/null
>+++ b/tests/qemuxml2argvdata/graphics-spice-gl-no-rendernode.args

A test called '-no-rendernode'...

[...]

>+-rtc base=utc \
>+-no-shutdown \
>+-no-acpi \
>+-usb \
>+-spice port=0,gl=on,rendernode=/dev/dri/foo \

... with a rendernode on the command line.

How about '-auto-rendernode'?

>+-vga cirrus \
>+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3

The test cases are not used anywhere. Please use DO_TEST_CAPS_LATEST
when adding them.

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181130/c06ff7a6/attachment-0001.sig>