Add ability to maintain disk leases indirectly

The default lockd driver behavour is to acquire leases
directly on the disk files. This introduces an alternative
mode, where leases are acquire indirectly on a file that
is based on a SHA256 hash of the disk filename.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/bootstrap.conf b/bootstrap.conf
index 59dd2580f1ffd77b297583e95465fa5cd9a3be04..37a0ae1b5bb4d3863ac0bcfc1954faad62cb3bf0 100644 (file)
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -36,6 +36,7 @@ connect
 configmake
 count-one-bits
 crypto/md5
+crypto/sha256
 dirname-lgpl
 environ
 execinfo

diff --git a/src/Makefile.am b/src/Makefile.am
index a5c666e76ba37de5c5c54441758da891cf8ef3b2..bb80992c0ab6187c8db39a793be571a1ee84175f 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1942,6 +1942,7 @@ EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES)
 install-data-local: install-init install-systemd
 if WITH_LIBVIRTD
        $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd"
+       $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd/files"
        $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lockd"
 endif
        $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt"
@@ -1994,6 +1995,7 @@ endif
 
 uninstall-local:: uninstall-init uninstall-systemd
 if WITH_LIBVIRTD
+       rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd/files" ||:
        rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" ||:
        rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" ||:
 endif

diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug
index 4649644562206b77fcbf869a14192762646b289e..dafd8f9d0b73b478a9b855494b4c63c6a69731f9 100644 (file)
--- 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"
+             | str_entry "file_lockspace_dir"
    let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
    let empty = [ label "#empty" . eol ]
 

diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 5a7fbaf5c0d6df6529fda7a1fd4f55a84d9bfc13..aa0f94a4641c67b4d42fd8e88e477ea760e030e8 100644 (file)
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -32,6 +32,7 @@
 #include "rpc/virnetclient.h"
 #include "lock_protocol.h"
 #include "configmake.h"
+#include "sha256.h"
 
 #define VIR_FROM_THIS VIR_FROM_LOCKING
 
@@ -70,6 +71,8 @@ struct _virLockManagerLockDaemonPrivate {
 struct _virLockManagerLockDaemonDriver {
     bool autoDiskLease;
     bool requireLeaseForDisks;
+
+    char *fileLockSpaceDir;
 };
 
 static virLockManagerLockDaemonDriverPtr driver = NULL;
@@ -120,6 +123,17 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile)
     CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG);
     if (p) driver->autoDiskLease = p->l;
 
+    p = virConfGetValue(conf, "file_lockspace_dir");
+    CHECK_TYPE("file_lockspace_dir", VIR_CONF_STRING);
+    if (p && p->str) {
+        VIR_FREE(driver->fileLockSpaceDir);
+        if (!(driver->fileLockSpaceDir = strdup(p->str))) {
+            virReportOOMError();
+            virConfFree(conf);
+            return -1;
+        }
+    }
+
     p = virConfGetValue(conf, "require_lease_for_disks");
     CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG);
     if (p)
@@ -288,6 +302,47 @@ error:
 }
 
 
