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();
> 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.
> + 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 :|
