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

[Li Feng <fengli@smartx.com>]

Hi Kevin,

OK, thanks for your reply, I will send out the v2 patch.

Thanks,
Feng Li

Kevin Wolf <kwolf@redhat.com> 于2020年12月7日周一 下午7:17写道：
>
> 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
> > > > >
> > > >
> >
>