[libvirt] [PATCH v3 01/11] util: Introduce virHostGetDRMRenderNode helper

[Erik Skultety]

On Mon, Dec 03, 2018 at 03:18:47PM +0000, Daniel P. Berrangé wrote:
> On Thu, Nov 29, 2018 at 03:20:11PM +0100, Erik Skultety wrote:
> > This is the first step towards libvirt picking the first available
> > render node instead of QEMU. It also makes sense for us to be able to do
> > that, since we allow specifying the node directly for SPICE, so if
> > there's no render node specified by the user, we should pick the first
> > available one. The algorithm used for that is essentially the same as
> > the one QEMU uses.
> >
> > Signed-off-by: Erik Skultety <eskultet at redhat.com>
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/virutil.c       | 53 ++++++++++++++++++++++++++++++++++++++++
> >  src/util/virutil.h       |  2 ++
> >  3 files changed, 56 insertions(+)
> >
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 5018a13e9c..c7f08f9620 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -3151,6 +3151,7 @@ virGetUserName;
> >  virGetUserRuntimeDirectory;
> >  virGetUserShell;
> >  virHexToBin;
> > +virHostGetDRMRenderNode;
> >  virHostHasIOMMU;
> >  virIndexToDiskName;
> >  virIsDevMapperDevice;
> > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > index 974cffc2ee..da12a11e04 100644
> > --- a/src/util/virutil.c
> > +++ b/src/util/virutil.c
> > @@ -2145,3 +2145,56 @@ virHostHasIOMMU(void)
> >      VIR_DIR_CLOSE(iommuDir);
> >      return ret;
> >  }
> > +
> > +
> > +/**
> > + * virHostGetDRMRenderNode:
> > + *
> > + * Picks the first DRM render node available. Missing DRI or missing DRM render
> > + * nodes in the system results in an error.
> > + *
> > + * Returns an absolute path to the first render node available or NULL in case
> > + * of an error with the error being reported.
> > + * Caller is responsible for freeing the result string.
> > + *
> > + */
> > +char *
> > +virHostGetDRMRenderNode(void)
> > +{
> > +    char *ret = NULL;
> > +    DIR *driDir = NULL;
> > +    const char *driPath = "/dev/dri";
> > +    struct dirent *ent = NULL;
> > +    int dirErr = 0;
> > +    bool have_rendernode = false;
> > +
> > +    if (virDirOpen(&driDir, driPath) < 0)
> > +        return NULL;
> > +
> > +    while ((dirErr = virDirRead(driDir, &ent, driPath)) > 0) {
> > +        if (ent->d_type != DT_CHR)
> > +            continue;
>
> The 'd_type' field is a Linux-ism, so this has broken the build
>
> Is it even needed ? ie are then any files in /dev/dri that have
> a prefix of "renderD" but which are not character devices ?

Can't say it's impossible, but I haven't bumped into a setup like that - I had
different permissions, I had DRI missing, I had renderD missing, but I haven't
seen a non-character DRI node...You're right in the thinking, the reason why I
put it in there is to spare a few cycles wasted on string comparison, so either
I can shrug my shoulders, remove the check, waste the cycles and be happy or I
can have a look whether gnulib can help us out here.

Erik