Re: [Qemu-devel] [PATCH RFC] file-posix: make lock_fd read-only

[Denis V. Lunev]

On 10/10/2017 09:50 PM, Eric Blake wrote:
> On 10/10/2017 08:42 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We do not reopen lock_fd on bdrv_reopen which leads to problems on
>> reopen image RO. So, lets make lock_fd be always RO.
>> This is correct, because qemu_lock_fd always called with exclusive=false
>> on lock_fd.
> How is that correct?  file-posix.c calls:
>             ret = qemu_lock_fd_test(s->lock_fd, off, 1, true);
> where exclusive being true in turn sets:
>         .l_type   = exclusive ? F_WRLCK : F_RDLCK,
>
> and F_WRLCK requests fail on a RO fd with EBADF.
this works fine for us as here we do not get the lock but just
query it.

#include <stdio.h>
#include <fcntl.h>
#include <sys/types.h>
#include <unistd.h>

int main()
{
        int fd = open("file", O_RDONLY | O_CREAT, 0666);
        struct flock fl_rd = {
                .l_whence = SEEK_SET,
                .l_start  = 0,
                .l_len    = 1,
                .l_type   = F_RDLCK,
        };
        struct flock fl_wr = {
                .l_whence = SEEK_SET,
                .l_start  = 0,
                .l_len    = 1,
                .l_type   = F_WRLCK,
        };

        ftruncate(fd, 1024);
        fcntl(fd, F_SETLK, &fl_rd);
        fcntl(fd, F_GETLK, &fl_wr);
        sleep(1000);
        return 0;
}

The first process:
open("file", O_RDONLY|O_CREAT, 0666)    = 3
ftruncate(3, 1024)                      = -1 EINVAL (Invalid argument)
fcntl(3, F_SETLK, {l_type=F_RDLCK, l_whence=SEEK_SET, l_start=0,
l_len=1}) = 0
fcntl(3, F_GETLK, {l_type=F_UNLCK, l_whence=SEEK_SET, l_start=0,
l_len=1, l_pid=0}) = 0
nanosleep({1000, 0},

The second process:
open("file", O_RDONLY|O_CREAT, 0666)    = 3
ftruncate(3, 1024)                      = -1 EINVAL (Invalid argument)
fcntl(3, F_SETLK, {l_type=F_RDLCK, l_whence=SEEK_SET, l_start=0,
l_len=1}) = 0
fcntl(3, F_GETLK, {l_type=F_RDLCK, l_whence=SEEK_SET, l_start=0,
l_len=1, l_pid=19540}) = 0
nanosleep({1000, 0},

The key is that in test cod we do not _set_ the lock, but query it! This
is allowed even on
RDONLY file descriptor.

Den