[libvirt] [PATCH] domain: conf: graphics: Fix picking DRI renderer automatically for SPICE

[Erik Skultety]

Commit 255e0732 introduced a few graphics-related helpers. The problem
is that virDomainGraphicsNeedsAutoRenderNode returns true if it gets
NULL as a response from virDomainGraphicsNeedsAutoRenderNode. That's
okay for egl-headless because that one always needs a DRM render node,
the same is not true for SPICE though, and unless the XML specifies
<gl enable='yes'> for SPICE, there's no need for any renderer.

https://bugzilla.redhat.com/show_bug.cgi?id=1656895

Signed-off-by: Erik Skultety <***@redhat.com>
---
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b70dca6c61..efa0a94f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30982,8 +30982,7 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics)

switch (graphics->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
- if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES)
- ret = graphics->data.spice.rendernode;
+ ret = graphics->data.spice.rendernode;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
ret = graphics->data.egl_headless.rendernode;
@@ -31006,6 +31005,10 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics)
if (!virDomainGraphicsSupportsRenderNode(graphics))
return false;

+ if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+ graphics->data.spice.gl != VIR_TRISTATE_BOOL_YES)
+ return false;
+
if (virDomainGraphicsGetRenderNode(graphics))
return false;

[Ján Tomko]

Post by Erik Skultety
Commit 255e0732 introduced a few graphics-related helpers. The problem
is that virDomainGraphicsNeedsAutoRenderNode returns true if it gets
NULL as a response from virDomainGraphicsNeedsAutoRenderNode. That's
okay for egl-headless because that one always needs a DRM render node,
the same is not true for SPICE though, and unless the XML specifies
<gl enable='yes'> for SPICE, there's no need for any renderer.
https://bugzilla.redhat.com/show_bug.cgi?id=1656895

If it's NOTABUG, I don't see a reason to include it in the git history.

Post by Erik Skultety
---
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b70dca6c61..efa0a94f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30982,8 +30982,7 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics)
switch (graphics->type) {
- if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES)
- ret = graphics->data.spice.rendernode;
+ ret = graphics->data.spice.rendernode;
break;
ret = graphics->data.egl_headless.rendernode;
@@ -31006,6 +31005,10 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics)
if (!virDomainGraphicsSupportsRenderNode(graphics))
return false;
+ if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+ graphics->data.spice.gl != VIR_TRISTATE_BOOL_YES)
+ return false;
+
if (virDomainGraphicsGetRenderNode(graphics))
return false;

Reviewed-by: Ján Tomko <***@redhat.com>

Jano

[Erik Skultety]

Post by Ján Tomko

Post by Erik Skultety
Commit 255e0732 introduced a few graphics-related helpers. The problem
is that virDomainGraphicsNeedsAutoRenderNode returns true if it gets
NULL as a response from virDomainGraphicsNeedsAutoRenderNode. That's
okay for egl-headless because that one always needs a DRM render node,
the same is not true for SPICE though, and unless the XML specifies
<gl enable='yes'> for SPICE, there's no need for any renderer.
https://bugzilla.redhat.com/show_bug.cgi?id=1656895

If it's NOTABUG, I don't see a reason to include it in the git history.

Yes, I wanted to reply that I'd drop it, but I first created it, patched it and
then was notified that the bugzilla doesn't make sense.

Post by Ján Tomko

Post by Erik Skultety
---
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b70dca6c61..efa0a94f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30982,8 +30982,7 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics)
switch (graphics->type) {
- if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES)
- ret = graphics->data.spice.rendernode;
+ ret = graphics->data.spice.rendernode;
break;
ret = graphics->data.egl_headless.rendernode;
@@ -31006,6 +31005,10 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics)
if (!virDomainGraphicsSupportsRenderNode(graphics))
return false;
+ if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+ graphics->data.spice.gl != VIR_TRISTATE_BOOL_YES)
+ return false;
+
if (virDomainGraphicsGetRenderNode(graphics))
return false;

Thanks,
Erik