[PATCH v2 15/23] plugins/amdgpu: Fix open_drm_render_device()

Open_drm_render_device() is a little bit confused. It can log confusing
errors using errno which hasn't been set, or it has been overwritten. It
can also inspect errno after it had reset it creating effectively
unreachable code. And finally it sometimes returns the negative errno, and
sometimes a plain -1. Fix it all up and make it consistent, while at the
same time correcting two calls site incorrectly using pr_perror(), while
the function does not guarantee a valid errno.

Signed-off-by: Tvrtko Ursulin <[email protected]>
Reviewed-By: David Francis <[email protected]>
---
 plugins/amdgpu/amdgpu_plugin.c          |  9 +++++----
 plugins/amdgpu/amdgpu_plugin_topology.c | 21 ++++++++++-----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/plugins/amdgpu/amdgpu_plugin.c b/plugins/amdgpu/amdgpu_plugin.c
index 2f63dcfa30a9..f2d1bcf44cab 100644
--- a/plugins/amdgpu/amdgpu_plugin.c
+++ b/plugins/amdgpu/amdgpu_plugin.c
@@ -1599,7 +1599,8 @@ static int restore_devices(struct kfd_ioctl_criu_args *args, CriuKfd *e)
 
 		drm_fd = node_get_drm_render_device(tp_node);
 		if (drm_fd < 0) {
-			pr_perror("Can't open drm render fd %d", tp_node->drm_render_minor);
+			pr_err("Can't open drm render fd for minor %d - %s",
+			       tp_node->drm_render_minor, strerror(-drm_fd));
 			goto exit;
 		} else {
 			pr_info("passing drm render fd = %d to driver\n", drm_fd);
@@ -1907,7 +1908,8 @@ int amdgpu_plugin_restore_file(int id, bool *retry_needed)
 
 		fd = node_get_drm_render_device(tp_node);
 		if (fd < 0) {
-			pr_err("Failed to open render device (minor:%d)\n", tp_node->drm_render_minor);
+			pr_err("Failed to open render device (minor:%d) - %s\n",
+			       tp_node->drm_render_minor, strerror(-fd));
 			return -1;
 		}
 
@@ -2201,9 +2203,8 @@ int init_dev(int dev_minor, amdgpu_device_handle *h_dev, uint64_t *max_copy_size
 	struct amdgpu_gpu_info gpu_info = { 0 };
 
 	drm_fd = open_drm_render_device(dev_minor);
-	if (drm_fd < 0) {
+	if (drm_fd < 0)
 		return drm_fd;
-	}
 
 	ret = amdgpu_device_initialize(drm_fd, &major, &minor, h_dev);
 	if (ret) {
diff --git a/plugins/amdgpu/amdgpu_plugin_topology.c b/plugins/amdgpu/amdgpu_plugin_topology.c
index 730f2e028430..c576cdfa4825 100644
--- a/plugins/amdgpu/amdgpu_plugin_topology.c
+++ b/plugins/amdgpu/amdgpu_plugin_topology.c
@@ -51,31 +51,30 @@ int open_drm_render_device(int minor)
 	int fd, ret_fd;
 
 	if (minor < DRM_FIRST_RENDER_NODE || minor > DRM_LAST_RENDER_NODE) {
-		pr_perror("DRM render minor %d out of range [%d, %d]", minor, DRM_FIRST_RENDER_NODE,
-			  DRM_LAST_RENDER_NODE);
+		pr_err("DRM render minor %d out of range [%d, %d]",
+			   minor, DRM_FIRST_RENDER_NODE, DRM_LAST_RENDER_NODE);
 		return -EINVAL;
 	}
 
 	snprintf(path, sizeof(path), "/dev/dri/renderD%d", minor);
 	fd = open(path, O_RDWR | O_CLOEXEC);
 	if (fd < 0) {
-		if (errno != ENOENT && errno != EPERM) {
-			pr_err("Failed to open %s: %s\n", path, strerror(errno));
-			if (errno == EACCES)
-				pr_err("Check user is in \"video\" group\n");
-		}
-		return -EBADFD;
+		fd = -errno;
+		pr_perror("Failed to open %s", path);
+		return fd;
 	}
 
 	if (fd_next < 0)
 		return fd;
 
 	ret_fd = fcntl(fd, F_DUPFD, fd_next++);
+	if (ret_fd < 0) {
+		ret_fd = -errno;
+		pr_perror("Failed to duplicate fd for minor:%d (fd_next:%d)",
+				  minor, fd_next);
+	}
 	close(fd);
 
-	if (ret_fd < 0)
-		pr_perror("Failed to duplicate fd for minor:%d (fd_next:%d)", minor, fd_next);
-
 	return ret_fd;
 }
 
-- 
2.52.0