From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55096) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1arxkC-0005xv-QI for qemu-devel@nongnu.org; Sun, 17 Apr 2016 21:12:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1arxkB-0007EC-HJ for qemu-devel@nongnu.org; Sun, 17 Apr 2016 21:12:52 -0400 Date: Mon, 18 Apr 2016 09:12:44 +0800 From: Fam Zheng Message-ID: <20160418011244.GB18893@ad-mail.usersys.redhat.com> References: <1460690887-32751-1-git-send-email-famz@redhat.com> <1460690887-32751-6-git-send-email-famz@redhat.com> <57123E3B.3070604@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57123E3B.3070604@openvz.org> Subject: Re: [Qemu-devel] [PATCH for-2.7 v2 05/17] raw-posix: Implement .bdrv_lockf List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: qemu-devel@nongnu.org, Kevin Wolf , Max Reitz , Jeff Cody , Markus Armbruster , Eric Blake , John Snow , qemu-block@nongnu.org, berrange@redhat.com, pbonzini@redhat.com On Sat, 04/16 16:29, Denis V. Lunev wrote: > On 04/15/2016 06:27 AM, Fam Zheng wrote: > >virtlockd in libvirt locks the first byte, we lock byte 1 to avoid > >the intervene. > > > >Suggested-by: "Daniel P. Berrange" > >Signed-off-by: Fam Zheng > >--- > > block/raw-posix.c | 35 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > >diff --git a/block/raw-posix.c b/block/raw-posix.c > >index 906d5c9..3a2c17f 100644 > >--- a/block/raw-posix.c > >+++ b/block/raw-posix.c > >@@ -35,6 +35,7 @@ > > #include "raw-aio.h" > > #include "qapi/util.h" > > #include "qapi/qmp/qstring.h" > >+#include "glib.h" > > #if defined(__APPLE__) && (__MACH__) > > #include > >@@ -397,6 +398,38 @@ static void raw_attach_aio_context(BlockDriverState *bs, > > #endif > > } > >+static int raw_lockf(BlockDriverState *bs, BdrvLockfCmd cmd) > >+{ > >+ > >+ BDRVRawState *s = bs->opaque; > >+ int ret; > >+ struct flock fl = (struct flock) { > >+ .l_whence = SEEK_SET, > >+ /* Locking byte 1 avoids interfereing with virtlockd. */ > >+ .l_start = 1, > >+ .l_len = 1, > >+ }; > >+ > >+ switch (cmd) { > >+ case BDRV_LOCKF_RWLOCK: > >+ fl.l_type = F_WRLCK; > >+ break; > >+ case BDRV_LOCKF_ROLOCK: > >+ fl.l_type = F_RDLCK; > >+ break; > >+ case BDRV_LOCKF_UNLOCK: > >+ fl.l_type = F_UNLCK; > >+ break; > >+ default: > >+ abort(); > >+ } > >+ ret = fcntl(s->fd, F_SETLK, &fl); > >+ if (ret) { > >+ ret = -errno; > >+ } > >+ return ret; > >+} > >+ > > #ifdef CONFIG_LINUX_AIO > > static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags) > > { > >@@ -1960,6 +1993,8 @@ BlockDriver bdrv_file = { > > .bdrv_detach_aio_context = raw_detach_aio_context, > > .bdrv_attach_aio_context = raw_attach_aio_context, > >+ .bdrv_lockf = raw_lockf, > >+ > > .create_opts = &raw_create_opts, > > }; > would it be better to use > > int flock(int fd, int operation); > > DESCRIPTION > Apply or remove an advisory lock on the open file specified by fd. > The > argument operation is one of the following: > > LOCK_SH Place a shared lock. More than one process may hold > a > shared lock for a given file at a given time. > > LOCK_EX Place an exclusive lock. Only one process may hold > an > exclusive lock for a given file at a given time. > > LOCK_UN Remove an existing lock held by this process. > > for this purpose? > > Sorry that missed this point in the initial review... > We will not intersect with libvirt for the case. > As noted in the cover letter, flock() is nop on NFS mount points on Linux, so fcntl is safer. Fam