qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH RFC] file-posix: make lock_fd read-only
Date: Wed, 11 Oct 2017 12:38:36 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

11.10.2017 12:22, Kevin Wolf wrote:
[ Cc: Fam ]

Am 10.10.2017 um 15:42 hat Vladimir Sementsov-Ogievskiy geschrieben:
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.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---

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.

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).

However it will not work if we are going to use write locks.
I was sure that we had discussed this during review, so I just went back
and checked. Indeed, Fam originally had an unconditional O_RDONLY in
some version of the image locking patches, but I actually found a
potential problem with that back then:

Note that with /dev/fdset there can be cases where we can open a file
O_RDWR, but not O_RDONLY. Should we better just use the same flags as
for the s->fd?
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05107.html

However, I'm now wondering whether we really still need a separate
s->lock_fd or whether we can just use the normal image fd for this. If I
understood the old threads correctly, the original reason for it was
that during bdrv_reopen(), we couldn't safely migrate exclusive locks
from the old fd to the new one. But as we aren't using exclusive locks
any more, this shouldn't be a problem today.

Fam, are there more reasons why we need a separate lock_fd?

Kevin

If I understand correctly, posix lock will be lost on fd close anyway, so other app will have an opportunity of taking this lock, so it's unsafe.

--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]