<div dir="ltr">Thank you Andrei.<br><div>Also this unchecked fcntl call already exists for another lock type so we'll send a patch to fix both of them.</div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-01-05 4:47 GMT+03:00 Andrei Vagin <span dir="ltr"><<a href="mailto:avagin@virtuozzo.com" target="_blank">avagin@virtuozzo.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Dec 20, 2016 at 01:24:38PM +0300, Pavel Begunkov wrote:<br>
> From: Begunkov Pavel <<a href="mailto:asml.silence@gmail.com">asml.silence@gmail.com</a>><br>
><br>
> OFD locks logic reuses existing locks c/r functionality.<br>
><br>
> Signed-off-by: Begunkov Pavel <<a href="mailto:asml.silence@gmail.com">asml.silence@gmail.com</a>><br>
> Signed-off-by: Eugene Batalov <<a href="mailto:eabatalov89@gmail.com">eabatalov89@gmail.com</a>><br>
> ---<br>
> criu/file-lock.c | 80 ++++++++++++++++++++++++++++++<wbr>++++++++++++++++--<br>
> criu/include/file-lock.h | 8 +++++<br>
> criu/proc_parse.c | 2 ++<br>
> 3 files changed, 87 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/criu/file-lock.c b/criu/file-lock.c<br>
> index 1ab1e58..fbb963b 100644<br>
> --- a/criu/file-lock.c<br>
> +++ b/criu/file-lock.c<br>
> @@ -204,6 +204,57 @@ static int lock_check_fd(int lfd, struct file_lock *fl)<br>
> return 1;<br>
> }<br>
><br>
> +static int lock_ofd_check_fd(int lfd, struct file_lock *fl)<br>
> +{<br>
> + int ret;<br>
> +<br>
> + struct flock lck = {<br>
> + .l_whence = SEEK_SET,<br>
> + .l_type = F_WRLCK,<br>
> + .l_start = fl->start<br>
> + };<br>
> + if (strcmp(fl->end, "EOF")) {<br>
> + unsigned long end;<br>
> +<br>
> + ret = sscanf(fl->end, "%lu", &end);<br>
> + if (ret <= 0) {<br>
> + pr_err("Invalid lock entry\n");<br>
> + return -1;<br>
> + }<br>
> + lck.l_len = end - fl->start + 1;<br>
> + } else {<br>
> + lck.l_len = 0;<br>
> + }<br>
> +<br>
> + ret = fcntl(lfd, F_OFD_SETLK, &lck);<br>
> + pr_debug(" `- %d/%d\n", ret, errno);<br>
> + if (ret != 0) {<br>
> + if (errno != EAGAIN) {<br>
> + pr_err("Bogus lock test result %d\n", ret);<br>
> + return -1;<br>
> + }<br>
> +<br>
> + return 0;<br>
> + } else {<br>
> + /*<br>
> + * The ret == 0 means, that new lock doesn't conflict<br>
> + * with any others on the file. But since we do know,<br>
> + * that there should be some other one (file is found<br>
> + * in /proc/locks), it means that the lock is already<br>
> + * on file pointed by fd.<br>
> + */<br>
> + pr_debug(" `- downgrading lock back\n");<br>
> + if (fl->fl_ltype & LOCK_WRITE)<br>
> + lck.l_type = F_WRLCK;<br>
> + else<br>
> + lck.l_type = F_RDLCK;<br>
> +<br>
> + fcntl(lfd, F_OFD_SETLK, &lck);<br>
<br>
<br>
</div></div>CID 173734 (#1 of 1): Unchecked return value from library (CHECKED_RETURN)<br>
7. check_return: Calling fcntl(lfd, 37, &lck) without checking return<br>
value. This library function may fail and return an error code.<br>
<div><div class="h5"><br>
> + }<br>
> +<br>
> + return 1;<br>
> +}<br>
> +<br>
> int note_file_lock(struct pid *pid, int fd, int lfd, struct fd_parms *p)<br>
> {<br>
> struct file_lock *fl;<br>
> @@ -232,11 +283,11 @@ int note_file_lock(struct pid *pid, int fd, int lfd, struct fd_parms *p)<br>
> */<br>
> if (fl->fl_owner != pid->real)<br>
> continue;<br>
> - } else /* fl->fl_kind == FL_FLOCK */ {<br>
> + } else /* fl->fl_kind == FL_FLOCK || fl->fl_kind == FL_OFD */ {<br>
> int ret;<br>
><br>
> /*<br>
> - * FLOCKs can be inherited across fork,<br>
> + * OFD locks & FLOCKs can be inherited across fork,<br>
> * thus we can have any task as lock<br>
> * owner. But the creator is preferred<br>
> * anyway.<br>
> @@ -247,7 +298,11 @@ int note_file_lock(struct pid *pid, int fd, int lfd, struct fd_parms *p)<br>
> continue;<br>
><br>
> pr_debug("Checking lock holder %d:%d\n", pid->real, fd);<br>
> - ret = lock_check_fd(lfd, fl);<br>
> + if (fl->fl_kind == FL_FLOCK)<br>
> + ret = lock_check_fd(lfd, fl);<br>
> + else<br>
> + ret = lock_ofd_check_fd(lfd, fl);<br>
> +<br>
> if (ret < 0)<br>
> return ret;<br>
> if (ret == 0)<br>
> @@ -312,6 +367,25 @@ static int restore_file_lock(<wbr>FileLockEntry *fle)<br>
> pr_err("Can not set posix lock!\n");<br>
> goto err;<br>
> }<br>
> + } else if (fle->flag & FL_OFD) {<br>
> + struct flock flk = {<br>
> + .l_whence = SEEK_SET,<br>
> + .l_start = fle->start,<br>
> + .l_len = fle->len,<br>
> + .l_pid = 0,<br>
> + .l_type = fle->type<br>
> + };<br>
> +<br>
> + pr_info("(ofd)flag: %d, type: %d, pid: %d, fd: %d, "<br>
> + "start: %8"PRIx64", len: %8"PRIx64"\n",<br>
> + fle->flag, fle->type, fle->pid, fle->fd,<br>
> + fle->start, fle->len);<br>
> +<br>
> + ret = fcntl(fle->fd, F_OFD_SETLK, &flk);<br>
> + if (ret < 0) {<br>
> + pr_err("Can not set ofd lock!\n");<br>
> + goto err;<br>
> + }<br>
> } else {<br>
> pr_err("Unknown file lock style!\n");<br>
> goto err;<br>
> diff --git a/criu/include/file-lock.h b/criu/include/file-lock.h<br>
> index 5c1bb5b..c3f2dab 100644<br>
> --- a/criu/include/file-lock.h<br>
> +++ b/criu/include/file-lock.h<br>
> @@ -9,6 +9,7 @@<br>
> #define FL_UNKNOWN -1<br>
> #define FL_POSIX 1<br>
> #define FL_FLOCK 2<br>
> +#define FL_OFD 4<br>
><br>
> /* for posix fcntl() and lockf() */<br>
> #ifndef F_RDLCK<br>
> @@ -17,6 +18,13 @@<br>
> #define F_UNLCK 2<br>
> #endif<br>
><br>
> +/* for OFD locks fcntl() */<br>
> +#ifndef F_OFD_GETLK<br>
> +#define F_OFD_GETLK 36<br>
> +#define F_OFD_SETLK 37<br>
> +#define F_OFD_SETLKW 38<br>
> +#endif<br>
> +<br>
> /* operations for bsd flock(), also used by the kernel implementation */<br>
> #define LOCK_SH 1 /* shared lock */<br>
> #define LOCK_EX 2 /* exclusive lock */<br>
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c<br>
> index 7c48b33..1f0bff8 100644<br>
> --- a/criu/proc_parse.c<br>
> +++ b/criu/proc_parse.c<br>
> @@ -1973,6 +1973,8 @@ static int parse_file_lock_buf(char *buf, struct file_lock *fl,<br>
> fl->fl_kind = FL_POSIX;<br>
> else if (!strcmp(fl_flag, "FLOCK"))<br>
> fl->fl_kind = FL_FLOCK;<br>
> + else if (!strcmp(fl_flag, "OFDLCK"))<br>
> + fl->fl_kind = FL_OFD;<br>
> else<br>
> fl->fl_kind = FL_UNKNOWN;<br>
><br>
> --<br>
> 2.10.0<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> CRIU mailing list<br>
> <a href="mailto:CRIU@openvz.org">CRIU@openvz.org</a><br>
> <a href="https://lists.openvz.org/mailman/listinfo/criu" rel="noreferrer" target="_blank">https://lists.openvz.org/<wbr>mailman/listinfo/criu</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Best regards,<br>Eugene Batalov.</div>
</div>