locks: Don't dump locks in per-task manner (v3)

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.

One thing to note about flocks -- these belong to file entries,
not to tasks. Thus, when we meet one, we should check whether
the flock is really held by task's FD by trying to set yet
another one. In case of success -- lock really belongs to fd
we dump, in case it doesn't trylock should fail.

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@parallels.com>

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;
 

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;
 }
@@ -78,57 +81,26 @@ static void fill_flock_entry(FileLockEntry *fle, int fl_kind, int fl_ltype)
 	fle->type = fl_ltype;
 }
 
-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:%d %x %d %02x:%02x:%ld %lld %s\n",
-			fl->fl_id, fl->fl_kind, fl->fl_ltype,
-			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;
 		fill_flock_entry(&fle, fl->fl_kind, fl->fl_ltype);
-		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
@@ -145,6 +117,76 @@ 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;
+}
+
+int note_file_lock(struct pid *pid, int fd, int lfd, 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 (fl->fl_kind == FL_POSIX) {
+			/*
+			 * POSIX locks cannot belong to anyone
+			 * but creator.
+			 */
+			if (fl->fl_owner != pid->real)
+				continue;
+		} else /* fl->fl_kind == FL_FLOCK */ {
+			int ret;
+
+			/*
+			 * FLOCKs can be inherited across fork,
+			 * thus we can have any task as lock
+			 * owner. But the creator is preferred
+			 * anyway.
+			 */
+
+			if (fl->fl_owner != pid->real &&
+					fl->real_owner != -1)
+				continue;
+
+			pr_debug("Checking lock holder %d:%d\n", pid->real, fd);
+			ret = flock(lfd, LOCK_EX | LOCK_NB);
+			pr_debug("   `- %d/%d\n", ret, errno);
+			if (ret != 0) {
+				if (errno != EAGAIN) {
+					pr_err("Bogus lock test result %d\n", ret);
+					return -1;
+				}
+
+				continue;
+			} else if (fl->fl_ltype == F_RDLCK) {
+				pr_debug("   `- downgrading lock back\n");
+				flock(lfd, LOCK_SH);
+			}
+
+			/*
+			 * The ret == 0 means, that new lock doesn't conflict
+			 * with any others on the file. But since we do know, 
+			 * that there should be some other one (file is found
+			 * in /proc/locks), it means that the lock is already
+			 * on file pointed by fd.
+			 */
+		}
+
+		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;

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, lfd, &p))
+		return -1;
+
 	if (S_ISSOCK(p.stat.st_mode))
 		return dump_socket(&p, lfd, fdinfo);
 

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, int lfd, struct fd_parms *);
+extern int dump_file_locks(void);
+
 #define OPT_FILE_LOCKS	"file-locks"
 
 #endif /* __FILE_LOCK_H__ */