Re: [Qemu-devel] [PATCH v8 00/36] block: Image locking series

[Kevin Wolf]

Am 26.10.2016 um 17:12 hat Max Reitz geschrieben:
> On 26.10.2016 13:01, Fam Zheng wrote:
> > On Tue, 10/25 16:57, Kevin Wolf wrote:
> >> Am 25.10.2016 um 15:30 hat Max Reitz geschrieben:
> >>> On 25.10.2016 10:24, Kevin Wolf wrote:
> >>>> Am 24.10.2016 um 20:03 hat Max Reitz geschrieben:
> >>>>> On 24.10.2016 12:11, Kevin Wolf wrote:
> >>>>>
> >>>>> [...]
> >>>>>
> >>>>>> Now, the big question is how to translate this into file locking. This
> >>>>>> could become a little tricky. I had a few thoughts involving another
> >>>>>> lock on byte 2, but none of them actually worked out so far, because
> >>>>>> what we want is essentially a lock that can be shared by readers, that
> >>>>>> can also be shared by writers, but not by readers and writers at the
> >>>>>> same time.
> >>>>>
> >>>>> You can also share it between readers and writers, as long as everyone
> >>>>> can cope with volatile data.
> >>>>
> >>>> Sorry, that was ambiguous. I meant a file-level lock rather than the
> >>>> high-level one. If we had a lock that can be shared by one or the other,
> >>>> but not both, then two locks would be enough to build what we really
> >>>> want.
> >>>>
> >>>>> I agree that it's very similar to the proposed op blocker style, but I
> >>>>> can't really come up with a meaningful translation either.
> >>>>>
> >>>>> Maybe something like this (?): All readers who do not want the file to
> >>>>> be modified grab a shared lock on byte 1. All writers who can deal with
> >>>>> volatile data grab a shared lock on byte 2. Exclusive writers grab an
> >>>>> exclusive lock on byte 1 and 2. Readers who can cope with volatile data
> >>>>> get no lock at all.
> >>>>>
> >>>>> When opening, the first and second group would always have to test
> >>>>> whether there is a lock on the other byte, respectively. E.g. sharing
> >>>>> writers would first grab an exclusive lock on byte 1, then the shared
> >>>>> lock on byte 2 and then release the exclusive lock again.
> >>>>>
> >>>>> Would that work?
> >>>>
> >>>> I'm afraid it wouldn't. If you start the sharing writer first and then
> >>>> the writer-blocking reader, the writer doesn't hold a lock on byte 1 any
> >>>> more,
> >>>
> >>> But it holds a lock on byte 2.
> >>>
> >>>>       so the reader can start even though someone is writing to the
> >>>> image.
> >>>
> >>> It can't because it would try to grab an exclusive lock on byte 2 before
> >>> grabbing the shared lock on byte 1.
> >>
> >> Apparently I failed to understand the most important part of the
> >> proposal. :-)
> >>
> >> So we have two locks. Both are only held for a longer time in shared
> >> mode. Exclusive mode is only used for testing whether the lock is being
> >> held and is immediately given up again.
> >>
> >> The meaning of holding a shared lock is:
> >>
> >>     byte 1: I can't allow other processes to write to the image
> >>     byte 2: I am writing to the image
> >>
> >> The four cases that we have involve:
> >>
> >> * shared writer: Take shared lock on byte 2. Test whether byte 1 is
> >>   locked using an exclusive lock, and fail if so.
> >>
> >> * exclusive writer: Take shared lock on byte 2. Test whether byte 1 is
> >>   locked using an exclusive lock, and fail if so. Then take shared lock
> >>   on byte 1. I suppose this is racy, but we can probably tolerate that.
> >>
> >> * reader that can tolerate writers: Don't do anything
> >>
> >> * reader that can't tolerate writers: Take shared lock on byte 1. Test
> >>   whether byte 2 is locked, and fail if so.
> >>
> >> Seems to work if I got that right.
> > 
> > Does this mean I should change ImageLockMode to:
> > 
> >  * exclusive
> >  * shared-write
> >  * shared-read
> 
> Hm, those don't sound quite right, since it sounds as if you could mix
> shared-read and shared-write. But you shouldn't be able to open an image
> in shared-read lock mode when someone has opened it in shared-write lock
> mode already.
> 
> It's difficult to come up with a clear but short name for shared-read
> ("exclusive", "shared-write", and "nolock" sound good to me). Maybe
> "non-volatile" or "constant"? Or maybe "shared-only-read" would be clear
> enough?

As we concluded above, this is really a matrix a two bools rather than a
single property. We need a new option for "I can't allow other processes
to write to the image", but we don't need one for "I am writing to the
image" because that's the read-only property that we already have.

> >  * nolock
> >  * auto
> > 
> > Where "auto" maps to exclusive for O_RDWR and shared-read for O_RDONLY?
> 
> Yep, that would be the correct mapping. Maybe later we can introduce an
> auto-shared mode that maps to shared-write or nolock, respectively.

No auto needed any more, the default is simply false (i.e. don't share
with other writers).

Kevin

pgpI_d7CmOB4T.pgp
Description: PGP signature