[CRIU] Re: [PATCH cr 3/3] zdtm: remove extra code from file_fown

Cyrill Gorcunov gorcunov at openvz.org
Wed May 30 09:43:12 EDT 2012


On Wed, May 30, 2012 at 03:22:36PM +0400, Pavel Emelyanov wrote:
> 
> What do we need the shmem for since we synchronize child and parent
> via pipes anyway?
> 
> And using assert_xxx doing { ret = xxx if (ret) goto } is obfuscating.
> Plz, opencode these parts.

Here is simplified one.

	Cyrill
-------------- next part --------------
>From e44002597f60e761bfd1217f58fef47c6db8cdfd Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Wed, 30 May 2012 17:41:39 +0400
Subject: [PATCH] zdtm: Simplify file_fown test case

 - sync via blocking pipes
 - drop socks, not needed

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 test/zdtm/live/static/file_fown.c |  127 +++++++++++--------------------------
 1 files changed, 38 insertions(+), 89 deletions(-)

diff --git a/test/zdtm/live/static/file_fown.c b/test/zdtm/live/static/file_fown.c
index e138a0a..040b239 100644
--- a/test/zdtm/live/static/file_fown.c
+++ b/test/zdtm/live/static/file_fown.c
@@ -20,19 +20,11 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-const char *test_doc	= "Check for signal delivery for file owners";
+const char *test_doc	= "Check for signal delivery on file owners";
 const char *test_author	= "Cyrill Gorcunov <gorcunov at openvz.org>";
 
 static int received_io;
 
