[CRIU] [PATCH 2/4] locks: Don't dump locks in per-task manner

Pavel Emelyanov xemul at parallels.com
Tue Aug 26 05:05:14 PDT 2014


We have a problem with file locks (bug #2512) -- the /proc/locks
file shows the ID of lock creator, not the owner. Thus, if the
creator died, but holder is still alive, criu fails to dump the
lock held by latter task.

The proposal is to find who _might_ hold the lock by checking
for dev:inode pairs on lock vs file descriptors being dumped.
If the creator of the lock is still alive, then he will take
the priority.

At the very end -- walk the list of locks and dump them all at
once, which is possible by merge of per-task file-locks images
into one global one.

Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
---
 cr-dump.c           | 12 ++-----
 file-lock.c         | 91 +++++++++++++++++++++++++++++------------------------
 files.c             |  4 +++
 include/file-lock.h | 12 ++++---
 4 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index cec579e..8abb142 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -1605,15 +1605,6 @@ static int dump_one_task(struct pstree_item *item)
 		}
 	}
 
-	if (opts.handle_file_locks) {
-		ret = dump_task_file_locks(parasite_ctl, cr_fdset, dfds);
-		if (ret) {
-			pr_err("Dump file locks (pid: %d) failed with %d\n",
-				pid, ret);
-			goto err_cure;
-		}
-	}
-
 	ret = parasite_dump_pages_seized(parasite_ctl, &vmas, NULL);
 	if (ret)
 		goto err_cure;
@@ -1856,6 +1847,9 @@ int cr_dump_tasks(pid_t pid)
 	if (dump_mnt_namespaces() < 0)
 		goto err;
 
+	if (dump_file_locks())
+		goto err;
+
 	if (dump_verify_tty_sids())
 		goto err;
 
diff --git a/file-lock.c b/file-lock.c
index b90c774..f6b637b 100644
--- a/file-lock.c
+++ b/file-lock.c
@@ -8,6 +8,7 @@
 
 #include "cr_options.h"
 #include "fdset.h"
+#include "files.h"
 #include "image.h"
 #include "servicefd.h"
 #include "file-lock.h"
@@ -48,6 +49,8 @@ struct file_lock *alloc_file_lock(void)
 		return NULL;
 
 	INIT_LIST_HEAD(&flock->list);
+	flock->real_owner = -1;
+	flock->owners_fd = -1;
 
 	return flock;
 }
@@ -115,62 +118,31 @@ err:
 	return -1;
 }
 
