[v2,0/9] epoll: Add support for duped targets

Submitted by Andrei Vagin on June 25, 2018, 5 p.m.

Details

Message ID 20180625170023.GA23863@outlook.office365.com
State New
Headers show

Patch hide | download patch | download mbox

diff --git a/test/zdtm/static/eventfs00.c b/test/zdtm/static/eventfs00.c
index 61d26e90b..7261050db 100644
--- a/test/zdtm/static/eventfs00.c
+++ b/test/zdtm/static/eventfs00.c
@@ -37,6 +37,7 @@  int main(int argc, char *argv[])
 	int efd, ret, epollfd, fd;
 	uint64_t v = EVENTFD_INITIAL;
 	struct epoll_event ev;
+	int i;
 
 	test_init(argc, argv);
 
@@ -55,33 +56,36 @@  int main(int argc, char *argv[])
 	memset(&ev, 0xff, sizeof(ev));
 	ev.events = EPOLLIN | EPOLLOUT;
 
-	if (pipe(pipefd1) || pipe(pipefd2)) {
-		fail("pipe");
-		exit(1);
-	}
-
-	test_msg("epoll %d add %d native\n", epollfd, pipefd1[0]);
-	if (epoll_ctl(epollfd, EPOLL_CTL_ADD, pipefd1[0], &ev)) {
-		fail("epoll_ctl");
-		exit(1);
+	for (i = 0; i < 100; i++) {
+		if (pipe(pipefd1) || pipe(pipefd2)) {
+			fail("pipe");
+			exit(1);
+		}
+
+		test_msg("epoll %d add %d native\n", epollfd, pipefd1[0]);
+		if (epoll_ctl(epollfd, EPOLL_CTL_ADD, pipefd1[0], &ev)) {
+			fail("epoll_ctl");
+			exit(1);
+		}
+
+		fd = dup2(pipefd2[0], 999);
+		if (fd < 0) {
+			fail(" dup on pipe");
+			exit(1);
+		}
+
+		test_msg("epoll %d add %d dup'ed from %d\n", epollfd, fd, pipefd2[0]);
+		if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev)) {
+			fail("epoll_ctl on duped pipe");
+			exit(1);
+		}
+		close(fd);
+		test_msg("epoll source %d closed\n", fd);
+
+
+		test_msg("created eventfd with %"PRIu64"\n", v);
 	}
 
-	fd = dup2(pipefd2[0], 64);
-	if (fd < 0) {
-		fail(" dup on pipe");
-		exit(1);
-	}
-
-	test_msg("epoll %d add %d dup'ed from %d\n", epollfd, fd, pipefd2[0]);
-	if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev)) {
-		fail("epoll_ctl on duped pipe");
-		exit(1);
-	}
-	close(fd);
-	test_msg("epoll source %d closed\n", fd);
-
-	test_msg("created eventfd with %"PRIu64"\n", v);
-
 	ret = write(efd, &v, sizeof(v));
 	if (ret != sizeof(v)) {
 		fail("write");

Comments

Cyrill Gorcunov June 25, 2018, 5:55 p.m.
On Mon, Jun 25, 2018 at 10:00:24AM -0700, Andrey Vagin wrote:
> This test doesn't pass my 5-minutes test
> 

This series is *not* intended to cover all possible scenarios,
in particular what you're doing in your patch -- you do assign
multiple same numberder fd into epoll. It won't work on this
series, as expected. I not sure yet what is better way to
cover such scenario. But it definitely gonna be implemented
on top, so I encourage to apply the series since without it
we can't even be sure what the files we're putting into the
epoll.
Andrei Vagin June 25, 2018, 5:57 p.m.
On Mon, Jun 25, 2018 at 08:55:08PM +0300, Cyrill Gorcunov wrote:
> On Mon, Jun 25, 2018 at 10:00:24AM -0700, Andrey Vagin wrote:
> > This test doesn't pass my 5-minutes test
> > 
> 
> This series is *not* intended to cover all possible scenarios,
> in particular what you're doing in your patch -- you do assign
> multiple same numberder fd into epoll. It won't work on this
> series, as expected. I not sure yet what is better way to
> cover such scenario. But it definitely gonna be implemented
> on top, so I encourage to apply the series since without it
> we can't even be sure what the files we're putting into the
> epoll.

In this case, criu has to return an error on dump...

And pls add a test with crfail flag.
Cyrill Gorcunov June 25, 2018, 6:05 p.m.
On Mon, Jun 25, 2018 at 10:57:48AM -0700, Andrey Vagin wrote:
> 
> In this case, criu has to return an error on dump...

Currently criu is simply proceed epolls as is which almost guarantee
the failure on restore if something gone wrong, mostly because we've
had to kernel support and had no other choise. Now here is some
improvement: we detect single duplications, and for sure I'm not
leaving it as is -- I'll implement detection of multiple numbers
duplications this week. But series is already big enough so I
prefer the scond patr to be implemented later.

That said feel free to defer this series, once second series
is ready you could fetch both.

> And pls add a test with crfail flag.