[09/10] epoll: Ignore inaccessible targets

Submitted by Cyrill Gorcunov on Feb. 28, 2017, 3:05 p.m.

Details

Message ID 1488294349-2742-10-git-send-email-gorcunov@openvz.org
State New
Series "epoll: Start using new kcmp operation to find targets"
Headers show

Commit Message

Cyrill Gorcunov Feb. 28, 2017, 3:05 p.m.
Not all targets can be restored by now,
so simply ignore them on dump this should
not be a common case.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 criu/eventpoll.c      | 57 +++++++++++++++++++++++++++++++++++++++++----------
 criu/include/fdinfo.h |  1 +
 criu/proc_parse.c     |  1 +
 3 files changed, 48 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/eventpoll.c b/criu/eventpoll.c
index 0515b736603a..670cb06f95bd 100644
--- a/criu/eventpoll.c
+++ b/criu/eventpoll.c
@@ -151,6 +151,7 @@  int dump_eventpoll(void)
 	struct kid_elem *kid;
 
 	list_for_each_entry(dinfo, &eventpoll_fds, list) {
+		unsigned long nr_valid = 0;
 		kcmp_epoll_slot_t slot = {
 			.efd	= dinfo->fd,
 			.toff	= 0,
@@ -167,22 +168,56 @@  int dump_eventpoll(void)
 				kid = fd_kid_epoll_lookup(dinfo->pid,
 							  te->epl.gen_id,
 							  &slot);
-				if (!kid || kid->pid != dinfo->pid) {
-					pr_err("Target %d not found for pid %d "
-					       "(or pid mismatsh %d)\n",
-					       te->epl.e.tfd, dinfo->pid,
-					       kid ? kid->pid : -1);
-					goto out;
+				if (!kid || kid->pid != dinfo->pid ||
+				    kid->idx != te->epl.e.tfd) {
+					pr_warn("Target %d not found for pid %d "
+						"(or pid mismatsh %d), ignoring\n",
+						te->epl.e.tfd, dinfo->pid,
+						kid ? kid->pid : -1);
+					te->epl.valid = 0;
+				} else {
+					pr_debug("Target %d for pid %d matched fd %d\n",
+						 te->epl.e.tfd, dinfo->pid, kid->idx);
+					nr_valid++;
+				}
+			} else {
+				char path[PATH_MAX];
+
+				snprintf(path, sizeof(path), "/proc/%d/fd/%d",
+					 dinfo->pid, slot.tfd);
+				if (access(path, F_OK)) {
+					pr_warn("Target %d not found for pid %d, ignoring\n",
+						te->epl.e.tfd, dinfo->pid);
+					te->epl.valid = 0;
+				} else {
+					pr_debug("Target %d for pid %d is accessible\n",
+						 te->epl.e.tfd, dinfo->pid);
+					nr_valid++;
 				}
-
-				pr_debug("Target %d for pid %d matched fd %d\n",
-					 te->epl.e.tfd, dinfo->pid, kid->idx);
 			}
+		}
+
+		if (nr_valid != dinfo->efe.n_tfd) {
+			size_t i = 0;
 
-			if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
-					 &dinfo->efe, PB_EVENTPOLL_FILE))
+			EventpollTfdEntry **tfd = xmalloc(sizeof(struct EventpollTfdEntry *) * nr_valid);
+			if (!tfd)
 				goto out;
+
+			list_for_each_entry(te, &dinfo->ep_list, epl.node) {
+				if (te->epl.valid)
+					tfd[i++] = &te->epl.e;
+			}
+
+			xfree(dinfo->efe.tfd);
+			dinfo->efe.tfd = tfd;
+			dinfo->efe.n_tfd = nr_valid;
 		}
+
+		pr_info_eventpoll("Dumping ", &dinfo->efe);
+		if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
+				 &dinfo->efe, PB_EVENTPOLL_FILE))
+			goto out;
 	}
 
 	ret = 0;
