[CRIU] [PATCH 01/21] pipes: Split pipes data dumping into a separate procedure

Cyrill Gorcunov gorcunov at openvz.org
Wed Jun 6 18:21:41 EDT 2012


This makes code easier to read and will allow to
use dump_one_pipe_data for fifo data dumping.

Also I cleaned up a separate function as well
since it's was close to rewrite the whole function

 - Use explicit PAGE_MASK
 - Add comment on pipe_data_entry::off member
 - Use explicit PIPE_DEF_BUFFERS to show where
   this limitation number on aligment comes from

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 include/image.h |    6 ++-
 include/types.h |    4 ++
 pipes.c         |  120 ++++++++++++++++++++++++++++--------------------------
 3 files changed, 70 insertions(+), 60 deletions(-)

diff --git a/include/image.h b/include/image.h
index cb95330..4acbc86 100644
--- a/include/image.h
+++ b/include/image.h
@@ -157,16 +157,18 @@ struct pipe_entry {
 struct pipe_data_entry {
 	u32	pipe_id;
 	u32	bytes;
-	u32	off;
+	u32	off;		/* offset to data from current pos */
 	u8	data[0];
 } __packed;
 
 /*
  * splice() connect cache pages to pipe buffer, so
+ * thus if data are not aligned in file the summary
  * some part of pages may be loosed if data are not
  * aligned in a file.
  */
-#define PIPE_NONALIG_DATA (15 * PAGE_SIZE)
+#define PIPE_DEF_BUFFERS	16
+#define PIPE_MAX_NONALIG_SIZE	((PIPE_DEF_BUFFERS - 1) * PAGE_SIZE)
 
 #define USK_EXTERN	(1 << 0)
 
diff --git a/include/types.h b/include/types.h
index 499280e..29d1535 100644
--- a/include/types.h
+++ b/include/types.h
@@ -186,6 +186,10 @@ typedef struct {
 # define PAGE_SIZE	4096
 #endif
 
+#ifndef PAGE_MASK
+# define PAGE_MASK	(~(PAGE_SIZE - 1))
+#endif
+
 /* For sys_kcmp */
 enum kcmp_type {
 	KCMP_FILE,
diff --git a/pipes.c b/pipes.c
index 85f4bd7..547fdf8 100644
--- a/pipes.c
+++ b/pipes.c
@@ -304,36 +304,30 @@ static int open_pipe(struct file_desc *d)
 static u32 *pipes_with_data;	/* pipes for which data already dumped */
 static int nr_pipes = 0;
 
-static int dump_one_pipe(int lfd, u32 id, const struct fd_parms *p)
+static int dump_one_pipe_data(int lfd, u32 id, const struct fd_parms *p)
 {
-	struct pipe_entry pe;
-	int fd_pipes;
-	int steal_pipe[2];
-	int pipe_size;
-	int has_bytes = 0;
+	int steal_pipe[2] = { -1, -1 };
+	int pipe_size, i, bytes;
 	int ret = -1;
-	int i = 0;
-
-	pr_info("Dumping pipe %d with id %#x pipe_id %#x\n", lfd, id, p->id);
-
-	fd_pipes = fdset_fd(glob_fdset, CR_FD_PIPES);
 
 	if (p->flags & O_WRONLY)
-		goto dump;
+		return 0;
 
-	for (i = 0; i < nr_pipes; i++)
+	/* Maybe we've dumped it already */
+	for (i = 0; i < nr_pipes; i++) {
 		if (pipes_with_data[i] == p->id)
-			goto dump; /* data was dumped already */
+			return 0;
+	}
 
-	nr_pipes++;
-	if (nr_pipes > PIPES_SIZE) {
+	pr_info("Dumping data from pipe %#x fd %d\n", id, lfd);
+
+	if (PIPES_SIZE < nr_pipes + 1) {
 		pr_err("OOM storing pipe\n");
 		return -1;
 	}
 
-	pr_info("Dumping data from pipe %#x fd %d\n", id, lfd);
-
-	pipes_with_data[nr_pipes - 1] = p->id;
+	pipes_with_data[nr_pipes] = p->id;
+	nr_pipes++;
 
 	pipe_size = fcntl(lfd, F_GETPIPE_SZ);
 	if (pipe_size < 0) {
@@ -346,69 +340,79 @@ static int dump_one_pipe(int lfd, u32 id, const struct fd_parms *p)
 		goto err;
 	}
 
-	has_bytes = tee(lfd, steal_pipe[1], pipe_size, SPLICE_F_NONBLOCK);
-	if (has_bytes < 0) {
-		if (errno != EAGAIN) {
-			pr_perror("Can't pick pipe data");
-			goto err_close;
-		} else
-			has_bytes = 0;
-	}
-dump:
-	pe.id = id;
-	pe.pipe_id = p->id;
-	pe.flags = p->flags;
-	pe.fown = p->fown;
-
-	if (write_img(fd_pipes, &pe))
-		goto err_close;
-
-	if (has_bytes) {
-		off_t off;
+	bytes = tee(lfd, steal_pipe[1], pipe_size, SPLICE_F_NONBLOCK);
+	if (bytes > 0) {
+		int fd_pipes_data = fdset_fd(glob_fdset, CR_FD_PIPES_DATA);
 		struct pipe_data_entry pde;
+		int wrote;
 
-		fd_pipes = fdset_fd(glob_fdset, CR_FD_PIPES_DATA);
+		pde.pipe_id	= p->id;
+		pde.bytes	= bytes;
+		pde.off		= 0;
 
-		pde.pipe_id = p->id;
-		pde.bytes = has_bytes;
-		pde.off = 0;
+		if (bytes > PIPE_MAX_NONALIG_SIZE) {
+			off_t off;
 
-		if (has_bytes > PIPE_NONALIG_DATA) {
-			off = lseek(fd_pipes, 0, SEEK_CUR);
+			off  = lseek(fd_pipes_data, 0, SEEK_CUR);
 			off += sizeof(pde);
-			off &= PAGE_SIZE -1;
+			off &= ~PAGE_MASK;
+
 			if (off)
 				pde.off = PAGE_SIZE - off;
-			pr_info("off 0x%lx %#x\n", off, pde.off);
+
+			pr_info("\toff %#lx %#x bytes %#x\n", off, pde.off, bytes);
 		}
 
-		if (write_img(fd_pipes, &pde))
+		if (write_img(fd_pipes_data, &pde))
 			goto err_close;
 
-		if (pde.off) {
-			off = lseek(fd_pipes, pde.off, SEEK_CUR);
-			pr_info("off 0x%lx\n", off);
-		}
+		/* Don't forget to advance position if a hole needed */
+		if (pde.off)
+			lseek(fd_pipes_data, pde.off, SEEK_CUR);
 
-		ret = splice(steal_pipe[0], NULL, fd_pipes,
-			     NULL, has_bytes, 0);
-		if (ret < 0) {
+		wrote = splice(steal_pipe[0], NULL, fd_pipes_data, NULL, bytes, 0);
+		if (wrote < 0) {
 			pr_perror("Can't push pipe data");
 			goto err_close;
+		} else if (wrote != bytes) {
+			pr_err("%#x: Wanted to write %d bytes, but wrote %d\n", id, bytes, wrote);
+			goto err_close;
+		}
+	} else if (bytes < 0) {
+		if (errno != EAGAIN) {
+			pr_perror("Can't pick pipe data");
+			goto err_close;
 		}
 	}
 
 	ret = 0;
 
 err_close:
-	if (has_bytes) {
-		close(steal_pipe[0]);
-		close(steal_pipe[1]);
-	}
+	close(steal_pipe[0]);
+	close(steal_pipe[1]);
 err:
 	return ret;
 }
 
+static int dump_one_pipe(int lfd, u32 id, const struct fd_parms *p)
+{
+	struct pipe_entry pe;
+	int fd_pipes;
+
+	pr_info("Dumping pipe %d with id %#x pipe_id %#x\n", lfd, id, p->id);
+
+	fd_pipes = fdset_fd(glob_fdset, CR_FD_PIPES);
+
+	pe.id = id;
+	pe.pipe_id = p->id;
+	pe.flags = p->flags;
+	pe.fown = p->fown;
+
+	if (write_img(fd_pipes, &pe))
+		return -1;
+	return dump_one_pipe_data(lfd, id, p);
+}
+
 static const struct fdtype_ops pipe_ops = {
 	.type		= FDINFO_PIPE,
 	.make_gen_id	= make_gen_id,
-- 
1.7.7.6



More information about the CRIU mailing list