[CRIU] [PATCH 3/6] locks: Don't dump locks in per-task manner (v2)
Pavel Emelyanov
xemul at parallels.com
Tue Aug 26 10:30:40 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.
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 at parallels.com>
---
cr-dump.c | 12 ++----
file-lock.c | 121 ++++++++++++++++++++++++++++++++++------------------
files.c | 4 ++
include/file-lock.h | 12 ++++--
4 files changed, 95 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 665600b..6e445d6 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;
}
@@ -108,62 +111,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:%d %s %s %d %02x:%02x:%ld %lld %s\n",
- fl->fl_id, fl->fl_kind, 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_kind, 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
@@ -180,6 +152,73 @@ 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;
+ }
+
+ /*
+ * 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;
diff --git a/files.c b/files.c
index 68c06c7..39d9914 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, lfd, &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 b64720f..009530d 100644
--- a/include/file-lock.h
+++ b/include/file-lock.h
@@ -42,20 +42,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__ */
--
1.8.4.2
More information about the CRIU
mailing list