[01/10] files: split collect_fd on allocate_fd and handle_fd

Submitted by Andrei Vagin on Feb. 9, 2017, 8:10 a.m.

Details

Message ID 1486627830-13588-2-git-send-email-avagin@openvz.org
State New
Series "Dump and restore nested network namespaces"
Headers show

Commit Message

Andrei Vagin Feb. 9, 2017, 8:10 a.m.
From: Andrew Vagin <avagin@virtuozzo.com>

We are going to open root directories for each namespace in
the root task, so we need to collect all file descriptors to
be able to find unused file descriptors.

Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 criu/cr-restore.c    | 18 ++++++++++++++++++
 criu/files.c         | 50 +++++++++++++++++++++++++++++++++++++++++---------
 criu/include/files.h |  1 +
 3 files changed, 60 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index 785d724..e7babe9 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -1317,6 +1317,21 @@  static int create_children_and_session(void)
 	return 0;
 }
 
+static int collect_fd_shared()
+{
+	struct pstree_item *pi;
+
+	for_each_pstree_item(pi) {
+		if (pi->pid->state == TASK_HELPER)
+			continue;
+
+		if  (collect_fd_pid(pi))
+			return -1;
+	}
+
+	return 0;
+}
+
 static int restore_task_with_children(void *_arg)
 {
 	struct cr_clone_arg *ca = _arg;
@@ -1404,6 +1419,9 @@  static int restore_task_with_children(void *_arg)
 		pr_info("Calling restore_sid() for init\n");
 		restore_sid();
 
+		if (collect_fd_shared())
+			goto err;
+
 		/*
 		 * We need non /proc proc mount for restoring pid and mount
 		 * namespaces and do not care for the rest of the cases.
diff --git a/criu/files.c b/criu/files.c
index 1a76313..4ef75b3 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -686,19 +686,28 @@  int rst_file_params(int fd, FownEntry *fown, int flags)
 	return 0;
 }
 
-static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
+static struct fdinfo_list_entry *allocate_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
 {
-	struct fdinfo_list_entry *le, *new_le;
-	struct file_desc *fdesc;
+	struct fdinfo_list_entry *new_le;
 
 	pr_info("Collect fdinfo pid=%d fd=%d id=%#x\n",
 		pid, e->fd, e->id);
 
 	new_le = shmalloc(sizeof(*new_le));
 	if (!new_le)
-		return -1;
+		return NULL;
 
 	fle_init(new_le, pid, e);
+	collect_task_fd(new_le, rst_info);
+
+	return new_le;
+}
+
+static int handle_fd(struct fdinfo_list_entry *new_le, struct rst_info *rst_info)
+{
+	struct fdinfo_list_entry *le;
+	struct file_desc *fdesc;
+	FdinfoEntry *e = new_le->fe;
 
 	fdesc = find_file_desc(e);
 	if (fdesc == NULL) {
@@ -713,14 +722,23 @@  static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
 	if (fdesc->ops->collect_fd)
 		fdesc->ops->collect_fd(fdesc, new_le, rst_info);
 
-	collect_task_fd(new_le, rst_info);
-
 	list_add_tail(&new_le->desc_list, &le->desc_list);
 	new_le->desc = fdesc;
 
 	return 0;
 }
 
+static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
+{
+	struct fdinfo_list_entry *le;
+
+	le = allocate_fd(pid, e, rst_info);
+	if (le == NULL)
+		return -1;
+
+	return handle_fd(le, rst_info);
+}
+
 FdinfoEntry *dup_fdinfo(FdinfoEntry *old, int fd, unsigned flags)
 {
 	FdinfoEntry *e;
@@ -777,7 +795,7 @@  int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id)
 	return 0;
 }
 
-int prepare_fd_pid(struct pstree_item *item)
+int collect_fd_pid(struct pstree_item *item)
 {
 	int ret = 0;
 	struct cr_img *img;
@@ -798,6 +816,7 @@  int prepare_fd_pid(struct pstree_item *item)
 		return -1;
 
 	while (1) {
+		struct fdinfo_list_entry *le;
 		FdinfoEntry *e;
 
 		ret = pb_read_one_eof(img, &e, PB_FDINFO);
@@ -810,9 +829,10 @@  int prepare_fd_pid(struct pstree_item *item)
 			break;
 		}
 
-		ret = collect_fd(pid, e, rst_info);
-		if (ret < 0) {
+		le = allocate_fd(pid, e, rst_info);
+		if (le == NULL) {
 			fdinfo_entry__free_unpacked(e, NULL);
+			ret = -1;
 			break;
 		}
 	}
@@ -821,6 +841,18 @@  int prepare_fd_pid(struct pstree_item *item)
 	return ret;
 }
 
+int prepare_fd_pid(struct pstree_item *item)
+{
+	struct rst_info *rst_info = rsti(item);
+	struct fdinfo_list_entry *le, *t;
+	list_for_each_entry_safe(le, t, &rst_info->fds, ps_list) {
+		if (handle_fd(le, rst_info))
+			return -1;
+	}
+
+	return 0;
+}
+
 #define SETFL_MASK (O_APPEND | O_ASYNC | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
 int set_fd_flags(int fd, int flags)
 {
diff --git a/criu/include/files.h b/criu/include/files.h
index 39ef23b..f1216a1 100644
--- a/criu/include/files.h
+++ b/criu/include/files.h
@@ -156,6 +156,7 @@  extern int rst_file_params(int fd, FownEntry *fown, int flags);
 extern void show_saved_files(void);
 
 extern int prepare_fds(struct pstree_item *me);
+extern int collect_fd_pid(struct pstree_item *me);
 extern int prepare_fd_pid(struct pstree_item *me);
 extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
 extern int prepare_shared_fdinfo(void);

Comments

Pavel Emelianov Feb. 10, 2017, 12:16 p.m.
On 02/09/2017 11:10 AM, Andrei Vagin wrote:
> From: Andrew Vagin <avagin@virtuozzo.com>
> 
> We are going to open root directories for each namespace in
> the root task, so we need to collect all file descriptors to
> be able to find unused file descriptors.

In other words we need to call prepare_net_ns() after prepare_fd_pid(),
right?

But the former one is called from prepare_namespace() and the latter
from root_prepare_shared(), which in turn are both called from
restore_task_with_children() in the "wrong" order.

Maybe it's simpler to move prepare_net_ns() call out of prepare_namespaces()
into root_prepare_shared()'s end?

> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  criu/cr-restore.c    | 18 ++++++++++++++++++
>  criu/files.c         | 50 +++++++++++++++++++++++++++++++++++++++++---------
>  criu/include/files.h |  1 +
>  3 files changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 785d724..e7babe9 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1317,6 +1317,21 @@ static int create_children_and_session(void)
>  	return 0;
>  }
>  
> +static int collect_fd_shared()
> +{
> +	struct pstree_item *pi;
> +
> +	for_each_pstree_item(pi) {
> +		if (pi->pid->state == TASK_HELPER)
> +			continue;
> +
> +		if  (collect_fd_pid(pi))
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int restore_task_with_children(void *_arg)
>  {
>  	struct cr_clone_arg *ca = _arg;
> @@ -1404,6 +1419,9 @@ static int restore_task_with_children(void *_arg)
>  		pr_info("Calling restore_sid() for init\n");
>  		restore_sid();
>  
> +		if (collect_fd_shared())
> +			goto err;
> +
>  		/*
>  		 * We need non /proc proc mount for restoring pid and mount
>  		 * namespaces and do not care for the rest of the cases.
> diff --git a/criu/files.c b/criu/files.c
> index 1a76313..4ef75b3 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -686,19 +686,28 @@ int rst_file_params(int fd, FownEntry *fown, int flags)
>  	return 0;
>  }
>  
> -static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> +static struct fdinfo_list_entry *allocate_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>  {
> -	struct fdinfo_list_entry *le, *new_le;
> -	struct file_desc *fdesc;
> +	struct fdinfo_list_entry *new_le;
>  
>  	pr_info("Collect fdinfo pid=%d fd=%d id=%#x\n",
>  		pid, e->fd, e->id);
>  
>  	new_le = shmalloc(sizeof(*new_le));
>  	if (!new_le)
> -		return -1;
> +		return NULL;
>  
>  	fle_init(new_le, pid, e);
> +	collect_task_fd(new_le, rst_info);
> +
> +	return new_le;
> +}
> +
> +static int handle_fd(struct fdinfo_list_entry *new_le, struct rst_info *rst_info)
> +{
> +	struct fdinfo_list_entry *le;
> +	struct file_desc *fdesc;
> +	FdinfoEntry *e = new_le->fe;
>  
>  	fdesc = find_file_desc(e);
>  	if (fdesc == NULL) {
> @@ -713,14 +722,23 @@ static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>  	if (fdesc->ops->collect_fd)
>  		fdesc->ops->collect_fd(fdesc, new_le, rst_info);
>  
> -	collect_task_fd(new_le, rst_info);
> -
>  	list_add_tail(&new_le->desc_list, &le->desc_list);
>  	new_le->desc = fdesc;
>  
>  	return 0;
>  }
>  
> +static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> +{
> +	struct fdinfo_list_entry *le;
> +
> +	le = allocate_fd(pid, e, rst_info);
> +	if (le == NULL)
> +		return -1;
> +
> +	return handle_fd(le, rst_info);
> +}
> +
>  FdinfoEntry *dup_fdinfo(FdinfoEntry *old, int fd, unsigned flags)
>  {
>  	FdinfoEntry *e;
> @@ -777,7 +795,7 @@ int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id)
>  	return 0;
>  }
>  
> -int prepare_fd_pid(struct pstree_item *item)
> +int collect_fd_pid(struct pstree_item *item)
>  {
>  	int ret = 0;
>  	struct cr_img *img;
> @@ -798,6 +816,7 @@ int prepare_fd_pid(struct pstree_item *item)
>  		return -1;
>  
>  	while (1) {
> +		struct fdinfo_list_entry *le;
>  		FdinfoEntry *e;
>  
>  		ret = pb_read_one_eof(img, &e, PB_FDINFO);
> @@ -810,9 +829,10 @@ int prepare_fd_pid(struct pstree_item *item)
>  			break;
>  		}
>  
> -		ret = collect_fd(pid, e, rst_info);
> -		if (ret < 0) {
> +		le = allocate_fd(pid, e, rst_info);
> +		if (le == NULL) {
>  			fdinfo_entry__free_unpacked(e, NULL);
> +			ret = -1;
>  			break;
>  		}
>  	}
> @@ -821,6 +841,18 @@ int prepare_fd_pid(struct pstree_item *item)
>  	return ret;
>  }
>  
> +int prepare_fd_pid(struct pstree_item *item)
> +{
> +	struct rst_info *rst_info = rsti(item);
> +	struct fdinfo_list_entry *le, *t;
> +	list_for_each_entry_safe(le, t, &rst_info->fds, ps_list) {
> +		if (handle_fd(le, rst_info))
> +			return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  #define SETFL_MASK (O_APPEND | O_ASYNC | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
>  int set_fd_flags(int fd, int flags)
>  {
> diff --git a/criu/include/files.h b/criu/include/files.h
> index 39ef23b..f1216a1 100644
> --- a/criu/include/files.h
> +++ b/criu/include/files.h
> @@ -156,6 +156,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
>  extern void show_saved_files(void);
>  
>  extern int prepare_fds(struct pstree_item *me);
> +extern int collect_fd_pid(struct pstree_item *me);
>  extern int prepare_fd_pid(struct pstree_item *me);
>  extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
>  extern int prepare_shared_fdinfo(void);
>
Andrey Vagin Feb. 10, 2017, 9:45 p.m.
On Fri, Feb 10, 2017 at 03:16:46PM +0300, Pavel Emelyanov wrote:
> On 02/09/2017 11:10 AM, Andrei Vagin wrote:
> > From: Andrew Vagin <avagin@virtuozzo.com>
> > 
> > We are going to open root directories for each namespace in
> > the root task, so we need to collect all file descriptors to
> > be able to find unused file descriptors.
> 
> In other words we need to call prepare_net_ns() after prepare_fd_pid(),
> right?
> 
> But the former one is called from prepare_namespace() and the latter
> from root_prepare_shared(), which in turn are both called from
> restore_task_with_children() in the "wrong" order.
> 
> Maybe it's simpler to move prepare_net_ns() call out of prepare_namespaces()
> into root_prepare_shared()'s end?

        /*
         * On netns restore we launch an IP tool, thus we
         * have to restore it _before_ altering the mount
         * tree (i.e. -- mnt_ns restoring)
         */

        id = ns_per_id ? item->ids->net_ns_id : pid; 
        if ((clone_flags & CLONE_NEWNET) && prepare_net_ns(id)) 
                return -1; 

but mntns-es are required to create ghost files.

In a future we probably will need to support bind-mounts for ns files,
so we have to restore all namespaces before mount namespaces.

Actually we can start using fdstore here...

> 
> > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > ---
> >  criu/cr-restore.c    | 18 ++++++++++++++++++
> >  criu/files.c         | 50 +++++++++++++++++++++++++++++++++++++++++---------
> >  criu/include/files.h |  1 +
> >  3 files changed, 60 insertions(+), 9 deletions(-)
> > 
> > diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> > index 785d724..e7babe9 100644
> > --- a/criu/cr-restore.c
> > +++ b/criu/cr-restore.c
> > @@ -1317,6 +1317,21 @@ static int create_children_and_session(void)
> >  	return 0;
> >  }
> >  
> > +static int collect_fd_shared()
> > +{
> > +	struct pstree_item *pi;
> > +
> > +	for_each_pstree_item(pi) {
> > +		if (pi->pid->state == TASK_HELPER)
> > +			continue;
> > +
> > +		if  (collect_fd_pid(pi))
> > +			return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int restore_task_with_children(void *_arg)
> >  {
> >  	struct cr_clone_arg *ca = _arg;
> > @@ -1404,6 +1419,9 @@ static int restore_task_with_children(void *_arg)
> >  		pr_info("Calling restore_sid() for init\n");
> >  		restore_sid();
> >  
> > +		if (collect_fd_shared())
> > +			goto err;
> > +
> >  		/*
> >  		 * We need non /proc proc mount for restoring pid and mount
> >  		 * namespaces and do not care for the rest of the cases.
> > diff --git a/criu/files.c b/criu/files.c
> > index 1a76313..4ef75b3 100644
> > --- a/criu/files.c
> > +++ b/criu/files.c
> > @@ -686,19 +686,28 @@ int rst_file_params(int fd, FownEntry *fown, int flags)
> >  	return 0;
> >  }
> >  
> > -static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> > +static struct fdinfo_list_entry *allocate_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> >  {
> > -	struct fdinfo_list_entry *le, *new_le;
> > -	struct file_desc *fdesc;
> > +	struct fdinfo_list_entry *new_le;
> >  
> >  	pr_info("Collect fdinfo pid=%d fd=%d id=%#x\n",
> >  		pid, e->fd, e->id);
> >  
> >  	new_le = shmalloc(sizeof(*new_le));
> >  	if (!new_le)
> > -		return -1;
> > +		return NULL;
> >  
> >  	fle_init(new_le, pid, e);
> > +	collect_task_fd(new_le, rst_info);
> > +
> > +	return new_le;
> > +}
> > +
> > +static int handle_fd(struct fdinfo_list_entry *new_le, struct rst_info *rst_info)
> > +{
> > +	struct fdinfo_list_entry *le;
> > +	struct file_desc *fdesc;
> > +	FdinfoEntry *e = new_le->fe;
> >  
> >  	fdesc = find_file_desc(e);
> >  	if (fdesc == NULL) {
> > @@ -713,14 +722,23 @@ static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> >  	if (fdesc->ops->collect_fd)
> >  		fdesc->ops->collect_fd(fdesc, new_le, rst_info);
> >  
> > -	collect_task_fd(new_le, rst_info);
> > -
> >  	list_add_tail(&new_le->desc_list, &le->desc_list);
> >  	new_le->desc = fdesc;
> >  
> >  	return 0;
> >  }
> >  
> > +static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
> > +{
> > +	struct fdinfo_list_entry *le;
> > +
> > +	le = allocate_fd(pid, e, rst_info);
> > +	if (le == NULL)
> > +		return -1;
> > +
> > +	return handle_fd(le, rst_info);
> > +}
> > +
> >  FdinfoEntry *dup_fdinfo(FdinfoEntry *old, int fd, unsigned flags)
> >  {
> >  	FdinfoEntry *e;
> > @@ -777,7 +795,7 @@ int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id)
> >  	return 0;
> >  }
> >  
> > -int prepare_fd_pid(struct pstree_item *item)
> > +int collect_fd_pid(struct pstree_item *item)
> >  {
> >  	int ret = 0;
> >  	struct cr_img *img;
> > @@ -798,6 +816,7 @@ int prepare_fd_pid(struct pstree_item *item)
> >  		return -1;
> >  
> >  	while (1) {
> > +		struct fdinfo_list_entry *le;
> >  		FdinfoEntry *e;
> >  
> >  		ret = pb_read_one_eof(img, &e, PB_FDINFO);
> > @@ -810,9 +829,10 @@ int prepare_fd_pid(struct pstree_item *item)
> >  			break;
> >  		}
> >  
> > -		ret = collect_fd(pid, e, rst_info);
> > -		if (ret < 0) {
> > +		le = allocate_fd(pid, e, rst_info);
> > +		if (le == NULL) {
> >  			fdinfo_entry__free_unpacked(e, NULL);
> > +			ret = -1;
> >  			break;
> >  		}
> >  	}
> > @@ -821,6 +841,18 @@ int prepare_fd_pid(struct pstree_item *item)
> >  	return ret;
> >  }
> >  
> > +int prepare_fd_pid(struct pstree_item *item)
> > +{
> > +	struct rst_info *rst_info = rsti(item);
> > +	struct fdinfo_list_entry *le, *t;
> > +	list_for_each_entry_safe(le, t, &rst_info->fds, ps_list) {
> > +		if (handle_fd(le, rst_info))
> > +			return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  #define SETFL_MASK (O_APPEND | O_ASYNC | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
> >  int set_fd_flags(int fd, int flags)
> >  {
> > diff --git a/criu/include/files.h b/criu/include/files.h
> > index 39ef23b..f1216a1 100644
> > --- a/criu/include/files.h
> > +++ b/criu/include/files.h
> > @@ -156,6 +156,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
> >  extern void show_saved_files(void);
> >  
> >  extern int prepare_fds(struct pstree_item *me);
> > +extern int collect_fd_pid(struct pstree_item *me);
> >  extern int prepare_fd_pid(struct pstree_item *me);
> >  extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
> >  extern int prepare_shared_fdinfo(void);
> > 
>