[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Use flock() instead of fcntl()



Am Donnerstag, 25. Juli 2013, 11:29:37 schrieb Daniel P. Berrange:
> On Thu, Jul 25, 2013 at 12:07:44PM +0200, David Weber wrote:
> > Thank you for your quick response!
> > 
> > Am Donnerstag, 25. Juli 2013, 10:31:40 schrieb Daniel P. Berrange:
> > > On Thu, Jul 25, 2013 at 08:23:24AM +0000, David Weber wrote:
> > > > Hi,
> > > > 
> > > > we are interested in using virtlockd on an OCFS2 shared filesystem.
> > > > We are now facing the problem that virtlockd uses fcntl() locks which
> > > > aren't supported by OCFS2 with the o2cb cluster stack and we want
> > > > to avoid using indirect leases.
> > > > 
> > > > OCFS2 instead supports flock() which is quite similar to fcntl(). I
> > > > attached a patch which makes libvirt use flock() *instead* of fcntl()
> > > > and it seems to work.
> > > > 
> > > > NFS on the contrast only supports fcntl() so it should be configurable
> > > > which lock type to use.
> > > > 
> > > > I'm not very experienced with locking, so would such a patch be
> > > > acceptable or do you see possible problems with it?
> > > 
> > > We definitely can't use flock() unconditionally like that, as it is
> > > less widely supported than fcntl() locking and is known broken on
> > > most network filesystems (flock() will only lock wrt the local node,
> > > not network-wide).
> > 
> > flock() locks are cluster aware in recent versions of OCFS2 and I would
> > try to make it configurable which lock type to use.
> > 
> > > I'm pretty surprised that you can fcntl() is not supported in ocfs2.
> > > Are you really sure about that ?
> > > 
> > > This mail message suggests it is supported
> > > 
> > >   https://oss.oracle.com/pipermail/ocfs2-users/2009-June/003572.html
> > > 
> > > "Support for fcntl locking aka file-range locking aka posix locking is
> > > 
> > >  provided by vfs for all file systems. However, that support is
> > >  appropriate
> > >  only for local file systems.
> > >  
> > >  In ocfs2, we have added support for cluster-aware fcntl locking via
> > >  the userspace clustering framework that allows one to use ocfs2 with
> > >  different cluster-stacks."
> > 
> > OCFS2 supports fcntl() only with the userspace cluster stacks (pacemaker
> > and cman) which we want to avoid.
> 
> Looking again at flock() I see it cannot support locking of ranges, only
> the entire file. This makes it unsuitable as a replacement for libvirt's
> use of fcntl() I'm afraid. I can only sugggest you configure OCFS2 so
> that it supports fcntl(), or setup virtlockd to use separate indirect
> leases on a diffrent shared filesystem, or perhaps try sanlock instead
> which doesn't require any special filesystem support.

It's true that flock() doesn't support locking of ranges but I can't see how 
this is necessary. 
I attached a patch with extends virtlockd so it can either use flock()  *or* 
fcntl(). Configurable in the configuration file.

> 
> > SUSE advises to use indirect leases which we also want to avoid:
> > https://www.suse.com/documentation//sled11/book_kvm/?page=/documentation//
> > sled11/book_kvm/data/sec_libvirt_storage_locking.html
> > 
> > "virtlockd's default configuration does not allow you to lock disk files
> > placed on a shared file system (for example NFS or OCFS2). Locking on
> > these file systems requires you to specify a lockspace directory on the
> > VM Host Server by setting"
> > 
> > Although that's not completely correct because NFS supports fcntl()
> 
> That's just badly written explanation for that config setting. It should
> really be saying that the default configuration does not provide protection
> across multiple hosts for file paths which are not visible via a shared
> filesystem. eg a SAN LUN in /dev/sdNNN won't be protected in the default
> config.
> 
> Daniel








>From 2a91cc505b4fb5b467d2add416b2ebf4baa53311 Mon Sep 17 00:00:00 2001
From: David Weber <wb munzinger de>
Date: Thu, 25 Jul 2013 14:10:23 +0200
Subject: [PATCH] Add flock() support for locking

Configurable in the virtlockd config
---
 src/locking/libvirt_lockd.aug      |  1 +
 src/locking/lock_daemon_dispatch.c |  6 ++-
 src/locking/lock_driver_lockd.c    | 14 ++++++-
 src/locking/lock_protocol.x        |  3 +-
 src/locking/lockd.conf             |  7 ++++
 src/util/virfile.c                 | 59 +++++++++++++++++++++++++++-
 src/util/virfile.h                 |  4 ++
 src/util/virlockspace.c            | 80 
++++++++++++++++++++++++++++----------
 src/util/virlockspace.h            |  1 +
 9 files changed, 148 insertions(+), 27 deletions(-)

diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug
index 6a3bcba..7df715f 100644
--- a/src/locking/libvirt_lockd.aug
+++ b/src/locking/libvirt_lockd.aug
@@ -19,6 +19,7 @@ module Libvirt_lockd =
    (* Each enty in the config is one of the following three ... *)
    let entry = bool_entry "auto_disk_leases"
              | bool_entry "require_lease_for_disks"
+             | bool_entry "useFlock"
              | str_entry "file_lockspace_dir"
 	     | str_entry "lvm_lockspace_dir"
 	     | str_entry "scsi_lockspace_dir"
diff --git a/src/locking/lock_daemon_dispatch.c 
b/src/locking/lock_daemon_dispatch.c
index c2e1000..930018c 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -50,7 +50,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr 
server ATTRIBUTE_UNU
     virMutexLock(&priv->lock);
 
     virCheckFlagsGoto(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
-                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE, 
cleanup);
+                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE |
+                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK, 
cleanup);
 
     if (priv->restricted) {
         virReportError(VIR_ERR_OPERATION_DENIED, "%s",
@@ -76,7 +77,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr 
server ATTRIBUTE_UNU
         newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED;
     if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE)
         newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE;
-
+    if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK)
+        newFlags |= VIR_LOCK_SPACE_ACQUIRE_FLOCK;
     if (virLockSpaceAcquireResource(lockspace,
                                     args->name,
                                     priv->ownerPid,
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index b115799..1e07162 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -71,6 +71,7 @@ struct _virLockManagerLockDaemonPrivate {
 struct _virLockManagerLockDaemonDriver {
     bool autoDiskLease;
     bool requireLeaseForDisks;
+    bool useFlock;
 
     char *fileLockSpaceDir;
     char *lvmLockSpaceDir;
@@ -125,6 +126,10 @@ static int virLockManagerLockDaemonLoadConfig(const char 
*configFile)
     CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG);
     if (p) driver->autoDiskLease = p->l;
 
+    p = virConfGetValue(conf, "use_flock");
+    CHECK_TYPE("use_flock", VIR_CONF_LONG);
+    if (p) driver->useFlock = p->l;
+
     p = virConfGetValue(conf, "file_lockspace_dir");
     CHECK_TYPE("file_lockspace_dir", VIR_CONF_STRING);
     if (p && p->str) {
@@ -379,6 +384,7 @@ static int virLockManagerLockDaemonInit(unsigned int 
version,
 
     driver->requireLeaseForDisks = true;
     driver->autoDiskLease = true;
+    driver->useFlock = false;
 
     if (virLockManagerLockDaemonLoadConfig(configFile) < 0)
         goto error;
@@ -724,6 +730,8 @@ static int 
virLockManagerLockDaemonAcquire(virLockManagerPtr lock,
             args.name = priv->resources[i].name;
             args.flags = priv->resources[i].flags;
 
+            if (driver->useFlock)
+                args.flags |= 
VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK;
             if (virNetClientProgramCall(program,
                                         client,
                                         counter++,
@@ -780,10 +788,12 @@ static int 
virLockManagerLockDaemonRelease(virLockManagerPtr lock,
         args.name = priv->resources[i].name;
         args.flags = priv->resources[i].flags;
 
-        args.flags &=
-            ~(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
+        args.flags &=~(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
               VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE);
 
+        if (driver->useFlock)
+            args.flags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK;
+
         if (virNetClientProgramCall(program,
                                     client,
                                     counter++,
diff --git a/src/locking/lock_protocol.x b/src/locking/lock_protocol.x
index 354d51a..7828674 100644
--- a/src/locking/lock_protocol.x
+++ b/src/locking/lock_protocol.x
@@ -52,7 +52,8 @@ struct virLockSpaceProtocolDeleteResourceArgs {
 
 enum virLockSpaceProtocolAcquireResourceFlags {
     VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED     = 1,
-    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2
+    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2,
+    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK  = 4
 };
 
 struct virLockSpaceProtocolAcquireResourceArgs {
diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf
index 85edb91..944c808 100644
--- a/src/locking/lockd.conf
+++ b/src/locking/lockd.conf
@@ -17,6 +17,13 @@
 #
 #require_lease_for_disks = 1
 
+#
+# The default lockd behaviour is to acquire locks via their
+# the fnctl(2) syscall. This isn't aviable on all filesystems.
+# In this case you can use this flag to use flock(2) instead.
+#
+#use_flock = 0
+
 
 #
 # The default lockd behaviour is to use the "direct"
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 8f0eec3..56900f0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -378,7 +378,7 @@ int virFileLock(int fd, bool shared, off_t start, off_t len)
  * @start: byte offset to start unlock
  * @len: length of lock (0 to release entire remaining file from @start)
  *
- * Release a lock previously acquired with virFileUnlock().
+ * Release a lock previously acquired with virFileLock().
  * NB the lock will also be released if any open file descriptor
  * pointing to the same file as @fd is closed
  *
@@ -398,6 +398,63 @@ int virFileUnlock(int fd, off_t start, off_t len)
 
     return 0;
 }
+
+/**
+ * virFileLockFlock:
+ * @fd: file descriptor to acquire the lock on
+ * @shared: type of lock to acquire
+ *
+ * Attempt to acquire a lock on the file @fd. If @shared
+ * is true, then a shared lock will be acquired,
+ * otherwise an exclusive lock will be acquired. If
+ * the lock cannot be acquired, an error will be
+ * returned. This will not wait to acquire the lock if
+ * another process already holds it.
+ *
+ * The lock will be released when @fd is closed. The lock
+ * will also be released if *any* other open file descriptor
+ * pointing to the same underlying file is closed. As such
+ * this function should not be relied on in multi-threaded
+ * apps where other threads can be opening/closing arbitrary
+ * files.
+ *
+ * Returns 0 on success, or -errno otherwise
+ */
+int virFileLockFlock(int fd, bool shared)
+{
+     if (shared) {
+         if (flock(fd, LOCK_SH | LOCK_NB) < 0)
+             return -errno;
+     } else {
+         if (flock(fd, LOCK_EX | LOCK_NB) < 0)
+             return -errno;
+     }
+
+    return 0;
+}
+
+
+/**
+ * virFileUnlockFlock:
+ * @fd: file descriptor to release the lock on
+ * @start: byte offset to start unlock
+ *
+ * Release a lock previously acquired with virFileLockFlock().
+ * NB the lock will also be released if any open file descriptor
+ * pointing to the same file as @fd is closed
+ *
+ * Returns 0 on succcess, or -errno on error
+ */
+int virFileUnlockFlock(int fd)
+{
+     if (flock(fd, LOCK_UN) < 0)
+         return -errno;
+
+    return 0;
+}
+
+
+
 #else
 int virFileLock(int fd ATTRIBUTE_UNUSED,
                 bool shared ATTRIBUTE_UNUSED,
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72d35ce..e8ced5c 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -100,6 +100,10 @@ void virFileWrapperFdFree(virFileWrapperFdPtr dfd);
 int virFileLock(int fd, bool shared, off_t start, off_t len);
 int virFileUnlock(int fd, off_t start, off_t len);
 
+int virFileLockFlock(int fd, bool shared);
+int virFileUnlockFlock(int fd);
+
+
 typedef int (*virFileRewriteFunc)(int fd, void *opaque);
 int virFileRewrite(const char *path,
                    mode_t mode,
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index afb1abb..ef95672 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -154,17 +154,32 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
                 goto error;
             }
 
-            if (virFileLock(res->fd, shared, 0, 1) < 0) {
-                if (errno == EACCES || errno == EAGAIN) {
-                    virReportError(VIR_ERR_RESOURCE_BUSY,
-                                   _("Lockspace resource '%s' is locked"),
-                                   resname);
-                } else {
-                    virReportSystemError(errno,
-                                         _("Unable to acquire lock on '%s'"),
-                                         res->path);
+            if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK) {
+                if (virFileLockFlock(res->fd, shared) < 0) {
+                    if (errno == EACCES || errno == EAGAIN) {
+                        virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                    } else {
+                        virReportSystemError(errno,
+                                            _("Unable to acquire lock on 
'%s'"),
+                                            res->path);
+                    }
+                    goto error;
+                }
+            } else {
+                if (virFileLock(res->fd, shared, 0, 1) < 0) {
+                    if (errno == EACCES || errno == EAGAIN) {
+                        virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                    } else {
+                        virReportSystemError(errno,
+                                            _("Unable to acquire lock on 
'%s'"),
+                                            res->path);
+                    }
+                    goto error;
                 }
-                goto error;
             }
 
             /* Now make sure the pidfile we locked is the same
@@ -201,17 +216,32 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
             goto error;
         }
 
-        if (virFileLock(res->fd, shared, 0, 1) < 0) {
-            if (errno == EACCES || errno == EAGAIN) {
-                virReportError(VIR_ERR_RESOURCE_BUSY,
-                               _("Lockspace resource '%s' is locked"),
-                               resname);
-            } else {
-                virReportSystemError(errno,
-                                     _("Unable to acquire lock on '%s'"),
-                                     res->path);
+        if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK) {
+            if (virFileLockFlock(res->fd, shared) < 0) {
+                if (errno == EACCES || errno == EAGAIN) {
+                    virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                } else {
+                    virReportSystemError(errno,
+                                            _("Unable to acquire lock on 
'%s'"),
+                                            res->path);
+                }
+                goto error;
+            }
+        } else {
+            if (virFileLock(res->fd, shared, 0, 1) < 0) {
+                if (errno == EACCES || errno == EAGAIN) {
+                    virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                } else {
+                    virReportSystemError(errno,
+                                            _("Unable to acquire lock on 
'%s'"),
+                                            res->path);
+                }
+                goto error;
             }
-            goto error;
         }
     }
     res->lockHeld = true;
@@ -616,9 +646,17 @@ int virLockSpaceAcquireResource(virLockSpacePtr 
lockspace,
 
     VIR_DEBUG("lockspace=%p resname=%s flags=%x owner=%lld",
               lockspace, resname, flags, (unsigned long long)owner);
+    
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)
+         VIR_DEBUG("David: shared");
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE)
+         VIR_DEBUG("David: autocreate");
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK)
+         VIR_DEBUG("David: flock");
 
     virCheckFlags(VIR_LOCK_SPACE_ACQUIRE_SHARED |
-                  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, -1);
+                  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE |
+                  VIR_LOCK_SPACE_ACQUIRE_FLOCK, -1);
 
     virMutexLock(&lockspace->lock);
 
diff --git a/src/util/virlockspace.h b/src/util/virlockspace.h
index 041cf20..a6ba3dd 100644
--- a/src/util/virlockspace.h
+++ b/src/util/virlockspace.h
@@ -45,6 +45,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
 typedef enum {
     VIR_LOCK_SPACE_ACQUIRE_SHARED     = (1 << 0),
     VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE = (1 << 1),
+    VIR_LOCK_SPACE_ACQUIRE_FLOCK      = (1 << 2),
 } virLockSpaceAcquireFlags;
 
 int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
-- 
1.8.1.2




>From 2a91cc505b4fb5b467d2add416b2ebf4baa53311 Mon Sep 17 00:00:00 2001
From: David Weber <wb munzinger de>
Date: Thu, 25 Jul 2013 14:10:23 +0200
Subject: [PATCH] Add flock() support for locking

Configurable in the virtlockd config
---
 src/locking/libvirt_lockd.aug      |  1 +
 src/locking/lock_daemon_dispatch.c |  6 ++-
 src/locking/lock_driver_lockd.c    | 14 ++++++-
 src/locking/lock_protocol.x        |  3 +-
 src/locking/lockd.conf             |  7 ++++
 src/util/virfile.c                 | 59 +++++++++++++++++++++++++++-
 src/util/virfile.h                 |  4 ++
 src/util/virlockspace.c            | 80 ++++++++++++++++++++++++++++----------
 src/util/virlockspace.h            |  1 +
 9 files changed, 148 insertions(+), 27 deletions(-)

diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug
index 6a3bcba..7df715f 100644
--- a/src/locking/libvirt_lockd.aug
+++ b/src/locking/libvirt_lockd.aug
@@ -19,6 +19,7 @@ module Libvirt_lockd =
    (* Each enty in the config is one of the following three ... *)
    let entry = bool_entry "auto_disk_leases"
              | bool_entry "require_lease_for_disks"
+             | bool_entry "useFlock"
              | str_entry "file_lockspace_dir"
 	     | str_entry "lvm_lockspace_dir"
 	     | str_entry "scsi_lockspace_dir"
diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
index c2e1000..930018c 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -50,7 +50,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU
     virMutexLock(&priv->lock);
 
     virCheckFlagsGoto(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
-                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE, cleanup);
+                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE |
+                      VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK, cleanup);
 
     if (priv->restricted) {
         virReportError(VIR_ERR_OPERATION_DENIED, "%s",
@@ -76,7 +77,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU
         newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED;
     if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE)
         newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE;
-
+    if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK)
+        newFlags |= VIR_LOCK_SPACE_ACQUIRE_FLOCK;
     if (virLockSpaceAcquireResource(lockspace,
                                     args->name,
                                     priv->ownerPid,
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index b115799..1e07162 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -71,6 +71,7 @@ struct _virLockManagerLockDaemonPrivate {
 struct _virLockManagerLockDaemonDriver {
     bool autoDiskLease;
     bool requireLeaseForDisks;
+    bool useFlock;
 
     char *fileLockSpaceDir;
     char *lvmLockSpaceDir;
@@ -125,6 +126,10 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile)
     CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG);
     if (p) driver->autoDiskLease = p->l;
 
+    p = virConfGetValue(conf, "use_flock");
+    CHECK_TYPE("use_flock", VIR_CONF_LONG);
+    if (p) driver->useFlock = p->l;
+
     p = virConfGetValue(conf, "file_lockspace_dir");
     CHECK_TYPE("file_lockspace_dir", VIR_CONF_STRING);
     if (p && p->str) {
@@ -379,6 +384,7 @@ static int virLockManagerLockDaemonInit(unsigned int version,
 
     driver->requireLeaseForDisks = true;
     driver->autoDiskLease = true;
+    driver->useFlock = false;
 
     if (virLockManagerLockDaemonLoadConfig(configFile) < 0)
         goto error;
@@ -724,6 +730,8 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock,
             args.name = priv->resources[i].name;
             args.flags = priv->resources[i].flags;
 
+            if (driver->useFlock)
+                args.flags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK;
             if (virNetClientProgramCall(program,
                                         client,
                                         counter++,
@@ -780,10 +788,12 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock,
         args.name = priv->resources[i].name;
         args.flags = priv->resources[i].flags;
 
-        args.flags &=
-            ~(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
+        args.flags &=~(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED |
               VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE);
 
+        if (driver->useFlock)
+            args.flags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK;
+
         if (virNetClientProgramCall(program,
                                     client,
                                     counter++,
diff --git a/src/locking/lock_protocol.x b/src/locking/lock_protocol.x
index 354d51a..7828674 100644
--- a/src/locking/lock_protocol.x
+++ b/src/locking/lock_protocol.x
@@ -52,7 +52,8 @@ struct virLockSpaceProtocolDeleteResourceArgs {
 
 enum virLockSpaceProtocolAcquireResourceFlags {
     VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED     = 1,
-    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2
+    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2,
+    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_USE_FLOCK  = 4
 };
 
 struct virLockSpaceProtocolAcquireResourceArgs {
diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf
index 85edb91..944c808 100644
--- a/src/locking/lockd.conf
+++ b/src/locking/lockd.conf
@@ -17,6 +17,13 @@
 #
 #require_lease_for_disks = 1
 
+#
+# The default lockd behaviour is to acquire locks via their
+# the fnctl(2) syscall. This isn't aviable on all filesystems.
+# In this case you can use this flag to use flock(2) instead.
+#
+#use_flock = 0
+
 
 #
 # The default lockd behaviour is to use the "direct"
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 8f0eec3..56900f0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -378,7 +378,7 @@ int virFileLock(int fd, bool shared, off_t start, off_t len)
  * @start: byte offset to start unlock
  * @len: length of lock (0 to release entire remaining file from @start)
  *
- * Release a lock previously acquired with virFileUnlock().
+ * Release a lock previously acquired with virFileLock().
  * NB the lock will also be released if any open file descriptor
  * pointing to the same file as @fd is closed
  *
@@ -398,6 +398,63 @@ int virFileUnlock(int fd, off_t start, off_t len)
 
     return 0;
 }
+
+/**
+ * virFileLockFlock:
+ * @fd: file descriptor to acquire the lock on
+ * @shared: type of lock to acquire
+ *
+ * Attempt to acquire a lock on the file @fd. If @shared
+ * is true, then a shared lock will be acquired,
+ * otherwise an exclusive lock will be acquired. If
+ * the lock cannot be acquired, an error will be
+ * returned. This will not wait to acquire the lock if
+ * another process already holds it.
+ *
+ * The lock will be released when @fd is closed. The lock
+ * will also be released if *any* other open file descriptor
+ * pointing to the same underlying file is closed. As such
+ * this function should not be relied on in multi-threaded
+ * apps where other threads can be opening/closing arbitrary
+ * files.
+ *
+ * Returns 0 on success, or -errno otherwise
+ */
+int virFileLockFlock(int fd, bool shared)
+{
+     if (shared) {
+         if (flock(fd, LOCK_SH | LOCK_NB) < 0)
+             return -errno;
+     } else {
+         if (flock(fd, LOCK_EX | LOCK_NB) < 0)
+             return -errno;
+     }
+
+    return 0;
+}
+
+
+/**
+ * virFileUnlockFlock:
+ * @fd: file descriptor to release the lock on
+ * @start: byte offset to start unlock
+ *
+ * Release a lock previously acquired with virFileLockFlock().
+ * NB the lock will also be released if any open file descriptor
+ * pointing to the same file as @fd is closed
+ *
+ * Returns 0 on succcess, or -errno on error
+ */
+int virFileUnlockFlock(int fd)
+{
+     if (flock(fd, LOCK_UN) < 0)
+         return -errno;
+
+    return 0;
+}
+
+
+
 #else
 int virFileLock(int fd ATTRIBUTE_UNUSED,
                 bool shared ATTRIBUTE_UNUSED,
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72d35ce..e8ced5c 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -100,6 +100,10 @@ void virFileWrapperFdFree(virFileWrapperFdPtr dfd);
 int virFileLock(int fd, bool shared, off_t start, off_t len);
 int virFileUnlock(int fd, off_t start, off_t len);
 
+int virFileLockFlock(int fd, bool shared);
+int virFileUnlockFlock(int fd);
+
+
 typedef int (*virFileRewriteFunc)(int fd, void *opaque);
 int virFileRewrite(const char *path,
                    mode_t mode,
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index afb1abb..ef95672 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -154,17 +154,32 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
                 goto error;
             }
 
-            if (virFileLock(res->fd, shared, 0, 1) < 0) {
-                if (errno == EACCES || errno == EAGAIN) {
-                    virReportError(VIR_ERR_RESOURCE_BUSY,
-                                   _("Lockspace resource '%s' is locked"),
-                                   resname);
-                } else {
-                    virReportSystemError(errno,
-                                         _("Unable to acquire lock on '%s'"),
-                                         res->path);
+            if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK) {
+                if (virFileLockFlock(res->fd, shared) < 0) {
+                    if (errno == EACCES || errno == EAGAIN) {
+                        virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                    } else {
+                        virReportSystemError(errno,
+                                            _("Unable to acquire lock on '%s'"),
+                                            res->path);
+                    }
+                    goto error;
+                }
+            } else {
+                if (virFileLock(res->fd, shared, 0, 1) < 0) {
+                    if (errno == EACCES || errno == EAGAIN) {
+                        virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                    } else {
+                        virReportSystemError(errno,
+                                            _("Unable to acquire lock on '%s'"),
+                                            res->path);
+                    }
+                    goto error;
                 }
-                goto error;
             }
 
             /* Now make sure the pidfile we locked is the same
@@ -201,17 +216,32 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
             goto error;
         }
 
-        if (virFileLock(res->fd, shared, 0, 1) < 0) {
-            if (errno == EACCES || errno == EAGAIN) {
-                virReportError(VIR_ERR_RESOURCE_BUSY,
-                               _("Lockspace resource '%s' is locked"),
-                               resname);
-            } else {
-                virReportSystemError(errno,
-                                     _("Unable to acquire lock on '%s'"),
-                                     res->path);
+        if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK) {
+            if (virFileLockFlock(res->fd, shared) < 0) {
+                if (errno == EACCES || errno == EAGAIN) {
+                    virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                } else {
+                    virReportSystemError(errno,
+                                            _("Unable to acquire lock on '%s'"),
+                                            res->path);
+                }
+                goto error;
+            }
+        } else {
+            if (virFileLock(res->fd, shared, 0, 1) < 0) {
+                if (errno == EACCES || errno == EAGAIN) {
+                    virReportError(VIR_ERR_RESOURCE_BUSY,
+                                    _("Lockspace resource '%s' is locked"),
+                                    resname);
+                } else {
+                    virReportSystemError(errno,
+                                            _("Unable to acquire lock on '%s'"),
+                                            res->path);
+                }
+                goto error;
             }
-            goto error;
         }
     }
     res->lockHeld = true;
@@ -616,9 +646,17 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
 
     VIR_DEBUG("lockspace=%p resname=%s flags=%x owner=%lld",
               lockspace, resname, flags, (unsigned long long)owner);
+    
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)
+         VIR_DEBUG("David: shared");
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE)
+         VIR_DEBUG("David: autocreate");
+    if (flags & VIR_LOCK_SPACE_ACQUIRE_FLOCK)
+         VIR_DEBUG("David: flock");
 
     virCheckFlags(VIR_LOCK_SPACE_ACQUIRE_SHARED |
-                  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, -1);
+                  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE |
+                  VIR_LOCK_SPACE_ACQUIRE_FLOCK, -1);
 
     virMutexLock(&lockspace->lock);
 
diff --git a/src/util/virlockspace.h b/src/util/virlockspace.h
index 041cf20..a6ba3dd 100644
--- a/src/util/virlockspace.h
+++ b/src/util/virlockspace.h
@@ -45,6 +45,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
 typedef enum {
     VIR_LOCK_SPACE_ACQUIRE_SHARED     = (1 << 0),
     VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE = (1 << 1),
+    VIR_LOCK_SPACE_ACQUIRE_FLOCK      = (1 << 2),
 } virLockSpaceAcquireFlags;
 
 int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
-- 
1.8.1.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]