Daniel P. Berrangé <berra...@redhat.com> 于2020年12月15日周二 下午6:08写道：
>
> On Tue, Dec 15, 2020 at 03:09:28PM +0800, Li Feng wrote:
> > This patch addresses this issue:
> > When accessing a volume on an NFS filesystem without supporting the file
> > lock,
> > tools, like qemu-img, will complain "Failed to lock byte 100".
> >
> > In the original code, the qemu_has_ofd_lock will test the lock on the
> > "/dev/null" pseudo-file. Actually, the file.locking is per-drive property,
> > which depends on the underlay filesystem.
> >
> > In this patch, add a new 'qemu_has_file_lock' to detect whether the
> > file supports the file lock. And disable the lock when the underlay file
> > system doesn't support locks.
> >
> > Signed-off-by: Li Feng <fen...@smartx.com>
> > ---
> > v4: use the fd as the qemu_has_file_lock argument.
> > v3: don't call the qemu_has_ofd_lock, use a new function instead.
> > v2: remove the refactoring.
> > ---
> > block/file-posix.c | 66 +++++++++++++++++++++++++-------------------
> > include/qemu/osdep.h | 1 +
> > util/osdep.c | 19 +++++++++++++
> > 3 files changed, 58 insertions(+), 28 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index 806764f7e3..9708212f01 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -584,6 +584,21 @@ static int raw_open_common(BlockDriverState *bs, QDict
> > *options,
> > s->use_linux_io_uring = (aio == BLOCKDEV_AIO_OPTIONS_IO_URING);
> > #endif
> >
> > + s->open_flags = open_flags;
> > + raw_parse_flags(bdrv_flags, &s->open_flags, false);
> > +
> > + s->fd = -1;
> > + fd = qemu_open(filename, s->open_flags, errp);
> > + ret = fd < 0 ? -errno : 0;
> > +
> > + if (ret < 0) {
> > + if (ret == -EROFS) {
> > + ret = -EACCES;
> > + }
> > + goto fail;
> > + }
> > + s->fd = fd;
> > +
> > locking = qapi_enum_parse(&OnOffAuto_lookup,
> > qemu_opt_get(opts, "locking"),
> > ON_OFF_AUTO_AUTO, &local_err);
> > @@ -607,6 +622,13 @@ static int raw_open_common(BlockDriverState *bs, QDict
> > *options,
> > break;
> > case ON_OFF_AUTO_AUTO:
> > s->use_lock = qemu_has_ofd_lock();
> > + if (s->use_lock && !qemu_has_file_lock(s->fd)) {
> > + /*
> > + * When the os supports the OFD lock, but the filesystem
> > doesn't
> > + * support, just disable the file lock.
> > + */
> > + s->use_lock = false;
> > + }
>
> This should just be
>
> s->use_lock = qemu_has_file_lock(s->fd) && qemu_has_ofd_lock();
>
Acked.
>
>
> > diff --git a/util/osdep.c b/util/osdep.c
> > index 66d01b9160..07de97e2c5 100644
> > --- a/util/osdep.c
> > +++ b/util/osdep.c
> > @@ -225,6 +225,25 @@ static void qemu_probe_lock_ops(void)
> > }
> > }
> >
> > +bool qemu_has_file_lock(int fd)
> > +{
> > +#ifdef F_OFD_SETLK
> > + int cmd = F_OFD_GETLK;
> > +#else
> > + int cmd = F_GETLK;
> > +#endif
>
> This should unconditionally use F_GETLK.
Acked.
>
> > + int ret;
> > + struct flock fl = {
> > + .l_whence = SEEK_SET,
> > + .l_start = 0,
> > + .l_len = 0,
> > + .l_type = F_WRLCK,
> > + };
> > +
> > + ret = fcntl(fd, cmd, &fl);
> > + return ret == 0;
> > +}
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
