4:53 a.m.
On Mon, May 14, 2012 at 08:32:01AM +0000, David Weber wrote:
> > 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.
Thanks, your patch looks good but needs two further small additions
to the libvirt_sanlock.aug and test_libvirt_sanlock.aug files to
take account of the new config parameter.
Oops, Updated patch attached (tested with augparse)
David
----------------------------------------
commit 33678a8b2d294bebf327106d586d41c9b157174f
Author: David Weber <wb(a)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/libvirt_sanlock.aug b/src/locking/libvirt_sanlock.aug
index 5f5f8a1..d65b002 100644
--- a/src/locking/libvirt_sanlock.aug
+++ b/src/locking/libvirt_sanlock.aug
module Libvirt_sanlock =
| bool_entry "auto_disk_leases"
| int_entry "host_id"
| bool_entry "require_lease_for_disks"
+ | bool_entry "ignore_readonly_and_shared_disks"
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_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
diff --git a/src/locking/test_libvirt_sanlock.aug b/src/locking/test_libvirt_sanlock.aug
index b5169e1..90ab59f 100644
--- a/src/locking/test_libvirt_sanlock.aug
+++ b/src/locking/test_libvirt_sanlock.aug
module Test_libvirt_sanlock =
disk_lease_dir = \"/var/lib/libvirt/sanlock\"
host_id = 1
require_lease_for_disks = 1
+ignore_readonly_and_shared_disks = 1
"
test Libvirt_sanlock.lns get conf =
require_lease_for_disks = 1
{ "disk_lease_dir" = "/var/lib/libvirt/sanlock" }
{ "host_id" = "1" }
{ "require_lease_for_disks" = "1" }
+{ "ignore_readonly_and_shared_disks" = "1" }
To: berrange(a)redhat.com
Cc: libvir-list(a)redhat.com
sanlock-devel(a)lists.fedorahosted.org
5:46 p.m.
On 05/14/2012 03:53 AM, David Weber wrote:
>
> Thanks, your patch looks good but needs two further small additions
> to the libvirt_sanlock.aug and test_libvirt_sanlock.aug files to
> take account of the new config parameter.
Oops, Updated patch attached (tested with augparse)
David
----------------------------------------
commit 33678a8b2d294bebf327106d586d41c9b157174f
Author: David Weber <wb(a)munzinger.de>
Date: Mon May 14 09:43:27 2012 +0200
Add ignore param for readonly and shared disk in sanlock
ACK and pushed, after fixing up whitespace nits ('make syntax-check'
would have caught the trailing whitespace, but not the other one), and
adding you to AUTHORS (let me know if you prefer an alternate spelling).
+
Trailing whitespace.
+ if (((flags & VIR_LOCK_MANAGER_RESOURCE_READONLY) ||
+ (flags &VIR_LOCK_MANAGER_RESOURCE_SHARED)) &&
Space on both sides of binary operators. Furthermore, you can simplify
this to: (flags & (VIR_LOCK_MANAGER_RESOURCE_READONLY |
VIR_LOCK_MANAGER_RESOURCE_SHARED)).
+ (driver->ignoreReadonlyShared)) {
Spurious parenthesis.
+
+#
+# Ignore readonly and shared disks as they aren't supportet yet
s/supportet/supported/
Is it sanlock, or libvirt, or a combination of both that don't yet
support shared resource locks? I modified this as follows (and hope
that it was good enough, since I pushed):
# Enable this flag to have sanlock ignore readonly and shared disks.
# If disabled, then this rejects attempts to share resources until
# sanlock gains support for shared locks.
#
#ignore_readonly_and_shared_disks = 1
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4575
days inactive
4575
days old
1 comments
2 participants
participants (2)
-
David Weber -
Eric Blake