-static int get_fd_by_ino(unsigned long i_no, struct parasite_drain_fd *dfds,
-			pid_t pid)
-{
-	int  i;
-	char buf[PATH_MAX];
-	struct stat fd_stat;
-
-	for (i = 0; i < dfds->nr_fds; i++) {
-		snprintf(buf, sizeof(buf), "/proc/%d/fd/%d", pid,
-			dfds->fds[i]);
-
-		if (stat(buf, &fd_stat) == -1) {
-			pr_msg("Could not get %s stat!\n", buf);
-			continue;
-		}
-
-		if (fd_stat.st_ino == i_no)
-			return dfds->fds[i];
-	}
-
-	return -1;
-}
-
-int dump_task_file_locks(struct parasite_ctl *ctl,
-			struct cr_fdset *fdset,	struct parasite_drain_fd *dfds)
+int dump_file_locks(void)
 {
 	FileLockEntry	 fle;
 	struct file_lock *fl;
-
-	pid_t	pid = ctl->pid.real;
 	int	ret = 0;
 
+	pr_info("Dumping file-locks\n");
+
 	list_for_each_entry(fl, &file_lock_list, list) {
-		if (fl->fl_owner != pid)
-			continue;
-		pr_info("lockinfo: %lld:%s %s %s %d %02x:%02x:%ld %lld %s\n",
-			fl->fl_id, fl->fl_flag, fl->fl_type, fl->fl_option,
-			fl->fl_owner, fl->maj, fl->min, fl->i_no,
-			fl->start, fl->end);
+		if (fl->real_owner == -1) {
+			pr_err("Unresolved lock found pid %d ino %ld\n",
+					fl->fl_owner, fl->i_no);
+			return -1;
+		}
 
 		file_lock_entry__init(&fle);
-		fle.pid = ctl->pid.virt;
+		fle.pid = fl->real_owner;
+		fle.fd = fl->owners_fd;
 
 		ret = fill_flock_entry(&fle, fl->fl_flag, fl->fl_type,
 				fl->fl_option);
 		if (ret)
 			goto err;
 
-		fle.fd = get_fd_by_ino(fl->i_no, dfds, pid);
-		if (fle.fd < 0) {
-			ret = -1;
-			goto err;
-		}
-
 		fle.start = fl->start;
-
 		if (!strncmp(fl->end, "EOF", 3))
 			fle.len = 0;
 		else
@@ -187,6 +159,43 @@ err:
 	return ret;
 }
 
+static inline bool lock_file_match(struct file_lock *fl, struct fd_parms *p)
+{
+	return fl->i_no == p->stat.st_ino &&
+		makedev(fl->maj, fl->min) == p->stat.st_dev;
+}
+
+static inline bool lock_better_owner(struct file_lock *fl, struct pid *pid)
+{
+	if (fl->real_owner == -1) /* no owner yet */
+		return true;
+	if (fl->fl_owner == pid->real) /* real owner found */
+		return true;
+
+	return false;
+}
+
+int note_file_lock(struct pid *pid, int fd, struct fd_parms *p)
+{
+	struct file_lock *fl;
+
+	list_for_each_entry(fl, &file_lock_list, list) {
+		if (!lock_file_match(fl, p))
+			continue;
+
+		if (lock_better_owner(fl, pid)) {
+			fl->real_owner = pid->virt;
+			fl->owners_fd = fd;
+
+			pr_info("Found lock entry %d.%d %d vs %d\n",
+					pid->real, pid->virt, fd,
+					fl->fl_owner);
+		}
+	}
+
+	return 0;
+}
+
 static int restore_file_lock(FileLockEntry *fle)
 {
 	int ret = -1;
diff --git a/files.c b/files.c
index 68c06c7..7818e57 100644
--- a/files.c
+++ b/files.c
@@ -16,6 +16,7 @@
 #include "files.h"
 #include "file-ids.h"
 #include "files-reg.h"
+#include "file-lock.h"
 #include "image.h"
 #include "list.h"
 #include "util.h"
@@ -304,6 +305,9 @@ static int dump_one_file(struct parasite_ctl *ctl, int fd, int lfd, struct fd_op
 		return -1;
 	}
 
+	if (note_file_lock(&ctl->pid, fd, &p))
+		return -1;
+
 	if (S_ISSOCK(p.stat.st_mode))
 		return dump_socket(&p, lfd, fdinfo);
 
diff --git a/include/file-lock.h b/include/file-lock.h
index 536ce94..bc2f3a4 100644
--- a/include/file-lock.h
+++ b/include/file-lock.h
@@ -41,20 +41,24 @@ struct file_lock {
 	char		end[32];
 
 	struct list_head list;		/* list of all file locks */
+
+	int		real_owner;
+	int		owners_fd;
 };
 
 extern struct list_head file_lock_list;
 
 extern struct file_lock *alloc_file_lock(void);
 extern void free_file_locks(void);
-struct parasite_ctl;
-struct parasite_drain_fd;
-extern int dump_task_file_locks(struct parasite_ctl *ctl,
-			struct cr_fdset *fdset,	struct parasite_drain_fd *dfds);
 
 extern int prepare_file_locks(int pid);
 extern struct collect_image_info file_locks_cinfo;
 
+struct pid;
+struct fd_parms;
+extern int note_file_lock(struct pid *, int fd, struct fd_parms *);
+extern int dump_file_locks(void);
+
 #define OPT_FILE_LOCKS	"file-locks"
 
 #endif /* __FILE_LOCK_H__ */
-- 
1.8.4.2




More information about the CRIU mailing list