+static int virLockManagerLockDaemonSetupLockspace(const char *path)
+{
+    virNetClientPtr client;
+    virNetClientProgramPtr program = NULL;
+    virLockSpaceProtocolCreateLockSpaceArgs args;
+    int rv = -1;
+    int counter = 0;
+
+    memset(&args, 0, sizeof(args));
+    args.path = (char*)path;
+
+    if (!(client = virLockManagerLockDaemonConnectionNew(getuid() == 0, &program)))
+        return -1;
+
+    if (virNetClientProgramCall(program,
+                                client,
+                                counter++,
+                                VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE,
+                                0, NULL, NULL, NULL,
+                                (xdrproc_t)xdr_virLockSpaceProtocolCreateLockSpaceArgs, (char*)&args,
+                                (xdrproc_t)xdr_void, NULL) < 0) {
+        virErrorPtr err = virGetLastError();
+        if (err && err->code == VIR_ERR_OPERATION_INVALID) {
+            /* The lockspace already exists */
+            virResetLastError();
+            rv = 0;
+        } else {
+            goto cleanup;
+        }
+    }
+
+    rv = 0;
+
+cleanup:
+    virObjectUnref(program);
+    virNetClientClose(client);
+    virObjectUnref(client);
+    return rv;
+}
+
+
 static int virLockManagerLockDaemonDeinit(void);
 
 static int virLockManagerLockDaemonInit(unsigned int version,
@@ -312,6 +367,13 @@ static int virLockManagerLockDaemonInit(unsigned int version,
     if (virLockManagerLockDaemonLoadConfig(configFile) < 0)
         goto error;
 
+    if (driver->autoDiskLease) {
+        if (driver->fileLockSpaceDir &&
+            virLockManagerLockDaemonSetupLockspace(driver->fileLockSpaceDir) < 0)
+            goto error;
+    }
+
+
     return 0;
 
 error:
@@ -324,6 +386,7 @@ static int virLockManagerLockDaemonDeinit(void)
     if (!driver)
         return 0;
 
+    VIR_FREE(driver->fileLockSpaceDir);
     VIR_FREE(driver);
 
     return 0;
@@ -421,6 +484,36 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock,
 }
 
 
+static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7',
+                            '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
+
+static char *virLockManagerLockDaemonDiskLeaseName(const char *path)
+{
+    unsigned char buf[SHA256_DIGEST_SIZE];
+    char *ret;
+    int i;
+
+    if (!(sha256_buffer(path, strlen(path), buf))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Unable to compute sha256 checksum"));
+        return NULL;
+    }
+
+    if (VIR_ALLOC_N(ret, (SHA256_DIGEST_SIZE * 2) + 1) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    for (i = 0 ; i < SHA256_DIGEST_SIZE ; i++) {
+        ret[i*2] = hex[(buf[i] >> 4) & 0xf];
+        ret[(i*2)+1] = hex[buf[i] & 0xf];
+    }
+    ret[(SHA256_DIGEST_SIZE * 2) + 1] = '\0';
+
+    return ret;
+}
+
+
 static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
                                                unsigned int type,
                                                const char *name,
@@ -429,8 +522,9 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
                                                unsigned int flags)
 {
     virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
-    char *newName;
+    char *newName = NULL;
     char *newLockspace = NULL;
+    bool autoCreate = false;
 
     virCheckFlags(VIR_LOCK_MANAGER_RESOURCE_READONLY |
                   VIR_LOCK_MANAGER_RESOURCE_SHARED, -1);
@@ -451,10 +545,22 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
                 priv->hasRWDisks = true;
             return 0;
         }
-        if (!(newLockspace = strdup(""))) {
-            virReportOOMError();
-            return -1;
+
+        if (driver->fileLockSpaceDir) {
+            if (!(newLockspace = strdup(driver->fileLockSpaceDir)))
+                goto no_memory;
+            if (!(newName = virLockManagerLockDaemonDiskLeaseName(name)))
+                goto no_memory;
+            autoCreate = true;
+            VIR_DEBUG("Using indirect lease %s for %s", newName, name);
+        } else {
+            if (!(newLockspace = strdup("")))
+                goto no_memory;
+            if (!(newName = strdup(name)))
+                goto no_memory;
+            VIR_DEBUG("Using direct lease for %s", name);
         }
+
         break;
     case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE: {
         size_t i;
@@ -488,6 +594,9 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
             virReportOOMError();
             return -1;
         }
+        if (!(newName = strdup(name)))
+            goto no_memory;
+
     }   break;
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -496,9 +605,6 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
         return -1;
     }
 
-    if (!(newName = strdup(name)))
-        goto no_memory;
-
     if (VIR_EXPAND_N(priv->resources, priv->nresources, 1) < 0)
         goto no_memory;
 
@@ -509,10 +615,15 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
         priv->resources[priv->nresources-1].flags |=
             VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
 
+    if (autoCreate)
+        priv->resources[priv->nresources-1].flags |=
+            VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE;
+
     return 0;
 
 no_memory:
     virReportOOMError();
+    VIR_FREE(newLockspace);
     VIR_FREE(newName);
     return -1;
 }
@@ -556,7 +667,7 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock,
             memset(&args, 0, sizeof(args));
 
             if (priv->resources[i].lockspace)
-            args.path = priv->resources[i].lockspace;
+                args.path = priv->resources[i].lockspace;
             args.name = priv->resources[i].name;
             args.flags = priv->resources[i].flags;
 

diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf
index 0b885c534ff40b2c5febe7889697c9011aad86b3..7545fd97ee06dc58476349af7b51926c73ca307e 100644 (file)
--- a/src/locking/lockd.conf
+++ b/src/locking/lockd.conf
@@ -16,3 +16,25 @@
 # to enabled, otherwise it defaults to disabled.
 #
 #require_lease_for_disks = 1
+
+
+#
+# The default lockd behaviour is to use the "direct"
+# lockspace, where the locks are acquired against the
+# actual file paths associated with the <disk> devices.
+#
+# Setting a directory here causes lockd to use "indirect"
+# lockspace, where a hash of the <disk> file path is
+# used to create a file in the lockspace directory. The
+# locks are then held on these hash files instead.
+#
+# This can be useful if the file paths refer to block
+# devices which are shared, since /dev fcntl() locks
+# don't propagate across hosts. It is also useful if
+# the filesystem does not support fcntl() locks.
+#
+# Typically this directory would be located on a shared
+# filesystem visible to all hosts accessing the same
+# storage.
+#
+#file_lockspace_dir = "/var/lib/libvirt/lockd/files"

diff --git a/src/locking/test_libvirt_lockd.aug.in b/src/locking/test_libvirt_lockd.aug.in
index 5be0d99b94f7eb14150192e1f4c66d509cc92a39..2e65af62175f3d93c12782e09ee979725e1b617f 100644 (file)
--- a/src/locking/test_libvirt_lockd.aug.in
+++ b/src/locking/test_libvirt_lockd.aug.in
@@ -4,3 +4,4 @@ module Test_libvirt_lockd =
    test Libvirt_lockd.lns get conf =
 { "auto_disk_leases" = "0" }
 { "require_lease_for_disks" = "1" }
+{ "file_lockspace_dir" = "/var/lib/libvirt/lockd/files" }