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

Erik Skultety posted 1 patch 5 years, 3 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b97b5660fb1fbe8dfe5312adfeb03068e2358b6b.1544112752.git.eskultet@redhat.com
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[libvirt] [PATCH] domain: conf: graphics: Fix picking DRI renderer automatically for SPICE
Posted by Erik Skultety 5 years, 3 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain: conf: graphics: Fix picking DRI renderer automatically for SPICE
Posted by Ján Tomko 5 years, 3 months ago
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.
>
>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.

>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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain: conf: graphics: Fix picking DRI renderer automatically for SPICE
Posted by Erik Skultety 5 years, 3 months ago
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.
> >
> > 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.

>
> > 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

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