[CRIU] [RFC] files: Add union { addr; fd } for file entries

Cyrill Gorcunov gorcunov at openvz.org
Wed Feb 22 10:46:02 EST 2012


I believe distinguishing which particular name is
used does improve readability. Since in most cases
we refer to fdinfo_entry::addr in context of
being file descriptor number. So when I read
this code it makes me scratching the head, why
the hell we use 'addr' here.

I covered 'dump' case. If I idea will be aproved
I'll change restore case as well.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 cr-dump.c       |   55 ++++++++++++++++++++++++++++++++++---------------------
 include/image.h |    5 ++++-
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index 642755e..fcc99f5 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -86,7 +86,10 @@ err:
 }
 
 struct fd_parms {
-	unsigned long	fd_name;
+	union {
+		unsigned long	fd;
+		unsigned long	addr;
+	};
 	unsigned long	pos;
 	unsigned int	flags;
 	char		*id;
@@ -110,7 +113,7 @@ static int dump_one_reg_file(int type, struct fd_parms *p, int lfd,
 
 	big_buffer[len] = '\0';
 	pr_info("Dumping path for %lx fd via self %d [%s]\n",
-		p->fd_name, lfd, big_buffer);
+		p->fd, lfd, big_buffer);
 
 	if (do_close_lfd)
 		close(lfd);
@@ -119,14 +122,23 @@ static int dump_one_reg_file(int type, struct fd_parms *p, int lfd,
 	e.len	= len;
 	e.flags = p->flags;
 	e.pos	= p->pos;
-	e.addr	= p->fd_name;
+
+	switch (type) {
+	case FDINFO_FD:
+		e.fd = p->fd;
+		break;
+	case FDINFO_MAP:
+		e.addr = p->addr;
+		break;
+	}
+
 	if (p->id)
 		memcpy(e.id, p->id, FD_ID_SIZE);
 	else
 		memzero(e.id, FD_ID_SIZE);
 
-	pr_info("fdinfo: type: %2x len: %2x flags: %4x pos: %8lx addr: %16lx\n",
-		type, len, p->flags, p->pos, p->fd_name);
+	pr_info("fdinfo: type: %2x len: %2x flags: %4x pos: %8lx addr/fd: %16lx\n",
+		type, len, p->flags, p->pos, p->fd);
 
 	if (write_img(cr_fdset->fds[CR_FD_FDINFO], &e))
 		goto err;
@@ -144,7 +156,7 @@ static int dump_task_special_files(pid_t pid, struct cr_fdset *cr_fdset)
 	int fd, ret;
 
 	/* Dump /proc/pid/cwd */
-	params = (struct fd_parms){ .fd_name = FDINFO_CWD, };
+	params = (struct fd_parms){ .fd = FDINFO_CWD, };
 	fd = open_proc(pid, "cwd");
 	if (fd < 0)
 		return -1;
@@ -153,7 +165,7 @@ static int dump_task_special_files(pid_t pid, struct cr_fdset *cr_fdset)
 		return ret;
 
 	/* Dump /proc/pid/exe */
-	params = (struct fd_parms){ .fd_name = FDINFO_EXE, };
+	params = (struct fd_parms){ .fd = FDINFO_EXE, };
 	fd = open_proc(pid, "exe");
 	if (fd < 0)
 		return -1;
@@ -223,9 +235,9 @@ static int dump_one_pipe(struct fd_parms *p, unsigned int id, int lfd,
 	struct pipe_entry e;
 	int ret = -1;
 
-	pr_info("Dumping pipe %ld/%x flags %x\n", p->fd_name, id, p->flags);
+	pr_info("Dumping pipe %ld/%x flags %x\n", p->fd, id, p->flags);
 
-	e.fd		= p->fd_name;
+	e.fd		= p->fd;
 	e.pipeid	= id;
 	e.flags		= p->flags;
 
@@ -240,7 +252,7 @@ err:
 		pr_info("Dumped pipe: fd: %8x pipeid: %8x flags: %8x bytes: %8x\n",
 			e.fd, e.pipeid, e.flags, e.bytes);
 	else
-		pr_err("Dumping pipe %ld/%x flags %x\n", p->fd_name, id, p->flags);
+		pr_err("Dumping pipe %ld/%x flags %x\n", p->fd, id, p->flags);
 
 	return ret;
 }
@@ -253,16 +265,16 @@ static int dump_one_fd(pid_t pid, int pid_fd_dir, int lfd,
 	int err = -1;
 
 	if (lfd < 0) {
-		err = try_dump_socket(pid, p->fd_name, cr_fdset);
+		err = try_dump_socket(pid, p->fd, cr_fdset);
 		if (err != 1)
 			return err;
 
-		pr_perror("Failed to open %d/%ld", pid_fd_dir, p->fd_name);
+		pr_perror("Failed to open %d/%ld", pid_fd_dir, p->fd);
 		return -1;
 	}
 
 	if (fstat(lfd, &st_buf) < 0) {
-		pr_perror("Can't get stat on %ld", p->fd_name);
+		pr_perror("Can't get stat on %ld", p->fd);
 		goto out_close;
 	}
 
@@ -270,10 +282,10 @@ static int dump_one_fd(pid_t pid, int pid_fd_dir, int lfd,
 	    (major(st_buf.st_rdev) == TTY_MAJOR ||
 	     major(st_buf.st_rdev) == UNIX98_PTY_SLAVE_MAJOR)) {
 		/* skip only standard destriptors */
-		if (p->fd_name < 3) {
+		if (p->fd < 3) {
 			err = 0;
 			pr_info("... Skipping tty ... %d/%ld\n",
-				pid_fd_dir, p->fd_name);
+				pid_fd_dir, p->fd);
 			goto out_close;
 		}
 		goto err;
@@ -286,7 +298,7 @@ static int dump_one_fd(pid_t pid, int pid_fd_dir, int lfd,
 
 	if (S_ISFIFO(st_buf.st_mode)) {
 		if (fstatfs(lfd, &stfs_buf) < 0) {
-			pr_perror("Can't fstatfs on %ld", p->fd_name);
+			pr_perror("Can't fstatfs on %ld", p->fd);
 			return -1;
 		}
 
@@ -295,7 +307,7 @@ static int dump_one_fd(pid_t pid, int pid_fd_dir, int lfd,
 	}
 
 err:
-	pr_err("Can't dump file %ld of that type [%x]\n", p->fd_name, st_buf.st_mode);
+	pr_err("Can't dump file %ld of that type [%x]\n", p->fd, st_buf.st_mode);
 
 out_close:
 	close_safe(&lfd);
@@ -311,7 +323,7 @@ static int read_fd_params(pid_t pid, char *fd, struct fd_parms *p)
 	if (!file)
 		return -1;
 
-	p->fd_name = atoi(fd);
+	p->fd = atoi(fd);
 	ret = fscanf(file, "pos:\t%li\nflags:\t%o\nid:\t%s\n", &p->pos, &p->flags, p->id);
 	fclose(file);
 
@@ -406,10 +418,11 @@ static int dump_task_mappings(pid_t pid, struct list_head *vma_area_list, struct
 					goto err;
 			} else if (vma_entry_is(vma, VMA_FILE_PRIVATE) ||
 				   vma_entry_is(vma, VMA_FILE_SHARED)) {
+
 				struct fd_parms p = {
-					.fd_name = vma->start,
-					.pos = 0,
-					.id = NULL,
+					.addr	= vma->start,
+					.pos	= 0,
+					.id	= NULL,
 				};
 
 				if (vma->prot & PROT_WRITE &&
diff --git a/include/image.h b/include/image.h
index c9b7209..8bbcb60 100644
--- a/include/image.h
+++ b/include/image.h
@@ -46,7 +46,10 @@ struct fdinfo_entry {
 	u8	len;
 	u16	flags;
 	u32	pos;
-	u64	addr;
+	union {
+		u64	addr;
+		u64	fd;
+	};
 	u8	id[FD_ID_SIZE];
 	u8	name[0];
 } __packed;
-- 
1.7.7.6



More information about the CRIU mailing list