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.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
>
> Hi all!
>
> We've faced the following problem with our shared-storage migration
> scheme. We make an external snapshot and need base image to be reopened
> RO. However, bdrv_reopen reopens only .fd of BDRVRawState but not
> .lock_fd. So, .lock_fd is left opened RW and this breaks the whole
> thing.
What exactly breaks? Any specific error message, or a backtrace, or
something with more details?
>
> The simple fix is here: let's just open lock_fd as RO always. This
> looks fine for current code, as we never try to set write locks
> (qemu_lock_fd always called with exclusive=false).
I just argued that we DO try to set write locks in file-posix, and
therefore, we need more details of what the real problem is, because
this does not feel like the right fix.
>
> However it will not work if we are going to use write locks.
>
>
> block/file-posix.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 36ee89e940..5c52df9174 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -514,7 +514,8 @@ static int raw_open_common(BlockDriverState *bs, QDict
> *options,
>
> s->lock_fd = -1;
> if (s->use_lock) {
> - fd = qemu_open(filename, s->open_flags);
> + /* open it read-only, as we do not reopen it on bdrv_reopen */
> + fd = qemu_open(filename, (s->open_flags & ~BDRV_O_RDWR));
> if (fd < 0) {
> ret = -errno;
> error_setg_errno(errp, errno, "Could not open '%s' for locking",
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