-#define MAP(map, i)		(((int *)map)[i])
-#define MAP_SYNC(map)		MAP(map, 0)
-#define MAP_PID_PIPE0(map)	MAP(map, 1)
-#define MAP_PID_PIPE1(map)	MAP(map, 2)
-#define MAP_PID_SOK(map)	MAP(map, 3)
-
-#define SK_DATA "packet"
-
 static void signal_handler_io(int status)
 {
 	received_io++;
@@ -40,76 +32,51 @@ static void signal_handler_io(int status)
 
 int main(int argc, char ** argv)
 {
-	pid_t pid, ppid;
-	struct sigaction saio;
-	int status;
-	int pipes[2];
-	void *map;
-	uid_t ruid;
-	uid_t euid;
-	uid_t suid;
-
-	int ssk_pair[2];
-	char buf[64];
+	struct sigaction saio = { };
+	uid_t ruid, euid, suid;
+	int status, pipes[2];
+	pid_t pid;
 
 	test_init(argc, argv);
 
 	if (getresuid(&ruid, &euid, &suid)) {
-		fail("getresuid failed");
-		exit(1);
-	}
-
-	map = mmap(NULL, 1024, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-	if (map == MAP_FAILED) {
-		fail("Can't map");
+		fail("getresuid failed\n");
 		exit(1);
 	}
 
 	if (pipe(pipes)) {
-		err("Can't create pipes: %m\n");
+		err("Can't create pipe: %m\n");
 		exit(1);
 	}
 
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, ssk_pair) == -1) {
-		fail("socketpair\n");
+	saio.sa_handler	= (sig_t)signal_handler_io;
+	saio.sa_flags	= SA_RESTART;
+	if (sigaction(SIGIO, &saio, 0)) {
+		fail("sigaction failed\n");
 		exit(1);
 	}
 
-	memset(&saio, 0, sizeof(saio));
-	saio.sa_handler = (sig_t)signal_handler_io;
-	saio.sa_flags = SA_RESTART;
-	sigaction(SIGIO, &saio, 0);
-
 	if (setresuid(-1, 1, -1)) {
-		fail("setresuid failed");
+		fail("setresuid failed\n");
 		exit(1);
 	}
 
-	fcntl(pipes[0], F_SETOWN, getpid());
-	fcntl(pipes[1], F_SETOWN, getpid());
-
-	test_msg("main owner pipes[0]: %d\n", fcntl(pipes[0], F_GETOWN));
-
-	fcntl(pipes[0], F_SETSIG, SIGIO);
-	fcntl(pipes[1], F_SETSIG, SIGIO);
-
-	fcntl(pipes[0], F_SETFL, fcntl(pipes[0], F_GETFL) | O_NONBLOCK | O_ASYNC);
-	fcntl(pipes[1], F_SETFL, fcntl(pipes[1], F_GETFL) | O_NONBLOCK | O_ASYNC);
-
-	fcntl(ssk_pair[0], F_SETOWN, getpid());
-	fcntl(ssk_pair[0], F_SETSIG, SIGIO);
-	fcntl(ssk_pair[0], F_SETFL, fcntl(ssk_pair[0], F_GETFL) | O_NONBLOCK | O_ASYNC);
-	test_msg("main owner ssk_pair[0]: %d\n", fcntl(ssk_pair[0], F_GETOWN));
+	if (fcntl(pipes[0], F_SETOWN, getpid())					||
+	    fcntl(pipes[1], F_SETOWN, getpid())					||
+	    fcntl(pipes[0], F_SETSIG, SIGIO)					||
+	    fcntl(pipes[1], F_SETSIG, SIGIO)					||
+	    fcntl(pipes[0], F_SETFL, fcntl(pipes[0], F_GETFL) | O_ASYNC)	||
+	    fcntl(pipes[1], F_SETFL, fcntl(pipes[0], F_GETFL) | O_ASYNC)) {
+		fail("fcntl failed\n");
+		exit(1);
+	}
+	test_msg("main owner on pipes[0]: %d\n", fcntl(pipes[0], F_GETOWN));
 
 	if (setresuid(-1, euid, -1)) {
-		fail("setresuid failed");
+		fail("setresuid failed\n");
 		exit(1);
 	}
 
-	ppid = getpid();
-
-	MAP_SYNC(map) = 0;
-
 	pid = test_fork();
 	if (pid < 0) {
 		err("can't fork %m");
@@ -117,50 +84,32 @@ int main(int argc, char ** argv)
 	}
 
 	if (pid == 0) {
-		int v = 1;
-		MAP_SYNC(map) = 1;
-
-		while (MAP_SYNC(map) != 3)
-			sleep(1);
-
-		fcntl(pipes[1], F_SETOWN, getpid());
-
-		write(pipes[1], &v, sizeof(v));
-		read(pipes[0], &v, sizeof(v));
-
-		write(ssk_pair[0], SK_DATA, sizeof(SK_DATA));
-		read(ssk_pair[1], &buf, sizeof(buf));
-		if (strcmp(buf, SK_DATA)) {
-			fail("data corrupted\n");
+		pid_t me = fcntl(pipes[1], F_GETOWN);
+		test_msg("chile owner on pipes[1]: %d\n", fcntl(pipes[1], F_GETOWN));
+		if (write(pipes[1], &me, sizeof(me)) != sizeof(me)) {
+			fail("write failed\n");
 			exit(1);
 		}
-		test_msg("stream            : '%s'\n", buf);
-
-		MAP_PID_PIPE0(map) = fcntl(pipes[0], F_GETOWN);
-		MAP_PID_PIPE1(map) = fcntl(pipes[1], F_GETOWN);
-		MAP_PID_SOK(map) = fcntl(ssk_pair[0], F_GETOWN);
 
 		exit(0);
 	}
 
-	while (MAP_SYNC(map) != 1)
-		sleep(1);
-
 	test_daemon();
 	test_waitsig();
 
-	MAP_SYNC(map) = 3;
+	if (waitpid(pid, &status, P_ALL) == -1) {
+		fail("waitpid failed\n");
+		exit(1);
+	}
 
-	waitpid(pid, &status, P_ALL);
+	if (read(pipes[0], &pid, sizeof(pid)) != sizeof(pid)) {
+		fail("read failed\n");
+		exit(1);
+	}
 
-	if (received_io < 1		||
-	    MAP_PID_PIPE0(map) != ppid	||
-	    MAP_PID_PIPE1(map) != pid	||
-	    MAP_PID_SOK(map)   != ppid) {
-		fail("received_io = %d ppid: %d  MAP_PID_PIPE0(map): %d "
-		     "MAP_PID_PIPE1(map): %d MAP_PID_SOK(map): %d\n",
-		     received_io, ppid, MAP_PID_PIPE0(map),
-		     MAP_PID_PIPE1(map), MAP_PID_SOK(map));
+	if (received_io < 1 || pid != getpid()) {
+		fail("received_io = %d (> 0 expected) pid = %d (%d expected)\n",
+		     received_io, pid, getpid());
 		exit(1);
 	}
 
-- 
1.7.7.6



More information about the CRIU mailing list