diff --git a/criu/include/fdinfo.h b/criu/include/fdinfo.h
index be516efce5d6..c9af47eeadc5 100644
--- a/criu/include/fdinfo.h
+++ b/criu/include/fdinfo.h
@@ -30,6 +30,7 @@  struct eventpoll_tfd_entry {
 	struct list_head node;
 	unsigned int gen_id;
 	int use_kcmp;
+	int valid;
 };
 
 union fdinfo_entries {
diff --git a/criu/proc_parse.c b/criu/proc_parse.c
index a96123679aa2..f2255d42e6f7 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -1744,6 +1744,7 @@  static int parse_fdinfo_pid_s(int pid, int fd, int type,
 			e = xmalloc(sizeof(union fdinfo_entries));
 			if (!e)
 				goto out;
+			e->epl.valid = 1;
 
 			eventpoll_tfd_entry__init(&e->epl.e);
 

Comments

Kirill Tkhai March 13, 2017, 11:44 a.m.
On 28.02.2017 18:05, Cyrill Gorcunov wrote:
> Not all targets can be restored by now,
> so simply ignore them on dump this should
> not be a common case.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>  criu/eventpoll.c      | 57 +++++++++++++++++++++++++++++++++++++++++----------
>  criu/include/fdinfo.h |  1 +
>  criu/proc_parse.c     |  1 +
>  3 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/criu/eventpoll.c b/criu/eventpoll.c
> index 0515b736603a..670cb06f95bd 100644
> --- a/criu/eventpoll.c
> +++ b/criu/eventpoll.c
> @@ -151,6 +151,7 @@ int dump_eventpoll(void)
>  	struct kid_elem *kid;
>  
>  	list_for_each_entry(dinfo, &eventpoll_fds, list) {
> +		unsigned long nr_valid = 0;
>  		kcmp_epoll_slot_t slot = {
>  			.efd	= dinfo->fd,
>  			.toff	= 0,
> @@ -167,22 +168,56 @@ int dump_eventpoll(void)
>  				kid = fd_kid_epoll_lookup(dinfo->pid,
>  							  te->epl.gen_id,
>  							  &slot);
> -				if (!kid || kid->pid != dinfo->pid) {
> -					pr_err("Target %d not found for pid %d "
> -					       "(or pid mismatsh %d)\n",
> -					       te->epl.e.tfd, dinfo->pid,
> -					       kid ? kid->pid : -1);
> -					goto out;
> +				if (!kid || kid->pid != dinfo->pid ||
> +				    kid->idx != te->epl.e.tfd) {
> +					pr_warn("Target %d not found for pid %d "
> +						"(or pid mismatsh %d), ignoring\n",
> +						te->epl.e.tfd, dinfo->pid,
> +						kid ? kid->pid : -1);
> +					te->epl.valid = 0;
> +				} else {
> +					pr_debug("Target %d for pid %d matched fd %d\n",
> +						 te->epl.e.tfd, dinfo->pid, kid->idx);
> +					nr_valid++;
> +				}
> +			} else {
> +				char path[PATH_MAX];
> +
> +				snprintf(path, sizeof(path), "/proc/%d/fd/%d",
> +					 dinfo->pid, slot.tfd);
> +				if (access(path, F_OK)) {
> +					pr_warn("Target %d not found for pid %d, ignoring\n",
> +						te->epl.e.tfd, dinfo->pid);
> +					te->epl.valid = 0;
> +				} else {
> +					pr_debug("Target %d for pid %d is accessible\n",
> +						 te->epl.e.tfd, dinfo->pid);
> +					nr_valid++;
>  				}
> -
> -				pr_debug("Target %d for pid %d matched fd %d\n",
> -					 te->epl.e.tfd, dinfo->pid, kid->idx);
>  			}
> +		}
> +
> +		if (nr_valid != dinfo->efe.n_tfd) {
> +			size_t i = 0;
>  
> -			if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
> -					 &dinfo->efe, PB_EVENTPOLL_FILE))
> +			EventpollTfdEntry **tfd = xmalloc(sizeof(struct EventpollTfdEntry *) * nr_valid);


> +			if (!tfd)
>  				goto out;
> +
> +			list_for_each_entry(te, &dinfo->ep_list, epl.node) {
> +				if (te->epl.valid)
> +					tfd[i++] = &te->epl.e;
> +			}
> +
> +			xfree(dinfo->efe.tfd);
> +			dinfo->efe.tfd = tfd;
> +			dinfo->efe.n_tfd = nr_valid;
>  		}
> +
> +		pr_info_eventpoll("Dumping ", &dinfo->efe);
> +		if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
> +				 &dinfo->efe, PB_EVENTPOLL_FILE))

Have you checked that this works correct, when nr_valid == 0 and tfd == NULL?

> +			goto out;
>  	}
>  
>  	ret = 0;
> diff --git a/criu/include/fdinfo.h b/criu/include/fdinfo.h
> index be516efce5d6..c9af47eeadc5 100644
> --- a/criu/include/fdinfo.h
> +++ b/criu/include/fdinfo.h
> @@ -30,6 +30,7 @@ struct eventpoll_tfd_entry {
>  	struct list_head node;
>  	unsigned int gen_id;
>  	int use_kcmp;
> +	int valid;
>  };
>  
>  union fdinfo_entries {
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index a96123679aa2..f2255d42e6f7 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -1744,6 +1744,7 @@ static int parse_fdinfo_pid_s(int pid, int fd, int type,
>  			e = xmalloc(sizeof(union fdinfo_entries));
>  			if (!e)
>  				goto out;
> +			e->epl.valid = 1;
>  
>  			eventpoll_tfd_entry__init(&e->epl.e);
>  
>
Cyrill Gorcunov March 13, 2017, 12:23 p.m.
On Mon, Mar 13, 2017 at 02:44:11PM +0300, Kirill Tkhai wrote:

Suppose nr_valid = 0

> > +
> > +		if (nr_valid != dinfo->efe.n_tfd) {
> > +			size_t i = 0;
> >  
> > -			if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
> > -					 &dinfo->efe, PB_EVENTPOLL_FILE))
> > +			EventpollTfdEntry **tfd = xmalloc(sizeof(struct EventpollTfdEntry *) * nr_valid);
> 
> 
> > +			if (!tfd)
> >  				goto out;

We hit this

> > +
> > +			list_for_each_entry(te, &dinfo->ep_list, epl.node) {
> > +				if (te->epl.valid)
> > +					tfd[i++] = &te->epl.e;
> > +			}
> > +
> > +			xfree(dinfo->efe.tfd);
> > +			dinfo->efe.tfd = tfd;
> > +			dinfo->efe.n_tfd = nr_valid;
> >  		}
> > +
> > +		pr_info_eventpoll("Dumping ", &dinfo->efe);
> > +		if (pb_write_one(img_from_set(glob_imgset, CR_FD_EVENTPOLL_FILE),
> > +				 &dinfo->efe, PB_EVENTPOLL_FILE))
> 
> Have you checked that this works correct, when nr_valid == 0 and tfd == NULL?

We can't reach this point with nr_valid = 0

> 
> > +			goto out;
> >  	}
> >  
> >  	ret = 0;

And ret = -1 will be returned