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 
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: [e-mail]
Cc: [e-mail]
    [e-mail]