[CRIU] [PATCH] test/zdtm: use flock64 instead of flock

Kir Kolyshkin kir at virtuozzo.com
Wed Jan 25 12:44:27 PST 2017


On 01/19/2017 04:11 AM, Dmitry Safonov wrote:
> For 32-bit fcntl() Glibc function calls sys_fcntl64(), which
> needs struct flock64, otherwise the kernel gets a wrong struct.
> For 64-bit, it's all the same.
>
> Also unset errno before fcntl() and check return value of the call.

Can you explain why errno = 0 is needed in your patch?
 From what I see, errno is only used if fcntl() returned -1,
in which case errno must be set by glibc (well, at least
in theory, according to fcntl(2) man page).

I hope this is not a workaround for a bug in glibc
(in case it is, it's better to document it in the source code).

>
> Cc: Qiang Huang <h.huangqiang at huawei.com>
> Cc: Begunkov Pavel <asml.silence at gmail.com>
> Cc: Pavel Emelyanov <xemul at virtuozzo.com>
> Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
> ---
>   test/zdtm/static/file_locks00.c   | 34 ++++++++++++++++++++++++----------
>   test/zdtm/static/file_locks06.c   |  4 ++--
>   test/zdtm/static/file_locks07.c   |  4 ++--
>   test/zdtm/static/file_locks08.c   |  4 ++--
>   test/zdtm/static/ofd_file_locks.h | 12 ++++++------
>   5 files changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/test/zdtm/static/file_locks00.c b/test/zdtm/static/file_locks00.c
> index f701e00c0cb5..82888709e46a 100644
> --- a/test/zdtm/static/file_locks00.c
> +++ b/test/zdtm/static/file_locks00.c
> @@ -24,24 +24,25 @@ char file1[PATH_MAX];
>   static int lock_reg(int fd, int cmd, int type, int whence,
>   		off_t offset, off_t len)
>   {
> -	struct flock lock;
> +	struct flock64 lock;
>   
>   	lock.l_type   = type;     /* F_RDLCK, F_WRLCK, F_UNLCK */
>   	lock.l_whence = whence;   /* SEEK_SET, SEEK_CUR, SEEK_END */
>   	lock.l_start  = offset;   /* byte offset, relative to l_whence */
>   	lock.l_len    = len;      /* #bytes (0 means to EOF) */
>   
> +	errno = 0;
>   	return fcntl(fd, cmd, &lock);
>   }
>   
>   #define set_read_lock(fd, whence, offset, len) \
> -	lock_reg(fd, F_SETLK, F_RDLCK, whence, offset, len)
> +	lock_reg(fd, F_SETLK64, F_RDLCK, whence, offset, len)
>   #define set_write_lock(fd, whence, offset, len) \
> -	lock_reg(fd, F_SETLK, F_WRLCK, whence, offset, len)
> +	lock_reg(fd, F_SETLK64, F_WRLCK, whence, offset, len)
>   
>   static int check_read_lock(int fd, int whence, off_t offset, off_t len)
>   {
> -	struct flock lock;
> +	struct flock64 lock;
>   	int ret;
>   
>   	lock.l_type   = F_RDLCK;  /* F_RDLCK, F_WRLCK, F_UNLCK */
> @@ -50,7 +51,8 @@ static int check_read_lock(int fd, int whence, off_t offset, off_t len)
>   	lock.l_len    = len;      /* #bytes (0 means to EOF) */
>   	lock.l_pid    = -1;
>   
> -	ret = fcntl(fd, F_GETLK, &lock);
> +	errno = 0;
> +	ret = fcntl(fd, F_GETLK64, &lock);
>   	if (ret == -1) {
>   		pr_perror("F_GETLK failed.");
>   		return -1;
> @@ -67,7 +69,7 @@ static int check_read_lock(int fd, int whence, off_t offset, off_t len)
>   
>   static int check_write_lock(int fd, int whence, off_t offset, off_t len)
>   {
> -	struct flock lock;
> +	struct flock64 lock;
>   
>   	int ret;
>   	pid_t ppid = getppid();
> @@ -78,7 +80,8 @@ static int check_write_lock(int fd, int whence, off_t offset, off_t len)
>   	lock.l_len    = len;      /* #bytes (0 means to EOF) */
>   	lock.l_pid    = -1;
>   
> -	ret = fcntl(fd, F_GETLK, &lock);
> +	errno = 0;
> +	ret = fcntl(fd, F_GETLK64, &lock);
>   	if (ret == -1) {
>   		pr_perror("F_GETLK failed.");
>   		return -1;
> @@ -129,7 +132,7 @@ static int check_file_locks()
>   
>   int main(int argc, char **argv)
>   {
> -	int fd_0, fd_1;
> +	int fd_0, fd_1, ret;
>   	pid_t pid;
>   
>   	test_init(argc, argv);
> @@ -168,8 +171,19 @@ int main(int argc, char **argv)
>   		exit(0);
>   	}
>   
> -	set_read_lock(fd_0, SEEK_SET, 0, 0);
> -	set_write_lock(fd_1, SEEK_SET, 0, 0);
> +	ret = set_read_lock(fd_0, SEEK_SET, 0, 0);
> +	if (ret == -1) {
> +		pr_perror("Failed to set read lock");
> +		kill(pid, SIGTERM);
> +		return -1;
> +	}
> +
> +	ret = set_write_lock(fd_1, SEEK_SET, 0, 0);
> +	if (ret == -1) {
> +		pr_perror("Failed to set write lock");
> +		kill(pid, SIGTERM);
> +		return -1;
> +	}
>   
>   	test_daemon();
>   	test_waitsig();
> diff --git a/test/zdtm/static/file_locks06.c b/test/zdtm/static/file_locks06.c
> index 2e1ba4370ddb..8eafa7c580e7 100644
> --- a/test/zdtm/static/file_locks06.c
> +++ b/test/zdtm/static/file_locks06.c
> @@ -14,7 +14,7 @@ char *filename;
>   TEST_OPTION(filename, string, "file name", 1);
>   
>   
> -int init_lock(int *fd, struct flock *lck)
> +int init_lock(int *fd, struct flock64 *lck)
>   {
>   	*fd = open(filename, O_RDWR | O_CREAT, 0666);
>   	if (*fd < 0) {
> @@ -47,7 +47,7 @@ void cleanup(int *fd)
>   int main(int argc, char **argv)
>   {
>   	int fd;
> -	struct flock lck;
> +	struct flock64 lck;
>   
>   	test_init(argc, argv);
>   	if (init_lock(&fd, &lck))
> diff --git a/test/zdtm/static/file_locks07.c b/test/zdtm/static/file_locks07.c
> index 1f946254a81b..25051b92581e 100644
> --- a/test/zdtm/static/file_locks07.c
> +++ b/test/zdtm/static/file_locks07.c
> @@ -16,12 +16,12 @@ TEST_OPTION(filename, string, "file name", 1);
>   
>   #define FILE_NUM 4
>   static int fds[FILE_NUM];
> -static struct flock lcks[FILE_NUM];
> +static struct flock64 lcks[FILE_NUM];
>   static short types[] = {F_RDLCK, F_RDLCK, F_RDLCK, F_RDLCK};
>   static off_t starts[] = {0, 10, 0, 70};
>   static off_t lens[]  = {20, 30, 100, 200};
>   
> -void fill_lock(struct flock *lock, off_t start, off_t len, short int type)
> +void fill_lock(struct flock64 *lock, off_t start, off_t len, short int type)
>   {
>   	lock->l_start = start;
>   	lock->l_len = len;
> diff --git a/test/zdtm/static/file_locks08.c b/test/zdtm/static/file_locks08.c
> index aa26d8cff01b..7963704da836 100644
> --- a/test/zdtm/static/file_locks08.c
> +++ b/test/zdtm/static/file_locks08.c
> @@ -16,7 +16,7 @@ char *filename;
>   TEST_OPTION(filename, string, "file name", 1);
>   
>   
> -int init_file_lock(int *fd, struct flock *lck)
> +int init_file_lock(int *fd, struct flock64 *lck)
>   {
>   	*fd = open(filename, O_RDWR | O_CREAT, 0666);
>   	if (*fd < 0) {
> @@ -53,7 +53,7 @@ int main(int argc, char **argv)
>   	int status;
>   	int ret = 0;
>   	task_waiter_t tw;
> -	struct flock lck;
> +	struct flock64 lck;
>   
>   	test_init(argc, argv);
>   	if (init_file_lock(&fd, &lck))
> diff --git a/test/zdtm/static/ofd_file_locks.h b/test/zdtm/static/ofd_file_locks.h
> index 049401a9cb02..b2c249a9b2dc 100644
> --- a/test/zdtm/static/ofd_file_locks.h
> +++ b/test/zdtm/static/ofd_file_locks.h
> @@ -20,7 +20,7 @@
>    * from procfs and checking them after restoring.
>    */
>   
> -static int parse_ofd_lock(char *buf, struct flock *lck)
> +static int parse_ofd_lock(char *buf, struct flock64 *lck)
>   {
>   	char fl_flag[10], fl_type[15], fl_option[10], fl_end[32];
>   	long long start;
> @@ -61,7 +61,7 @@ static int parse_ofd_lock(char *buf, struct flock *lck)
>   	return 0;
>   }
>   
> -static int read_fd_ofd_lock(int pid, int fd, struct flock *lck)
> +static int read_fd_ofd_lock(int pid, int fd, struct flock64 *lck)
>   {
>   	char path[PATH_MAX];
>   	char buf[100];
> @@ -90,7 +90,7 @@ static int read_fd_ofd_lock(int pid, int fd, struct flock *lck)
>   	return num;
>   }
>   
> -static int check_lock_exists(const char *filename, struct flock *lck)
> +static int check_lock_exists(const char *filename, struct flock64 *lck)
>   {
>   	int ret = -1;
>   	int fd;
> @@ -129,16 +129,16 @@ out:
>   	return ret;
>   }
>   
> -static int check_file_locks_match(struct flock *orig_lck, struct flock *lck)
> +static int check_file_locks_match(struct flock64 *orig_lck, struct flock64 *lck)
>   {
>   	return orig_lck->l_start == lck->l_start &&
>   		orig_lck->l_len == lck->l_len &&
>   		orig_lck->l_type == lck->l_type;
>   }
>   
> -static int check_file_lock_restored(int pid, int fd, struct flock *lck)
> +static int check_file_lock_restored(int pid, int fd, struct flock64 *lck)
>   {
> -	struct flock lck_restored;
> +	struct flock64 lck_restored;
>   
>   	if (read_fd_ofd_lock(pid, fd, &lck_restored))
>   		return -1;



More information about the CRIU mailing list