[06/10] util: move open_proc_fd to service_fd

Submitted by Andrei Vagin on Feb. 13, 2017, 5:49 a.m.

Details

Message ID 1486964960-4872-7-git-send-email-avagin@openvz.org
State New
Series "Dump and restore nested network namespaces"
Headers show

Commit Message

Andrei Vagin Feb. 13, 2017, 5:49 a.m.
From: Andrei Vagin <avagin@virtuozzo.com>

We need this to avoid conflicts with file descriptors,
which has to be restored.

Currently open_proc_pid() doesn't used during restoring
file descriptors, but we are going to use it to restore
sockets in proper network namespaces.

Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 criu/include/servicefd.h |  1 +
 criu/util.c              | 18 +++++++++++-------
 2 files changed, 12 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
index c070480..c826d37 100644
--- a/criu/include/servicefd.h
+++ b/criu/include/servicefd.h
@@ -9,6 +9,7 @@  enum sfd_type {
 	LOG_FD_OFF,
 	IMG_FD_OFF,
 	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
+	PROC_PID_FD_OFF,
 	CTL_TTY_OFF,
 	SELF_STDIN_OFF,
 	CR_PROC_FD_OFF, /* some other's proc fd.
diff --git a/criu/util.c b/criu/util.c
index eb74be7..6b98a08 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -246,7 +246,6 @@  int move_fd_from(int *img_fd, int want_fd)
  */
 
 static pid_t open_proc_pid = PROC_NONE;
-static int open_proc_fd = -1;
 static pid_t open_proc_self_pid;
 static int open_proc_self_fd = -1;
 
@@ -259,13 +258,18 @@  static inline void set_proc_self_fd(int fd)
 	open_proc_self_pid = getpid();
 }
 
-static inline void set_proc_pid_fd(int pid, int fd)
+static inline int set_proc_pid_fd(int pid, int fd)
 {
-	if (open_proc_fd >= 0)
-		close(open_proc_fd);
+	int ret;
+
+	if (fd < 0)
+		return close_service_fd(PROC_PID_FD_OFF);
 
 	open_proc_pid = pid;
-	open_proc_fd = fd;
+	ret = install_service_fd(PROC_PID_FD_OFF, fd);
+	close(fd);
+
+	return ret;
 }
 
 static inline int get_proc_fd(int pid)
@@ -277,7 +281,7 @@  static inline int get_proc_fd(int pid)
 		}
 		return open_proc_self_fd;
 	} else if (pid == open_proc_pid)
-		return open_proc_fd;
+		return get_service_fd(PROC_PID_FD_OFF);
 	else
 		return -1;
 }
@@ -361,7 +365,7 @@  inline int open_pid_proc(pid_t pid)
 	if (pid == PROC_SELF)
 		set_proc_self_fd(fd);
 	else
-		set_proc_pid_fd(pid, fd);
+		fd = set_proc_pid_fd(pid, fd);
 
 	return fd;
 }

Comments

