[v4,05/41] cr-restore: Open transport socket earlier

Submitted by Kirill Tkhai on May 4, 2017, 4:05 p.m.

Details

Message ID 149391394090.11711.4943964071899310854.stgit@localhost.localdomain
State New
Series "Nested pid namespaces support"
Headers show

Commit Message

Kirill Tkhai May 4, 2017, 4:05 p.m.
I need named socket to communicate with pid_ns helpers
(see next patches) and receive answer from them
(it's impossible to send answer to unnamed socket).
As we already have transport socket, we'll reuse it
for the above goal too.

This patch makes transport sockets be created before
creation of children tasks. Also, now it's created
not only for alive tasks.

(It seems, it won't be good to make futex_lock and
futex_mutex union unless we define one as equal type
to other using typedef, but anyway I'd happy to hear
your opinions).

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/cr-restore.c       |    6 +++---
 criu/files.c            |   20 ++++++++++++--------
 criu/include/rst_info.h |    1 +
 criu/pstree.c           |    2 +-
 4 files changed, 17 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index 1a99797dc..9c6d4b041 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -1585,6 +1585,9 @@  static int restore_task_with_children(void *_arg)
 		BUG();
 	}
 
+	if (open_transport_socket())
+		goto err;
+
 	timing_start(TIME_FORK);
 
 	if (create_children_and_session())
@@ -1597,9 +1600,6 @@  static int restore_task_with_children(void *_arg)
 
 	restore_pgid();
 
-	if (open_transport_socket())
-		return -1;
-
 	if (current->parent == NULL) {
 		/*
 		 * Wait when all tasks passed the CR_STATE_FORKING stage.
diff --git a/criu/files.c b/criu/files.c
index 01cd4c0e9..eba506608 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -1325,6 +1325,7 @@  int shared_fdt_prepare(struct pstree_item *item)
 
 		rsti(parent)->fdt = fdt;
 
+		mutex_init(&fdt->fdt_mutex);
 		futex_init(&fdt->fdt_lock);
 		fdt->nr = 1;
 		fdt->pid = vpid(parent);
@@ -1635,29 +1636,32 @@  int open_transport_socket(void)
 	struct fdt *fdt = rsti(current)->fdt;
 	pid_t pid = vpid(current);
 	struct sockaddr_un saddr;
-	int sock, slen;
+	int sock, slen, ret = -1;
 
-	if (!task_alive(current) || (fdt && fdt->pid != pid))
-		return 0;
+	if (fdt)
+		mutex_lock(&fdt->fdt_mutex);
 
 	sock = socket(PF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
 	if (sock < 0) {
 		pr_perror("Can't create socket");
-		return -1;
+		goto out;
 	}
 
 	transport_name_gen(&saddr, &slen, pid);
 	if (bind(sock, (struct sockaddr *)&saddr, slen) < 0) {
 		pr_perror("Can't bind transport socket %s", saddr.sun_path + 1);
 		close(sock);
-		return -1;
+		goto out;
 	}
 
 	if (install_service_fd(TRANSPORT_FD_OFF, sock) < 0) {
 		close(sock);
-		return -1;
+		goto out;
 	}
 	close(sock);
-
-	return 0;
+	ret = 0;
+out:
+	if (fdt)
+		mutex_unlock(&fdt->fdt_mutex);
+	return ret;
 }
diff --git a/criu/include/rst_info.h b/criu/include/rst_info.h
index 92dfc9d93..1860c21fc 100644
--- a/criu/include/rst_info.h
+++ b/criu/include/rst_info.h
@@ -21,6 +21,7 @@  struct fdt {
 	 * The fdt table was restrored, if fdt_lock is equal to nr + 1
 	 */
 	futex_t			fdt_lock;
+	mutex_t			fdt_mutex;
 };
 
 struct _MmEntry;
diff --git a/criu/pstree.c b/criu/pstree.c
index b73f4405a..315f533ba 100644
--- a/criu/pstree.c
+++ b/criu/pstree.c
@@ -258,7 +258,7 @@  struct pstree_item *__alloc_pstree_item(bool rst, int level)
 void init_pstree_helper(struct pstree_item *ret)
 {
 	ret->pid->state = TASK_HELPER;
-	rsti(ret)->clone_flags = CLONE_FILES | CLONE_FS;
+	rsti(ret)->clone_flags = 0;
 	task_entries->nr_helpers++;
 }
 

Comments

