Re: [Qemu-devel] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd

[Fam Zheng]

On Fri, 06/17 14:12, Kevin Wolf wrote:
> Am 03.06.2016 um 10:48 hat Fam Zheng geschrieben:
> > They are wrappers of POSIX fcntl "file private locking".
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  include/qemu/osdep.h |  2 ++
> >  util/osdep.c         | 29 +++++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 6937694..749214a 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -280,6 +280,8 @@ int qemu_madvise(void *addr, size_t len, int advice);
> >  
> >  int qemu_open(const char *name, int flags, ...);
> >  int qemu_close(int fd);
> > +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);
> >  
> >  #if defined(__HAIKU__) && defined(__i386__)
> >  #define FMT_pid "%ld"
> > diff --git a/util/osdep.c b/util/osdep.c
> > index 9a7a439..085ed52 100644
> > --- a/util/osdep.c
> > +++ b/util/osdep.c
> > @@ -137,6 +137,35 @@ static int qemu_parse_fdset(const char *param)
> >  {
> >      return qemu_parse_fd(param);
> >  }
> > +
> > +static int qemu_lock_fcntl(int fd, int64_t start, int64_t len, int fl_type)
> > +{
> > +#ifdef F_OFD_SETLK
> > +    int ret;
> > +    struct flock fl = {
> > +        .l_whence = SEEK_SET,
> > +        .l_start  = start,
> > +        .l_len    = len,
> > +        .l_type   = fl_type,
> > +    };
> > +    do {
> > +        ret = fcntl(fd, F_OFD_SETLK, &fl);
> > +    } while (ret == -1 && errno == EINTR);
> > +    return ret == -1 ? -errno : 0;
> > +#else
> > +    return -ENOTSUP;
> > +#endif
> > +}
> 
> This will return -ENOTSUP in the case that the function wasn't available
> at build time, but -EINVAL if it was available at build time but the
> kernel doesn't support it at runtime. Should we unify this?

I'm not sure we can reliably distinguish "fcntl cmd not supported" and "fcntl
cmd supported but parameters have invalid value" out of -EINVAL.

Quoting the manual:

> https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
> 
> Macro: int F_OFD_SETL
> 
>     EINVAL
> 
>     Either the lockp argument doesn’t specify valid lock information, the
>     operating system kernel doesn’t support open file description locks, or 
> the
>     file associated with filedes doesn’t support locks.
> 

I'd expect the user to know what he's doing if he is using a kernel that is
much older than the one built QEMU, since this is relatively a very uncommon
thing to do.

Fam