backend/drm: Do not open new file descriptors in `DrmNode`

`DrmNode` was made to manage open drm nodes (including render nodes)
without having to create a `DrmDevice`. Even if opening render nodes
*should* work in general on most systems, the user should be in charge
of opening device, because they *may* want to use a `Session` or
something else. Therefor remove all `open` calls from drm/node.

The X11-backend, which tries to optain a render node, now opens the
render node itself, if getting a path proves to be successful.

src/backend/drm/mod.rs

@@ -71,14 +71,14 @@
 
 pub(crate) mod device;
 pub(self) mod error;
-pub(self) mod node;
+pub mod node;
 #[cfg(feature = "backend_session")]
 pub(self) mod session;
 pub(self) mod surface;
 
 pub use device::{DevPath, DrmDevice, DrmEvent};
 pub use error::Error as DrmError;
-pub use node::{ConvertErrorKind, ConvertNodeError, CreateDrmNodeError, DrmNode, NodeType};
+pub use node::{CreateDrmNodeError, DrmNode, NodeType};
 #[cfg(feature = "backend_gbm")]
 pub use surface::gbm::{Error as GbmBufferedSurfaceError, GbmBufferedSurface};
 pub use surface::DrmSurface;

src/backend/drm/node/constants.rs

@@ -37,9 +37,3 @@ pub const RENDER_NAME: &str = "renderD";
 
 #[cfg(target_os = "freebsd")]
 pub const RENDER_NAME: &str = "drmR";
-
-#[cfg(not(target_os = "openbsd"))]
-pub const DIR_NAME: &str = "dev";
-
-#[cfg(target_os = "openbsd")]
-pub const DIR_NAME: &str = "dev/dri";

src/backend/drm/node/mod.rs

@@ -1,3 +1,5 @@
+//! Module for abstractions on drm device nodes
+
 pub(crate) mod constants;
 
 use constants::*;
@@ -7,12 +9,11 @@ use std::{
     fmt::{self, Display, Formatter},
     fs, io,
     os::unix::prelude::{AsRawFd, IntoRawFd, RawFd},
-    path::PathBuf,
+    path::{Path, PathBuf},
 };
 
 use nix::{
-    fcntl::{self, OFlag},
+    sys::stat::{fstat, major, minor, stat},
-    sys::stat::{fstat, major, minor, Mode},
     unistd::close,
 };
 
@@ -62,15 +63,6 @@ impl DrmNode {
         })
     }
 
