[libvirt] Re-2: [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 at munzinger.de>
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 at redhat.com
Cc: libvir-list at redhat.com
    sanlock-devel at lists.fedorahosted.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: add_ignore_param_to_sanlock.patch
Type: application/octet-stream
Size: 1911 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120514/62e5ecef/attachment-0001.obj>