Pavel Emelianov Feb. 13, 2017, 1:12 p.m.
On 02/13/2017 08:49 AM, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@virtuozzo.com>
> 
> We need this to avoid conflicts with file descriptors,
> which has to be restored.
> 
> Currently open_proc_pid() doesn't used during restoring
> file descriptors, but we are going to use it to restore
> sockets in proper network namespaces.
> 
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  criu/include/servicefd.h |  1 +
>  criu/util.c              | 18 +++++++++++-------
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> index c070480..c826d37 100644
> --- a/criu/include/servicefd.h
> +++ b/criu/include/servicefd.h
> @@ -9,6 +9,7 @@ enum sfd_type {
>  	LOG_FD_OFF,
>  	IMG_FD_OFF,
>  	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
> +	PROC_PID_FD_OFF,
>  	CTL_TTY_OFF,
>  	SELF_STDIN_OFF,
>  	CR_PROC_FD_OFF, /* some other's proc fd.
> diff --git a/criu/util.c b/criu/util.c
> index eb74be7..6b98a08 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -246,7 +246,6 @@ int move_fd_from(int *img_fd, int want_fd)
>   */
>  
>  static pid_t open_proc_pid = PROC_NONE;
> -static int open_proc_fd = -1;
>  static pid_t open_proc_self_pid;
>  static int open_proc_self_fd = -1;

There's also open_proc_self_fd which works in the same manner, why
not moving it to service set?

>  
> @@ -259,13 +258,18 @@ static inline void set_proc_self_fd(int fd)
>  	open_proc_self_pid = getpid();
>  }
>  
> -static inline void set_proc_pid_fd(int pid, int fd)
> +static inline int set_proc_pid_fd(int pid, int fd)
>  {
> -	if (open_proc_fd >= 0)
> -		close(open_proc_fd);
> +	int ret;
> +
> +	if (fd < 0)
> +		return close_service_fd(PROC_PID_FD_OFF);
>  
>  	open_proc_pid = pid;
> -	open_proc_fd = fd;
> +	ret = install_service_fd(PROC_PID_FD_OFF, fd);
> +	close(fd);
> +
> +	return ret;
>  }
>  
>  static inline int get_proc_fd(int pid)
> @@ -277,7 +281,7 @@ static inline int get_proc_fd(int pid)
>  		}
>  		return open_proc_self_fd;
>  	} else if (pid == open_proc_pid)
> -		return open_proc_fd;
> +		return get_service_fd(PROC_PID_FD_OFF);
>  	else
>  		return -1;
>  }
> @@ -361,7 +365,7 @@ inline int open_pid_proc(pid_t pid)
>  	if (pid == PROC_SELF)
>  		set_proc_self_fd(fd);
>  	else
> -		set_proc_pid_fd(pid, fd);
> +		fd = set_proc_pid_fd(pid, fd);
>  
>  	return fd;
>  }
>
Andrey Vagin Feb. 13, 2017, 6:02 p.m.
On Mon, Feb 13, 2017 at 04:12:15PM +0300, Pavel Emelyanov wrote:
> On 02/13/2017 08:49 AM, Andrei Vagin wrote:
> > From: Andrei Vagin <avagin@virtuozzo.com>
> > 
> > We need this to avoid conflicts with file descriptors,
> > which has to be restored.
> > 
> > Currently open_proc_pid() doesn't used during restoring
> > file descriptors, but we are going to use it to restore
> > sockets in proper network namespaces.
> > 
> > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > ---
> >  criu/include/servicefd.h |  1 +
> >  criu/util.c              | 18 +++++++++++-------
> >  2 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/criu/include/servicefd.h b/criu/include/servicefd.h
> > index c070480..c826d37 100644
> > --- a/criu/include/servicefd.h
> > +++ b/criu/include/servicefd.h
> > @@ -9,6 +9,7 @@ enum sfd_type {
> >  	LOG_FD_OFF,
> >  	IMG_FD_OFF,
> >  	PROC_FD_OFF,	/* fd with /proc for all proc_ calls */
> > +	PROC_PID_FD_OFF,
> >  	CTL_TTY_OFF,
> >  	SELF_STDIN_OFF,
> >  	CR_PROC_FD_OFF, /* some other's proc fd.
> > diff --git a/criu/util.c b/criu/util.c
> > index eb74be7..6b98a08 100644
> > --- a/criu/util.c
> > +++ b/criu/util.c
> > @@ -246,7 +246,6 @@ int move_fd_from(int *img_fd, int want_fd)
> >   */
> >  
> >  static pid_t open_proc_pid = PROC_NONE;
> > -static int open_proc_fd = -1;
> >  static pid_t open_proc_self_pid;
> >  static int open_proc_self_fd = -1;
> 
> There's also open_proc_self_fd which works in the same manner, why
> not moving it to service set?

Because nobody uses it on restore. We can add it now or wait when it
will be required. What do you prefer?
> 
> >  
> > @@ -259,13 +258,18 @@ static inline void set_proc_self_fd(int fd)
> >  	open_proc_self_pid = getpid();
> >  }
> >  
> > -static inline void set_proc_pid_fd(int pid, int fd)
> > +static inline int set_proc_pid_fd(int pid, int fd)
> >  {
> > -	if (open_proc_fd >= 0)
> > -		close(open_proc_fd);
> > +	int ret;
> > +
> > +	if (fd < 0)
> > +		return close_service_fd(PROC_PID_FD_OFF);
> >  
> >  	open_proc_pid = pid;
> > -	open_proc_fd = fd;
> > +	ret = install_service_fd(PROC_PID_FD_OFF, fd);
> > +	close(fd);
> > +
> > +	return ret;
> >  }
> >  
> >  static inline int get_proc_fd(int pid)
> > @@ -277,7 +281,7 @@ static inline int get_proc_fd(int pid)
> >  		}
> >  		return open_proc_self_fd;
> >  	} else if (pid == open_proc_pid)
> > -		return open_proc_fd;
> > +		return get_service_fd(PROC_PID_FD_OFF);
> >  	else
> >  		return -1;
> >  }
> > @@ -361,7 +365,7 @@ inline int open_pid_proc(pid_t pid)
> >  	if (pid == PROC_SELF)
> >  		set_proc_self_fd(fd);
> >  	else
> > -		set_proc_pid_fd(pid, fd);
> > +		fd = set_proc_pid_fd(pid, fd);
> >  
> >  	return fd;
> >  }
> > 
>