Re: [Qemu-devel] [PATCH v4 08/27] osdep: Add qemu_lock_fd and qemu_unlock_fd

[Richard W.M. Jones]

On Tue, May 10, 2016 at 10:50:40AM +0800, Fam Zheng wrote:
> +int qemu_lock_fd(int fd, int64_t start, int64_t len, bool readonly)

I find this new API to be very unintuitive.  When I was reading the
other code in block/raw-posix.c I had to refer back to this file to
find out what all the integers meant.

It is also misleading.  There's aren't really such a thing as
"readonly locks", or "read locks" or "write locks".  I know that
(some) POSIX APIs use these terms, but the terms are wrong.

There are shared locks, and there are exclusive locks.  The locks have
nothing to do with reading or writing.  In fact the locks only apply
when writing, and are to do with whether you want to allow multiple
writers at the same time (shared lock), or only a single writer
(exclusive lock).

Anyway, I think the boolean "readonly" should be replaced by
some flag like:

#define QEMU_LOCK_SHARED 1
#define QEMU_LOCK_EXCLUSIVE 2

or similar.

Not sure about the start/len.  Do we need them in the API at all given
that we've decided to lock a particular byte of the file?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html