Re: [PATCH v2] virt-aa-helper: generate rules for gl enabled graphics devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 22, 2019 at 2:25 PM Erik Skultety <eskultet@xxxxxxxxxx> wrote:
>
> On Tue, Jan 22, 2019 at 11:30:50AM +0100, Christian Ehrhardt wrote:
> > This adds the virt-aa-helper support for gl enabled graphics devices to
> > generate rules for the needed rendernode paths.
> >
> > Example in domain xml:
> > <graphics type='spice'>
> >   <gl enable='yes' rendernode='/dev/dri/bar'/>
> > </graphics>
> >
> > results in:
> >   "/dev/dri/bar" rw,
> >
> > Special cases are:
> > - multiple devices with rendernodes -> all are added
> > - non explicit rendernodes -> follow recently added virHostGetDRMRenderNode
> > - rendernode without opengl (in egl-headless for example) -> still add
> >   the node
> >
> > Updates in V2:
> > - avoid leaking rendernode
> > - logically group patch hunks
> > - drop base /dev/dri access
> > - simplify detection of the need for a default rendernode
> > - add tests in tests/virt-aa-helper-test
> > - rebased to recent master
>
> ^This is irrelevant to the git history, so should be dropped before pushing

I added it with an --amend on the submission as I had no 0/X patch to
mention it.
Will not be in the push once ack's are in.

> >
> > Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1757085
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx>
> > ---
> >  src/security/virt-aa-helper.c | 12 ++++++++++++
> >  tests/virt-aa-helper-test     |  6 ++++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> > index 64a425671d..a84dd5a8c9 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -1062,6 +1062,18 @@ get_files(vahControl * ctl)
> >      for (i = 0; i < ctl->def->ngraphics; i++) {
> >          virDomainGraphicsDefPtr graphics = ctl->def->graphics[i];
> >          size_t n;
> > +        const char *rendernode = virDomainGraphicsGetRenderNode(graphics);
> > +
> > +        if (rendernode)
> > +            vah_add_file(&buf, rendernode, "rw");
> > +        else
> > +            if (virDomainGraphicsNeedsAutoRenderNode(graphics)) {
> > +                char * defaultRenderNode = virHostGetDRMRenderNode();
> > +                if (defaultRenderNode) {
> > +                    vah_add_file(&buf, defaultRenderNode, "rw");
> > +                    VIR_FREE(defaultRenderNode);
> > +            }
> > +        }
>
> Minor guideline nitpick about curly braces around complex bodies, with the
> following squashed in:

I was making syntaxcheck happy, but yours looks better - Integrated in
what would be pushed, thanks!

>
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index a84dd5a8c9..78666db4f8 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1064,14 +1064,15 @@ get_files(vahControl * ctl)
>          size_t n;
>          const char *rendernode = virDomainGraphicsGetRenderNode(graphics);
>
> -        if (rendernode)
> +        if (rendernode) {
>              vah_add_file(&buf, rendernode, "rw");
> -        else
> +        } else {
>              if (virDomainGraphicsNeedsAutoRenderNode(graphics)) {
> -                char * defaultRenderNode = virHostGetDRMRenderNode();
> +                char *defaultRenderNode = virHostGetDRMRenderNode();
> +
>                  if (defaultRenderNode) {
>                      vah_add_file(&buf, defaultRenderNode, "rw");
>                      VIR_FREE(defaultRenderNode);
> +                }
>              }
>          }
>
> Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux