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

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 <eskultet@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; -- 2.19.2

On Thu, Dec 06, 2018 at 05:12:36PM +0100, Erik Skultety wrote:
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.
If it's NOTABUG, I don't see a reason to include it in the git history.
Signed-off-by: Erik Skultety <eskultet@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;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Fri, Dec 07, 2018 at 10:45:00AM +0100, Ján Tomko wrote:
On Thu, Dec 06, 2018 at 05:12:36PM +0100, Erik Skultety wrote:
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.
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.
Signed-off-by: Erik Skultety <eskultet@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;
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks, Erik
participants (2)
-
Erik Skultety
-
Ján Tomko