From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b02V8-0005aF-IW for qemu-devel@nongnu.org; Tue, 10 May 2016 03:54:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b02V7-0002Rs-6C for qemu-devel@nongnu.org; Tue, 10 May 2016 03:54:42 -0400 Date: Tue, 10 May 2016 08:54:31 +0100 From: "Richard W.M. Jones" Message-ID: <20160510075431.GU1683@redhat.com> References: <1462848659-28659-1-git-send-email-famz@redhat.com> <1462848659-28659-9-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462848659-28659-9-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 08/27] osdep: Add qemu_lock_fd and qemu_unlock_fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-devel@nongnu.org, Kevin Wolf , Max Reitz , Jeff Cody , Markus Armbruster , Eric Blake , qemu-block@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, John Snow , berrange@redhat.com, den@openvz.org 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