-    /// Creates a DRM node of the specified type using the same DRM device as the provided node.
-    /// The provided node will be consumed if the new node is successfully created.
-    ///
-    /// This function is useful for obtaining the render node of a DRM device when the provided
-    /// node is a primary node.
-    pub fn from_node_with_type(node: DrmNode, ty: NodeType) -> Result<DrmNode, ConvertNodeError> {
-        from_node_with_type(node, ty)
-    }
-
     /// Returns the type of the DRM node.
     pub fn ty(&self) -> NodeType {
         self.ty
@@ -83,7 +75,12 @@ impl DrmNode {
 
     /// Returns the path of the open device if possible.
     pub fn dev_path(&self) -> Option<PathBuf> {
-        fs::read_link(format!("/proc/self/fd/{:?}", self.as_raw_fd())).ok()
+        node_path(self, self.ty).ok()
+    }
+
+    /// Returns the path of the specified node type matching the open device if possible.
+    pub fn dev_path_with_type(&self, ty: NodeType) -> Option<PathBuf> {
+        node_path(self, ty).ok()
     }
 
     /// Returns the major device number of the DRM device.
@@ -206,54 +203,14 @@ impl From<io::Error> for CreateDrmNodeError {
     }
 }
 
-/// A DRM node could not be used to get an alternative type of node.
+/// Returns if the given device by major:minor pair is a drm device
-///
-/// If this error is returned, the original DRM node may be retrieved using [`ConvertNodeError::node`].
-#[derive(Debug, thiserror::Error)]
-#[error("{kind}")]
-pub struct ConvertNodeError {
-    node: DrmNode,
-    kind: ConvertErrorKind,
-}
-
-impl ConvertNodeError {
-    /// Returns the original DrmNode.
-    pub fn node(self) -> DrmNode {
-        self.node
-    }
-
-    /// Returns the kind of error that occurred when obtaining a different type of DRM node.
-    pub fn kind(&self) -> &ConvertErrorKind {
-        &self.kind
-    }
-}
-
-/// An error that may occur when obtaining a different type of DRM node.
-#[derive(Debug, thiserror::Error)]
-pub enum ConvertErrorKind {
-    /// The requested node type is not available.
-    #[error("the requested node type, {0}, is not available.")]
-    NodeTypeNotAvailable(NodeType),
-
-    /// An IO error occurred when when obtaining a different type of DRM node.
-    #[error("{0}")]
-    Io(io::Error),
-}
-
-impl From<io::Error> for ConvertErrorKind {
-    fn from(err: io::Error) -> Self {
-        ConvertErrorKind::Io(err)
-    }
-}
-
 #[cfg(target_os = "linux")]
-fn is_device_drm(major: u64, minor: u64) -> bool {
+pub fn is_device_drm(major: u64, minor: u64) -> bool {
-    use nix::sys::stat::stat;
-
     let path = format!("/sys/dev/char/{}:{}/device/drm", major, minor);
     stat(path.as_str()).is_ok()
 }
 
+/// Returns if the given device by major:minor pair is a drm device
 #[cfg(target_os = "freebsd")]
 pub fn is_device_drm(major: u64, _minor: u64) -> bool {
     use nix::sys::stat::makedev;
@@ -287,71 +244,44 @@ pub fn is_device_drm(major: u64, _minor: u64) -> bool {
         || dev_name.starts_with("dri/renderD")
 }
 
+/// Returns if the given device by major:minor pair is a drm device
 #[cfg(not(any(target_os = "linux", target_os = "freebsd")))]
 pub fn is_device_drm(major: u64, _minor: u64) -> bool {
     major == DRM_MAJOR
 }
 
+/// Returns the path of a specific type of node from the same DRM device as another path of the same node.
 #[cfg(target_os = "linux")]
-fn from_node_with_type(node: DrmNode, ty: NodeType) -> Result<DrmNode, ConvertNodeError> {
+pub fn path_to_type<P: AsRef<Path>>(path: P, ty: NodeType) -> io::Result<PathBuf> {
-    match node_path(&node, ty) {
+    let stat = stat(path.as_ref()).map_err(Into::<io::Error>::into)?;
-        Ok(path) => {
+    let dev = stat.st_rdev;
-            let fd = match fcntl::open(&path, OFlag::O_RDWR | OFlag::O_CLOEXEC, Mode::empty())
+    let major = major(dev);
-                .map_err(Into::<io::Error>::into)
+    let minor = minor(dev);
-                .map_err(Into::<ConvertErrorKind>::into)
-            {
-                Ok(fd) => fd,
-                Err(kind) => return Err(ConvertNodeError { node, kind }),
-            };
 
-            let result = DrmNode::from_fd(fd);
+    dev_path(major, minor, ty)
-
-            match result {
-                Ok(node) => Ok(node),
-
-                // Some error occurred while opening the new node
-                Err(CreateDrmNodeError::Io(err)) => Err(ConvertNodeError {
-                    node,
-                    kind: err.into(),
-                }),
-
-                _ => unreachable!(),
-            }
-        }
-
-        Err(err) => Err(ConvertNodeError {
-            node,
-            kind: err.into(),
-        }),
-    }
-}
-
-#[cfg(target_os = "freebsd")]
-fn from_node_with_type(node: DrmNode, ty: NodeType) -> Result<DrmNode, ConvertNodeError> {
-    // TODO: Not implemented yet, so fail conversion
-    Err(ConvertNodeError {
-        node,
-        kind: ConvertErrorKind::NodeTypeNotAvailable(ty),
-    })
-}
-
-#[cfg(not(any(target_os = "linux", target_os = "freebsd")))]
-fn from_node_with_type(node: DrmNode, ty: NodeType) -> Result<DrmNode, ConvertNodeError> {
-    // TODO: Not implemented yet, so fail conversion
-    Err(ConvertNodeError {
-        node,
-        kind: ConvertErrorKind::NodeTypeNotAvailable(ty),
-    })
 }
 
 /// Returns the path of a specific type of node from the same DRM device as an existing DrmNode.
 #[cfg(target_os = "linux")]
-fn node_path(node: &DrmNode, ty: NodeType) -> io::Result<PathBuf> {
+pub fn node_path(node: &DrmNode, ty: NodeType) -> io::Result<PathBuf> {
-    use std::io::ErrorKind;
-
     let major = node.major();
     let minor = node.minor();
 
+    dev_path(major, minor, ty)
+}
+
+/// Returns the path of a specific type of node from the DRM device described by major and minor device numbers.
+#[cfg(target_os = "linux")]
+pub fn dev_path(major: u64, minor: u64, ty: NodeType) -> io::Result<PathBuf> {
+    use std::io::ErrorKind;
+
+    if !is_device_drm(major, minor) {
+        return Err(io::Error::new(
+            ErrorKind::NotFound,
+            format!("{}:{} is no DRM device", major, minor),
+        ));
+    }
+
     let read = fs::read_dir(format!("/sys/dev/char/{}:{}/device/drm", major, minor))?;
 
     for entry in read.flatten() {
@@ -361,9 +291,7 @@ fn node_path(node: &DrmNode, ty: NodeType) -> io::Result<PathBuf> {
         // Only 1 primary, control and render node may exist simultaneously, so the
         // first occurrence is good enough.
         if name.starts_with(ty.minor_name_prefix()) {
-            let mut path = entry.path();
+            let path = [r"/", "dev", "dri", &name].iter().collect::<PathBuf>();
-            path.push(DIR_NAME);
-
             if path.exists() {
                 return Ok(path);
             }

src/backend/x11/mod.rs

@@ -58,7 +58,7 @@ use self::{buffer::PixmapWrapperExt, window_inner::WindowInner};
 use crate::{
     backend::{
         allocator::dmabuf::{AsDmabuf, Dmabuf},
-        drm::{DrmNode, NodeType},
+        drm::{node::path_to_type, CreateDrmNodeError, DrmNode, NodeType},
         input::{Axis, ButtonState, InputEvent, KeyState},
     },
     utils::{x11rb::X11Source, Logical, Size},
@@ -66,7 +66,10 @@ use crate::{
 use calloop::{EventSource, Poll, PostAction, Readiness, Token, TokenFactory};
 use drm_fourcc::DrmFourcc;
 use gbm::BufferObjectFlags;
-use nix::fcntl;
+use nix::{
+    fcntl::{self, OFlag},
+    sys::stat::Mode,
+};
 use slog::{error, info, o, Logger};
 use std::{
     io, mem,
@@ -298,8 +301,7 @@ impl X11Surface {
         resize: Receiver<Size<u16, Logical>>,
     ) -> Result<X11Surface, X11Error> {
         let connection = &backend.connection;
-        let window = backend.window();
+     
-
         // Determine which drm-device the Display is using.
         let screen = &connection.setup().roots[backend.screen()];
         // provider being NONE tells the X server to use the RandR provider.
@@ -320,20 +322,38 @@ impl X11Surface {
                 });
             }
         };
-
+     
         // Take ownership of the container's inner value so we do not need to duplicate the fd.
         // This is fine because the X server will always open a new file descriptor.
         let drm_device_fd = dri3.device_fd.into_raw_fd();
-
+     
         let fd_flags =
             fcntl::fcntl(drm_device_fd.as_raw_fd(), fcntl::F_GETFD).map_err(AllocateBuffersError::from)?;
-
+     
         // Enable the close-on-exec flag.
         fcntl::fcntl(
             drm_device_fd,
             fcntl::F_SETFD(fcntl::FdFlag::from_bits_truncate(fd_flags) | fcntl::FdFlag::FD_CLOEXEC),
         )
         .map_err(AllocateBuffersError::from)?;
+        let mut drm_node = DrmNode::from_fd(drm_device_fd).map_err(Into::<AllocateBuffersError>::into)?;
+     
+        if drm_node.ty() != NodeType::Render {
+            if drm_node.has_render() {
+                // Try to get the render node.
+                if let Some(path) = drm_node.dev_path_with_type(NodeType::Render) {
+                    if let Ok(node) = fcntl::open(&path, OFlag::O_RDWR | OFlag::O_CLOEXEC, Mode::empty())
+                        .map_err(Into::<std::io::Error>::into)
+                        .map_err(CreateDrmNodeError::Io)
+                        .and_then(DrmNode::from_fd)
+                    {
+                        drm_node = node;
+                    } else {
+                        slog::warn!(&backend.log, "Could not create render node from existing DRM node, falling back to primary node");    
+                    }
+                }
+            }
+        }
 
         // Kernel documentation explains why we should prefer the node to be a render node:
         // https://kernel.readthedocs.io/en/latest/gpu/drm-uapi.html
@@ -349,27 +369,6 @@ impl X11Surface {
         //
         // Of course if the DRM device does not support render nodes, no DRIVER_RENDER capability, then
         // fall back to the primary node.
-        let drm_node = DrmNode::from_fd(drm_device_fd).map_err(Into::<AllocateBuffersError>::into)?;
-        let drm_node = if drm_node.ty() != NodeType::Render {
-            if drm_node.has_render() {
-                // Try to get the render node.
-                match DrmNode::from_node_with_type(drm_node, NodeType::Render) {
-                    Ok(node) => node,
-                    Err(err) => {
-                        slog::warn!(&backend.log, "Could not create render node from existing DRM node, falling back to primary node");
-                        err.node()
-                    }
-                }
-            } else {
-                slog::warn!(
-                    &backend.log,
-                    "DRM Device does not have a render node, falling back to primary node"
-                );
-                drm_node
-            }
-        } else {
-            drm_node
-        };
 
         // Finally create a GBMDevice to manage the buffers.
         let device = gbm::Device::new(drm_node).map_err(Into::<AllocateBuffersError>::into)?;
@@ -388,8 +387,8 @@ impl X11Surface {
             .map_err(Into::<AllocateBuffersError>::into)?;
 
         Ok(X11Surface {
-            connection: Arc::downgrade(connection),
+            connection: Arc::downgrade(&backend.connection),
-            window,
+            window: backend.window(),
             device,
             format,
             width: size.w,