Re: [PATCH] file-posix: check the use_lock

[Kevin Wolf]

Am 07.12.2020 um 11:50 hat Feng Li geschrieben:
> Hi Kevin,
> 
> I have read the 182 test, and I'm very confused about the test.
> I'm not familiar with the permissions and locks in the qemu.
> Could you give me more tips about how to complete the test?

Hm, actually, to produce a failure, we would have to have a filesystem
in the host that doesn't support locks. I don't even know how to get
such a filesystem manually, and it's probably completely impossible in a
test case without root permissions.

So maybe just add a more detailed description of the bug to the commit
message, and we'll have to apply it without a test.

Kevin

> Li Feng <fengli@smartx.com> 于2020年12月4日周五 下午6:55写道：
> >
> > Hi Kevin,
> > Thanks for your reply.
> >
> > In my scenario, my NFS server doesn't support the file lock.
> > And when I set the file.locking = false, the Qemu still reports:
> > qemu-system-x86_64: -drive
> > file=/tmp/nfs/a,format=raw,cache=none,aio=native,if=none,id=drive-virtio-disk1,file.locking=on:
> > Failed to lock byte 100
> >
> > I will look at the iotest 182 and try to add a test.
> >
> > Thanks,
> > Feng Li
> >
> > Kevin Wolf <kwolf@redhat.com> 于2020年12月4日周五 下午6:40写道：
> > >
> > > Am 04.12.2020 um 11:28 hat Li Feng geschrieben:
> > > > When setting the file.locking = false, we shouldn't set the lock.
> > > >
> > > > Signed-off-by: Li Feng <fengli@smartx.com>
> > >
> > > This looks right to me, but can you add a test for this scenario to
> > > iotest 182? This would both demonstrate the effect of the bug (I think
> > > it would be that files are locked after reopen even with locking=off?)
> > > and make sure that we won't have a regression later. Mentioning the
> > > effect in the commit message would be good, too.
> > >
> > > Kevin
> > >
> > > >  block/file-posix.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/block/file-posix.c b/block/file-posix.c
> > > > index d5fd1dbcd2..806764f7e3 100644
> > > > --- a/block/file-posix.c
> > > > +++ b/block/file-posix.c
> > > > @@ -3104,7 +3104,7 @@ static int raw_check_perm(BlockDriverState *bs, 
> > > > uint64_t perm, uint64_t shared,
> > > >      }
> > > >
> > > >      /* Copy locks to the new fd */
> > > > -    if (s->perm_change_fd) {
> > > > +    if (s->perm_change_fd && s->use_lock) {
> > > >          ret = raw_apply_lock_bytes(NULL, s->perm_change_fd, perm, 
> > > > ~shared,
> > > >                                     false, errp);
> > > >          if (ret < 0) {
> > > > --
> > > > 2.24.3
> > > >
> > >
>