[CRIU] [PATCH] util: use F_DUPFD when we don't want to overwrite an existing descriptor

[Andrei Vagin]

Right now we use fcntl(F_GETFD) to check whether a target descriptor
is used and then we call dup2(). Actually, we can do this for one system
call.

Cc: Cyrill Gorcunov <gorcunov at openvz.org>
Signed-off-by: Andrei Vagin <avagin at gmail.com>
---
 criu/include/servicefd.h | 11 -----------
 criu/servicefd.c         | 37 +++++++++++++++++++++++--------------
 criu/util.c              | 18 +++++++++---------
 3 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
index 7be472cf4..986c46af5 100644
--- a/criu/include/servicefd.h
+++ b/criu/include/servicefd.h
@@ -35,17 +35,6 @@ struct pstree_item;
 extern bool sfds_protected;
 
 
-#define sfd_verify_target(_type, _old_fd, _new_fd)			\
-	({								\
-		int __ret = 0;						\
-		if (fcntl(_new_fd, F_GETFD) != -1 && errno != EBADF) {	\
-			pr_err("%s busy target %d -> %d\n",		\
-			       sfd_type_name(_type), _old_fd, _new_fd);	\
-			__ret = -1;					\
-		}							\
-		__ret;							\
-	})
-
 extern const char *sfd_type_name(enum sfd_type type);
 extern int init_service_fd(void);
 extern int get_service_fd(enum sfd_type type);
diff --git a/criu/servicefd.c b/criu/servicefd.c
index 82147921c..dc423895b 100644
--- a/criu/servicefd.c
+++ b/criu/servicefd.c
@@ -153,6 +153,7 @@ static void sfds_protection_bug(enum sfd_type type)
 int install_service_fd(enum sfd_type type, int fd)
 {
 	int sfd = __get_service_fd(type, service_fd_id);
+	int tmp;
 
 	BUG_ON((int)type <= SERVICE_FD_MIN || (int)type >= SERVICE_FD_MAX);
 	if (sfds_protected && !test_bit(type, sfd_map))
@@ -166,16 +167,19 @@ int install_service_fd(enum sfd_type type, int fd)
 		return fd;
 	}
 
-	if (!test_bit(type, sfd_map)) {
-		if (sfd_verify_target(type, fd, sfd))
-			return -1;
-	}
-
-	if (dup3(fd, sfd, O_CLOEXEC) != sfd) {
+	if (!test_bit(type, sfd_map))
+		tmp = fcntl(fd, F_DUPFD, sfd);
+	else
+		tmp = dup3(fd, sfd, O_CLOEXEC);
+	if (tmp < 0) {
 		pr_perror("%s dup %d -> %d failed",
 			  sfd_type_name(type), fd, sfd);
 		close(fd);
 		return -1;
+	} else if (tmp != sfd) {
+		pr_err("%s busy target %d -> %d\n", sfd_type_name(type), fd, sfd);
+		close(fd);
+		return -1;
 	}
 
 	set_bit(type, sfd_map);
@@ -201,25 +205,30 @@ int close_service_fd(enum sfd_type type)
 	return 0;
 }
 
-static void move_service_fd(struct pstree_item *me, int type, int new_id, int new_base)
+static int move_service_fd(struct pstree_item *me, int type, int new_id, int new_base)
 {
 	int old = get_service_fd(type);
 	int new = new_base - type - SERVICE_FD_MAX * new_id;
 	int ret;
 
 	if (old < 0)
-		return;
+		return 0;
 
 	if (!test_bit(type, sfd_map))
-		sfd_verify_target(type, old, new);
-
-	ret = dup2(old, new);
+		ret = fcntl(old, F_DUPFD, new);
+	else
+		ret = dup2(old, new);
 	if (ret == -1) {
-		if (errno != EBADF)
-			pr_perror("%s unable to clone %d->%d",
-				  sfd_type_name(type), old, new);
+		pr_perror("%s unable to clone %d->%d",
+			  sfd_type_name(type), old, new);
+		return -1;
+	} else if (ret != new) {
+		pr_err("%s busy target %d -> %d\n", sfd_type_name(type), old, new);
+		return -1;
 	} else if (!(rsti(me)->clone_flags & CLONE_FILES))
 		close(old);
+
+	return 0;
 }
 
 static int choose_service_fd_base(struct pstree_item *me)
diff --git a/criu/util.c b/criu/util.c
index 41b6915b7..8faffb859 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -229,19 +229,19 @@ int reopen_fd_as_safe(char *file, int line, int new_fd, int old_fd, bool allow_r
 	int tmp;
 
 	if (old_fd != new_fd) {
-		if (!allow_reuse_fd) {
-			if (fcntl(new_fd, F_GETFD) != -1 || errno != EBADF) {
-				pr_err("fd %d already in use (called at %s:%d)\n",
-					new_fd, file, line);
-				return -1;
-			}
-		}
-
-		tmp = dup2(old_fd, new_fd);
+		if (!allow_reuse_fd)
+			tmp = fcntl(old_fd, F_DUPFD, new_fd);
+		else
+			tmp = dup2(old_fd, new_fd);
 		if (tmp < 0) {
 			pr_perror("Dup %d -> %d failed (called at %s:%d)",
 				  old_fd, new_fd, file, line);
 			return tmp;
+		} else if (tmp != new_fd) {
+			close(tmp);
+			pr_err("fd %d already in use (called at %s:%d)\n",
+				new_fd, file, line);
+			return -1;
 		}
 
 		/* Just to have error message if failed */
-- 
2.20.1