Andrey Vagin May 4, 2017, 9:32 p.m.
On Thu, May 04, 2017 at 07:05:40PM +0300, Kirill Tkhai wrote:
> I need named socket to communicate with pid_ns helpers
> (see next patches) and receive answer from them
> (it's impossible to send answer to unnamed socket).
> As we already have transport socket, we'll reuse it
> for the above goal too.
> 
> This patch makes transport sockets be created before
> creation of children tasks. Also, now it's created
> not only for alive tasks.
> 
> (It seems, it won't be good to make futex_lock and
> futex_mutex union unless we define one as equal type
> to other using typedef, but anyway I'd happy to hear
> your opinions).
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/cr-restore.c       |    6 +++---
>  criu/files.c            |   20 ++++++++++++--------
>  criu/include/rst_info.h |    1 +
>  criu/pstree.c           |    2 +-
>  4 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 1a99797dc..9c6d4b041 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1585,6 +1585,9 @@ static int restore_task_with_children(void *_arg)
>  		BUG();
>  	}
>  
> +	if (open_transport_socket())
> +		goto err;
> +
>  	timing_start(TIME_FORK);
>  
>  	if (create_children_and_session())
> @@ -1597,9 +1600,6 @@ static int restore_task_with_children(void *_arg)
>  
>  	restore_pgid();
>  
> -	if (open_transport_socket())
> -		return -1;
> -
>  	if (current->parent == NULL) {
>  		/*
>  		 * Wait when all tasks passed the CR_STATE_FORKING stage.
> diff --git a/criu/files.c b/criu/files.c
> index 01cd4c0e9..eba506608 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -1325,6 +1325,7 @@ int shared_fdt_prepare(struct pstree_item *item)
>  
>  		rsti(parent)->fdt = fdt;
>  
> +		mutex_init(&fdt->fdt_mutex);
>  		futex_init(&fdt->fdt_lock);
>  		fdt->nr = 1;
>  		fdt->pid = vpid(parent);
> @@ -1635,29 +1636,32 @@ int open_transport_socket(void)
>  	struct fdt *fdt = rsti(current)->fdt;
>  	pid_t pid = vpid(current);
>  	struct sockaddr_un saddr;
> -	int sock, slen;
> +	int sock, slen, ret = -1;
>  
> -	if (!task_alive(current) || (fdt && fdt->pid != pid))
> -		return 0;
> +	if (fdt)
> +		mutex_lock(&fdt->fdt_mutex);
>  
>  	sock = socket(PF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0);
>  	if (sock < 0) {
>  		pr_perror("Can't create socket");
> -		return -1;
> +		goto out;
>  	}
>  
>  	transport_name_gen(&saddr, &slen, pid);
>  	if (bind(sock, (struct sockaddr *)&saddr, slen) < 0) {
>  		pr_perror("Can't bind transport socket %s", saddr.sun_path + 1);
>  		close(sock);
> -		return -1;
> +		goto out;
>  	}
>  
>  	if (install_service_fd(TRANSPORT_FD_OFF, sock) < 0) {
>  		close(sock);
> -		return -1;
> +		goto out;
>  	}
>  	close(sock);
> -
> -	return 0;
> +	ret = 0;
> +out:
> +	if (fdt)
> +		mutex_unlock(&fdt->fdt_mutex);
> +	return ret;
>  }
> diff --git a/criu/include/rst_info.h b/criu/include/rst_info.h
> index 92dfc9d93..1860c21fc 100644
> --- a/criu/include/rst_info.h
> +++ b/criu/include/rst_info.h
> @@ -21,6 +21,7 @@ struct fdt {
>  	 * The fdt table was restrored, if fdt_lock is equal to nr + 1
>  	 */
>  	futex_t			fdt_lock;
> +	mutex_t			fdt_mutex;

can you write a comment why we need this mutex
>  };
>  
>  struct _MmEntry;
> diff --git a/criu/pstree.c b/criu/pstree.c
> index b73f4405a..315f533ba 100644
> --- a/criu/pstree.c
> +++ b/criu/pstree.c
> @@ -258,7 +258,7 @@ struct pstree_item *__alloc_pstree_item(bool rst, int level)
>  void init_pstree_helper(struct pstree_item *ret)
>  {
>  	ret->pid->state = TASK_HELPER;
> -	rsti(ret)->clone_flags = CLONE_FILES | CLONE_FS;
> +	rsti(ret)->clone_flags = 0;

Why do we need this ^^^
>  	task_entries->nr_helpers++;
>  }
>  
>