Re-2: [libvirt] [Patch] Libvirt - Fix locking for readonly devices

[David Weber]

>  On Tue, May 08, 2012 at 09:41:02AM +0000, David Weber wrote:
>  > Hi,
>  > 
>  > I'm currently working on getting sanlock into Debian/Ubuntu.
>  > While testing, I noticed that I wasn't able to add a readonly 
>  > or shared device: "internal error unsupported 
>  > configuration: Readonly leases are not supported".
>  > 
>  > After looking into the source, it seems to be the following
>  > situation:
>  > - Libvirt passes every device to the sanlock plugin, even if
>  > it is readonly
>  > - The sanlock plugin rejects to add a lease for the readonly
>  > device, returning an error so the machine starts to fail
>  > 
>  > The attached patch rejects passing readonly and shared devices 
>  > to the lock-plugin so they shouldn't be a problem anymore.
>  
>  This isn't good - the lock manager implementation must be the
>  one to decide what todo with readonly & shared disks. The sanlock
>  plugin, however, does not currently support readonly/shared leases
>  hence why it rejects them. We could probably add a config param
>  to allow readonly/shared leases to be skipped by the sanlock plugin.
>  
Thanks for clarification. I've attached an updated patch which 
adds such a config param. It works but I can't test 
live-migration at the moment. But as far as I understood it 
shouldn't be a problem.

David



----------------------------------------------------------------
commit 5a1546f328b166871fbf0e0556b5a2aafba93d63
Author: David Weber <wb(a)munzinger.de&gt;
Date:   Mon May 14 09:43:27 2012 +0200

    Add ignore param for readonly and shared disk in sanlock

diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index d344d6a..57b688a 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
                  struct _virLockManagerSanlockDriver {
     bool requireLeaseForDisks;
     int hostID;
     bool autoDiskLease;
+    bool ignoreReadonlyShared;
     char *autoDiskLeasePath;
 };
 
                     static int virLockManagerSanlockLoadConfig(const char *configFile)
     CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG);
     if (p) driver->autoDiskLease = p->l;
 
+    p = virConfGetValue(conf, "ignore_readonly_and_shared_disks");
+    CHECK_TYPE("ignore_readonly_and_shared_disks", VIR_CONF_LONG);
+    if (p) driver->ignoreReadonlyShared = p->l;
+
     p = virConfGetValue(conf, "disk_lease_dir");
     CHECK_TYPE("disk_lease_dir", VIR_CONF_STRING);
     if (p && p->str) {
                     static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
                      SANLK_MAX_RESOURCES);
         return -1;
     }
+    
+    if (((flags & VIR_LOCK_MANAGER_RESOURCE_READONLY) ||
+        (flags &VIR_LOCK_MANAGER_RESOURCE_SHARED)) &&
+        (driver->ignoreReadonlyShared)) {
+            return 0;
+    }
 
     if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY) {
         virLockError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/locking/sanlock.conf b/src/locking/sanlock.conf
index efc35ee..5429522 100644
--- a/src/locking/sanlock.conf
+++ b/src/locking/sanlock.conf
                 
 # to enabled, otherwise it defaults to disabled.
 #
 #require_lease_for_disks = 1
+
+#
+# Ignore readonly and shared disks as they aren't supportet yet
+#
+#ignore_readonly_and_shared_disks = 1



To: berrange(a)redhat.com
Cc: libvir-list(a)redhat.com
    sanlock-devel(a)lists.fedorahosted.org