[CRIU] [PATCH v4 2/4] locks: Add c/r of breaking leases (kernel>=v4.1)
Pavel Begunkov
asml.silence at gmail.com
Thu Sep 21 07:23:13 MSK 2017
On 11/09/17 21:17, Andrei Vagin wrote:
> On Sat, Sep 09, 2017 at 07:51:38PM +0300, Pavel Begunkov wrote:
>> Leases can be taken out only on regular files. In order to restore
>> breaking lease it break restored lease by opening a file with which
>> lease is associated.
>>
>> Also, the patch fix type of broken leases to 'target lease type',
>> because procfs always returns 'READ' in this case.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence at gmail.com>
>> ---
>> criu/file-lock.c | 163 +++++++++++++++++++++++++++++++++++++++++++++--
>> criu/files.c | 3 +
>> criu/include/file-lock.h | 4 ++
>> criu/proc_parse.c | 7 +-
>> 4 files changed, 170 insertions(+), 7 deletions(-)
>>
>> diff --git a/criu/file-lock.c b/criu/file-lock.c
>> index 816dbca5b..7fc1ef5c7 100644
>> --- a/criu/file-lock.c
>> +++ b/criu/file-lock.c
>> @@ -1,4 +1,5 @@
>> #include <stdlib.h>
>> +#include <signal.h>
>> #include <unistd.h>
>> #include <sys/file.h>
>> #include <fcntl.h>
>> @@ -19,6 +20,9 @@
>> #include "proc_parse.h"
>> #include "servicefd.h"
>> #include "file-lock.h"
>> +#include "pstree.h"
>> +#include "files-reg.h"
>> +
>>
>> struct file_lock_rst {
>> FileLockEntry *fle;
>> @@ -333,6 +337,93 @@ int note_file_lock(struct pid *pid, int fd, int lfd, struct fd_parms *p)
>> return 0;
>> }
>>
>> +int correct_file_leases_type(struct pid *pid, int fd, int lfd)
>> +{
>> + struct file_lock *fl;
>> + int target_type;
>> +
>> + list_for_each_entry(fl, &file_lock_list, list) {
>> + /* owners_fd should be set before usage */
>> + if (fl->fl_owner != pid->real || fl->owners_fd != fd)
>> + continue;
>> +
>> + if (fl->fl_kind == FL_LEASE &&
>> + (fl->fl_ltype & LEASE_BREAKING)) {
>> + /*
>> + * Set lease type to actual 'target lease type'
>> + * instead of 'READ' returned by procfs.
>> + */
>> + target_type = fcntl(lfd, F_GETLEASE);
>> + if (target_type < 0) {
>> + perror("Can't get lease type\n");
>> + return -1;
>> + }
>> + fl->fl_ltype &= ~O_ACCMODE;
>> + fl->fl_ltype |= target_type;
>> + break;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +static int open_break_cb(int ns_root_fd, struct reg_file_info *rfi, void *arg)
>> +{
>> + int signum_fcntl, signum;
>> + int ret = 0;
>> + int fd, flags = *(int *)arg | O_NONBLOCK;
>> + sigset_t blockmask, oldmask;
>> +
>> + signum_fcntl = fcntl(ns_root_fd, F_GETSIG);
>> + signum = signum_fcntl ? signum_fcntl : SIGIO;
>> + if (signum_fcntl < 0) {
>> + pr_perror("Can't get file i/o signum\n");
>> + return -1;
>> + }
>> + if (sigemptyset(&blockmask) ||
>> + sigaddset(&blockmask, signum) ||
>> + sigprocmask(SIG_BLOCK, &blockmask, &oldmask)) {
>
> Why do we need to block a signal? I don't understand this game.
>
1) We should block it to don't mess up with SIGCHLD. I/O signal can be
remapped to any signum with fcntl(fd, F_SETSIG), e.g. to SIGCHLD. This
one is only non-blocked signal in CRIU and it's used for internal
purposes. So, I think it's better to temporarily block it.
2) All signals during restore are blocked (except SIGCHLD), and seems
CRIU don't handles blocked signals (e.g. signalfd). If I haven't missed
anything, it's pretty safe to just block it.
>> + pr_perror("Can't block file i/o signal\n");
>> + return -1;
>> + }
>> +
>> + fd = openat(ns_root_fd, rfi->path, flags);
>> + if (fd >= 0) {
>> + pr_err("Conflicting lease wasn't found\n");
>> + close(fd);
>> + ret = -1;
>> + } else if (errno != EWOULDBLOCK) {
>> + pr_perror("Can't break lease\n");
>> + ret = -1;
>> + }
>> +
>> + if (sigprocmask(SIG_SETMASK, &oldmask, NULL)) {
>> + pr_perror("Can't restore sigmask\n");
>> + ret = -1;
>> + }
>> + return ret;
>> +}
>> +
>> +static int break_lease(int lease_type, struct file_desc *desc)
>> +{
>> + int target_type = lease_type & (~LEASE_BREAKING);
>> + int break_flags;
>> +
>> + /*
>> + * Flags for open call chosen in a way to even
>> + * 'target lease type' returned by fcntl(F_GETLEASE)
>> + * and lease type from the image.
>> + */
>> + if (target_type == F_UNLCK) {
>> + break_flags = O_WRONLY;
>> + } else if (target_type == F_RDLCK) {
>> + break_flags = O_RDONLY;
>> + } else {
>> + pr_err("Incorrect target lease type\n");
>> + return -1;
>> + }
>> + return open_path(desc, open_break_cb, (void *)&break_flags);
>> +}
>> +
>> static int set_file_lease(int fd, int type)
>> {
>> int old_fsuid, ret;
>> @@ -357,6 +448,72 @@ static int set_file_lease(int fd, int type)
>> return ret;
>> }
>>
>> +static int restore_lease_prebreaking_state(int fd, int fd_type)
>> +{
>> + int access_flags = fd_type & O_ACCMODE;
>> + int lease_type = (access_flags == O_RDONLY) ? F_RDLCK : F_WRLCK;
>> +
>> + if (set_file_lease(fd, lease_type) < 0) {
>> + pr_perror("Can't set lease (fd %i)\n", fd);
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> +static struct fdinfo_list_entry *find_fd_unordered(struct pstree_item *task,
>> + int fd)
>> +{
>> + struct list_head *head = &rsti(task)->fds;
>> + struct fdinfo_list_entry *fle;
>> +
>> + list_for_each_entry_reverse(fle, head, ps_list) {
>> + if (fle->fe->fd == fd)
>> + return fle;
>> + }
>> + return NULL;
>> +}
>> +
>> +static int restore_breaking_file_lease(FileLockEntry *fle)
>> +{
>> + struct fdinfo_list_entry *fdle;
>> +
>> + fdle = find_fd_unordered(current, fle->fd);
>> + if (fdle == NULL) {
>> + pr_err("Can't get file description\n");
>> + return -1;
>> + }
>> +
>> + if (restore_lease_prebreaking_state(fle->fd, fdle->desc->ops->type))
>> + return -1;
>> +
>> + /*
>> + * It could be broken by 2 types of open call:
>> + * 1. non-blocking: It failed because of the lease.
>> + * 2. blocking: It had been blocked at the moment
>> + * of dumping, otherwise lease wouldn't be broken.
>> + * Thus, it was canceled by CRIU.
>> + *
>> + * There are no files or leases in image, which will
>> + * conflict with each other. Therefore we should explicitly
>> + * break leases. Restoring can be done in any order.
>> + */
>> + return break_lease(fle->type, fdle->desc);
>> +}
>> +
>> +static int restore_file_lease(FileLockEntry *fle)
>> +{
>> + int ret;
>> +
>> + if (fle->type & LEASE_BREAKING) {
>> + return restore_breaking_file_lease(fle);
>> + } else {
>> + ret = set_file_lease(fle->fd, fle->type);
>> + if (ret < 0)
>> + pr_perror("Can't restore non breaking lease");
>> + return ret;
>> + }
>> +}
>> +
>> static int restore_file_lock(FileLockEntry *fle)
>> {
>> int ret = -1;
>> @@ -424,11 +581,9 @@ static int restore_file_lock(FileLockEntry *fle)
>> goto err;
>> }
>> } else if (fle->flag & FL_LEASE) {
>> - ret = set_file_lease(fle->fd, fle->type);
>> - if (ret < 0) {
>> - pr_perror("Can't set lease!\n");
>> + ret = restore_file_lease(fle);
>> + if (ret < 0)
>> goto err;
>> - }
>> } else {
>> pr_err("Unknown file lock style!\n");
>> goto err;
>> diff --git a/criu/files.c b/criu/files.c
>> index 01cd4c0e9..c103cbc1d 100644
>> --- a/criu/files.c
>> +++ b/criu/files.c
>> @@ -458,6 +458,9 @@ static int dump_one_file(struct pid *pid, int fd, int lfd, struct fd_opts *opts,
>> if (note_file_lock(pid, fd, lfd, &p))
>> return -1;
>>
>> + if (correct_file_leases_type(pid, fd, lfd))
>> + return -1;
>> +
>> p.fd_ctl = ctl; /* Some dump_opts require this to talk to parasite */
>>
>> if (S_ISSOCK(p.stat.st_mode))
>> diff --git a/criu/include/file-lock.h b/criu/include/file-lock.h
>> index f70739adb..7b19524d7 100644
>> --- a/criu/include/file-lock.h
>> +++ b/criu/include/file-lock.h
>> @@ -38,6 +38,9 @@
>> #define LOCK_WRITE 128 /* which allows concurrent write operations */
>> #define LOCK_RW 192 /* which allows concurrent read & write ops */
>>
>> +/* for leases */
>> +#define LEASE_BREAKING 4
>> +
>> struct file_lock {
>> long long fl_id;
>> int fl_kind;
>> @@ -65,6 +68,7 @@ extern struct collect_image_info file_locks_cinfo;
>>
>> struct pid;
>> struct fd_parms;
>> +extern int correct_file_leases_type(struct pid *, int fd, int lfd);
>> extern int note_file_lock(struct pid *, int fd, int lfd, struct fd_parms *);
>> extern int dump_file_locks(void);
>>
>> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
>> index d3893272c..89be4ceb5 100644
>> --- a/criu/proc_parse.c
>> +++ b/criu/proc_parse.c
>> @@ -1996,6 +1996,10 @@ static int parse_file_lock_buf(char *buf, struct file_lock *fl,
>> else
>> fl->fl_kind = FL_UNKNOWN;
>>
>> + if (fl->fl_kind == FL_LEASE && !strcmp(fl_type, "BREAKING")) {
>> + fl->fl_ltype |= LEASE_BREAKING;
>> + }
>> +
>> if (!strcmp(fl_type, "MSNFS")) {
>> fl->fl_ltype |= LOCK_MAND;
>>
>> @@ -2009,9 +2013,6 @@ static int parse_file_lock_buf(char *buf, struct file_lock *fl,
>> pr_err("Unknown lock option!\n");
>> return -1;
>> }
>> - } else if (fl->fl_kind == FL_LEASE && !strcmp(fl_type, "BREAKING")) {
>> - pr_err("Breaking leases are not supported (%d): %s\n",
>> - num, buf);
>> } else {
>> if (!strcmp(fl_option, "UNLCK")) {
>> fl->fl_ltype |= F_UNLCK;
>> --
>> 2.11.1
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
--
Yours sincerely,
Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openvz.org/pipermail/criu/attachments/20170921/d01574b7/attachment-0001.sig>
More information about the CRIU
mailing list