Am 26.04.2017 um 15:20 hat Fam Zheng geschrieben: > 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 <f...@redhat.com> > > > Reviewed-by: Max Reitz <mre...@redhat.com> > > > --- > > > 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.
Makes sense. > 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. Yes, this sounds reasonable, too. Dependent on s->use_locks, I guess. Kevin
