On Wed, 04/26 14:57, Kevin Wolf wrote:
> Am 26.04.2017 um 05:34 hat Fam Zheng geschrieben:
> > They are wrappers of POSIX fcntl "file private locking", with a
> > convenient "try lock" wrapper implemented with F_OFD_GETLK.
> >
> > Signed-off-by: Fam Zheng <[email protected]>
> > Reviewed-by: Max Reitz <[email protected]>
> > ---
> > include/qemu/osdep.h | 3 +++
> > util/osdep.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 51 insertions(+)
> >
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 122ff06..1c9f5e2 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -341,6 +341,9 @@ int qemu_close(int fd);
> > #ifndef _WIN32
> > int qemu_dup(int fd);
> > #endif
> > +int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
> > +int qemu_unlock_fd(int fd, int64_t start, int64_t len);
> > +int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
>
> For the record: On IRC, I proposed adding something like the following:
>
> #ifndef F_OFD_SETLK
> #define F_OFD_SETLK F_SETLK
> #define F_OFD_GETLK F_GETLK
> #endif
>
> F_OFD_* are still relatively new and e.g. RHEL 7 doesn't support it yet.
> Using process-based locks is suboptimal because we can easily lose them
> earlier than we want, but it's still better than nothing and covers the
> common simple cases.
Yes, we should add that. But I'd prefer:
#ifdef F_OFD_SETLK
#define QEMU_SETLK F_OFD_SETLK
#define QEMU_GETLK F_OFD_GETLK
#else
#define QEMU_SETLK F_SETLK
#define QEMU_GETLK F_GETLK
#endif
to avoid "abusing" the macro name.
Another question is whether we should print a warning to make users aware? Even
the test case in patch 21 can see three "lock losses" on RHEL with posix lock,
and there are way more corner cases, I believe.
We can print a warning to stderr in raw_open_common when F_OFD_GETLK is not
available, I think.
Fam
