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
