[CRIU] [PATCH 2/4] locks: Don't dump locks in per-task manner
Pavel Emelyanov
xemul at parallels.com
Tue Aug 26 10:28:39 PDT 2014
On 08/26/2014 04:05 PM, Pavel Emelyanov wrote:
> 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.
Em, one problem with it. PID of owner could be re-used or
the owner itself can re-open the file, so the proposed check
in better_owner doesn't work.
Update it coming.
> 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__ */
>
More information about the CRIU
mailing list