From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60378) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1as1li-0004qn-OR for qemu-devel@nongnu.org; Mon, 18 Apr 2016 01:30:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1as1lh-00056v-JP for qemu-devel@nongnu.org; Mon, 18 Apr 2016 01:30:42 -0400 References: <1460690887-32751-1-git-send-email-famz@redhat.com> <1460690887-32751-6-git-send-email-famz@redhat.com> <57123E3B.3070604@openvz.org> <20160418011244.GB18893@ad-mail.usersys.redhat.com> From: "Denis V. Lunev" Message-ID: <571470F4.7010902@openvz.org> Date: Mon, 18 Apr 2016 08:30:28 +0300 MIME-Version: 1.0 In-Reply-To: <20160418011244.GB18893@ad-mail.usersys.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit 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: Fam Zheng 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 04/18/2016 04:12 AM, Fam Zheng wrote: > 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 > that seems STRANGE to me. For the time being flock() was working for Linux NFS server. Here is the implementation. int nfs_flock(struct file *filp, int cmd, struct file_lock *fl) { struct inode *inode = filp->f_mapping->host; int is_local = 0; dprintk("NFS: flock(%pD2, t=%x, fl=%x)\n", filp, fl->fl_type, fl->fl_flags); if (!(fl->fl_flags & FL_FLOCK)) return -ENOLCK; /* * The NFSv4 protocol doesn't support LOCK_MAND, which is not part of * any standard. In principle we might be able to support LOCK_MAND * on NFSv2/3 since NLMv3/4 support DOS share modes, but for now the * NFS code is not set up for it. */ if (fl->fl_type & LOCK_MAND) return -EINVAL; if (NFS_SERVER(inode)->flags & NFS_MOUNT_LOCAL_FLOCK) is_local = 1; /* We're simulating flock() locks using posix locks on the server */ if (fl->fl_type == F_UNLCK) return do_unlk(filp, cmd, fl, is_local); return do_setlk(filp, cmd, fl, is_local); } Den